Skip to content

fix(tracing): send addStackToTracingNoReply on the LocalUtils connection - #1967

Open
monkey (Develop-KIM) wants to merge 2 commits into
microsoft:mainfrom
Develop-KIM:fix-1962
Open

fix(tracing): send addStackToTracingNoReply on the LocalUtils connection#1967
monkey (Develop-KIM) wants to merge 2 commits into
microsoft:mainfrom
Develop-KIM:fix-1962

Conversation

@Develop-KIM

Copy link
Copy Markdown
Contributor

Summary

  • Connection.internalSendMessage addressed addStackToTracingNoReply to localUtils.guid but sent it over its own transport. For a browser from connect() that transport is the server pipe, not the driver connection that owns LocalUtils, so the stack session opened by tracingStarted never received anything and setSources(true) produced no sources.
  • Sends it on localUtils.connection instead, matching this._localUtils?.addStackToTracingNoReply(...) in packages/playwright-core/src/client/connection.ts. For a local connection that is the same object, so nothing changes there.
  • TestBrowserTypeConnect#shouldRecordTraceWithSources already covered this and fails on main (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

@yury-s Yury Semikhatsky (yury-s) left a comment

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.

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.
@Develop-KIM

Copy link
Copy Markdown
Contributor Author

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.

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.

[Bug]: Tracing setSources(true) produces no sources when the browser is connected via connect()

2 participants