-
Notifications
You must be signed in to change notification settings - Fork 296
fix(tracing): send addStackToTracingNoReply on the LocalUtils connection - #1967
fix(tracing): send addStackToTracingNoReply on the LocalUtils connection #1967monkey (Develop-KIM) wants to merge 3 commits into
Conversation
@yury-s
Yury Semikhatsky (yury-s)
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mind adding a test?
The existing coverage lives in TestBrowserTypeConnect#shouldRecordTraceWithSources, which is skipped unless PLAYWRIGHT_JAVA_SRC is set, and the job that sets it runs only *TestTracing*. The new class passes the source root through Playwright.CreateOptions instead, so it runs in the regular test job.
monkey (Develop-KIM)
commented
Sep 6, 2026
Added a test: TestTracingOverConnect connects to a launch-server browser and traces with setSources(true). It passes the source root through Playwright.CreateOptions instead of reading PLAYWRIGHT_JAVA_SRC from the environment, so it runs in the regular test job, not only the *TestTracing* sources one. Without the fix it gets 0 sources instead of 1.
Since the driver roll in microsoft#1972 the trace zip stores sources as src/<sha1>.<ext> instead of resources/src@<sha1>.txt, so the .txt filter in TestTracingOverConnect found nothing on CI. shouldRecordTraceWithSources in TestBrowserTypeConnect used the same filter and gets the same update.
monkey (Develop-KIM)
commented
Sep 9, 2026
The CI failure came from main rolling the driver to 1.63 (#1972) after this branch was cut: the trace zip now stores sources as src/<sha1>.java instead of resources/src@<sha1>.txt, so the .txt filter in the new test found nothing. Switched it to the startsWith("src/") check that TestJavaSourceLocationInConstructor got in that roll, and gave shouldRecordTraceWithSources in TestBrowserTypeConnect the same update since it had the same filter.
Summary
Connection.internalSendMessageaddressedaddStackToTracingNoReplytolocalUtils.guidbut sent it over its own transport. For a browser fromconnect()that transport is the server pipe, not the driver connection that ownsLocalUtils, so the stack session opened bytracingStartednever received anything andsetSources(true)produced no sources.localUtils.connectioninstead, matchingthis._localUtils?.addStackToTracingNoReply(...)inpackages/playwright-core/src/client/connection.ts. For a local connection that is the same object, so nothing changes there.TestBrowserTypeConnect#shouldRecordTraceWithSourcesalready covered this and fails onmain(0 sources instead of 1); it passes with the change. It never ran on CI because the sources job filters on-D test=*TestTracing*.Fixes #1962