Skip to content

test-web: Capture all process output#8877

Open
jonbgamble wants to merge 5 commits intoLadybirdBrowser:masterfrom
jonbgamble:pr-test-web
Open

test-web: Capture all process output#8877
jonbgamble wants to merge 5 commits intoLadybirdBrowser:masterfrom
jonbgamble:pr-test-web

Conversation

@jonbgamble
Copy link
Copy Markdown
Contributor

@jonbgamble jonbgamble commented Apr 11, 2026

  • Suppress browser & helper process console writes so they don't "glitch scroll" stale test status lines above and past the live display area.
  • While we're suppressing them, might as well capture and log them.
  • Add the run date to the results header.
  • Add a button in the header by the filter box that copies the command line used to invoke the run to the clipboard.
  • Remove --debug-timeouts because it's not worth the code noise.
  • Use files rather than buffers for in-flight captures.

test-web

@jonbgamble jonbgamble force-pushed the pr-test-web branch 2 times, most recently from ab060fc to db69322 Compare April 12, 2026 00:21
@github-actions github-actions bot added the conflicts Pull request has merge conflicts that need resolution label Apr 12, 2026
@github-actions
Copy link
Copy Markdown

Your pull request has conflicts that need to be resolved before it can be reviewed and merged. Make sure to rebase your branch on top of the latest master.

It's almost 100% useless in practice and not worth the noise.
Use files instead of buffers. Consolidate stderr and stdout into one
view. Break handling out into TestRunCapture and CaptureFile helper
classes. We will use them to log output from ALL processes in the next
commit.
This prevents glitches where stale test status lines can bleed above
the live display list. Log this additional captured output so it can
be reviewed if a main or helper process failure is suspected.
@github-actions github-actions bot removed the conflicts Pull request has merge conflicts that need resolution label Apr 12, 2026

void ProcessManager::add_process(WebView::Process&& process)
{
Threading::MutexLocker locker { m_lock };
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This mutex seems dubious beforehand as well considering it does not protect find_process properly, but it does not seem correct to remove the mutex here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants