Skip to content

fix(trace-viewer): Keep network entry selected during test run#39076

Open
cpAdm wants to merge 3 commits intomicrosoft:mainfrom
cpAdm:fix-network-entry-selection-during-run
Open

fix(trace-viewer): Keep network entry selected during test run#39076
cpAdm wants to merge 3 commits intomicrosoft:mainfrom
cpAdm:fix-network-entry-selection-during-run

Conversation

@cpAdm
Copy link
Contributor

@cpAdm cpAdm commented Feb 1, 2026

The polling every 500s causes the networkModel to reset, causing the networkModel.resources to be a new array (with (partially) same values), so ===-comparison will not work. Ideally the Entry would have a unique id-property, this JSON.stringify solution is not ideal.

Closes: #39050

@cpAdm cpAdm changed the title fix: Keep network entry selected during test run fix(trace-viewer): Keep network entry selected during test run Feb 2, 2026
@dgozman dgozman requested a review from Skn0tt February 3, 2026 11:08
@Skn0tt
Copy link
Member

Skn0tt commented Feb 3, 2026

Ouch, sorry I didn't catch this in my earlier review. Calling JSON.stringify on each entry seems a little expensive for large traces.

Ideally the Entry would have a unique id-property

Agreed. Looking through utils/isomorphic/trace, it seems that the combination of contextEntry.contextId and resources index should be both unique and stable across reloads. Maybe we can use that?

@cpAdm
Copy link
Contributor Author

cpAdm commented Feb 3, 2026

Ouch, sorry I didn't catch this in my earlier review. Calling JSON.stringify on each entry seems a little expensive for large traces.

Ideally the Entry would have a unique id-property

Agreed. Looking through utils/isomorphic/trace, it seems that the combination of contextEntry.contextId and resources index should be both unique and stable across reloads. Maybe we can use that?

Seems good. Added the id in useNetworkTabModel right before it's filtered (due to timeline usage)

@cpAdm
Copy link
Contributor Author

cpAdm commented Feb 3, 2026

Seems good. Added the id in useNetworkTabModel right before it's filtered (due to timeline usage)

If this is a good id, I think we should also follow up and remove ordinal from RenderedEntry (referred as highlightedResourceOrdinal in Timeline) and also use this new stable id there.

Copy link
Member

@Skn0tt Skn0tt left a comment

Choose a reason for hiding this comment

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

looks good, thanks! the proposed refactoring sounds good.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 4, 2026

Test results for "tests 1"

2 failed
❌ [playwright-test] › ui-mode-test-network-tab.spec.ts:371 › should not preserve selection across test runs @macos-latest-node20
❌ [playwright-test] › update-aria-snapshot.spec.ts:592 › update-source-method › should overwrite source @macos-latest-node20

3 flaky ⚠️ [firefox-library] › library/inspector/cli-codegen-1.spec.ts:1082 › cli codegen › should not throw csp directive violation errors `@firefox-ubuntu-22.04-node20`
⚠️ [playwright-test] › ui-mode-trace.spec.ts:700 › should indicate current test status `@macos-latest-node20`
⚠️ [playwright-test] › ui-mode-trace.spec.ts:812 › should update state on subsequent run `@macos-latest-node20`

34277 passed, 662 skipped


Merge workflow run.

@Skn0tt
Copy link
Member

Skn0tt commented Feb 4, 2026

@cpAdm the failing test seems very much related. Looks like we need to somehow incorporate the trace identity into the key. For example via model.traceUri or model.timestamp or a combination.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 4, 2026

Test results for "MCP"

33 failed
❌ [firefox] › mcp/files.spec.ts:106 › clicking on download link emits download @mcp-ubuntu-latest
❌ [chrome] › mcp/autowait.spec.ts:19 › racy navigation destroys context @mcp-windows-latest
❌ [chrome] › mcp/console.spec.ts:198 › console log file is created on snapshot @mcp-windows-latest
❌ [chrome] › mcp/network.spec.ts:75 › network log file is returned on snapshot @mcp-windows-latest
❌ [chrome] › mcp/network.spec.ts:118 › network log file separates requests and assets @mcp-windows-latest
❌ [chrome] › mcp/network.spec.ts:168 › network log file stores method and status @mcp-windows-latest
❌ [chrome] › mcp/network.spec.ts:209 › new network log file after navigation @mcp-windows-latest
❌ [chrome] › mcp/session-log.spec.ts:22 › session log should record tool calls @mcp-windows-latest
❌ [chromium] › mcp/console.spec.ts:198 › console log file is created on snapshot @mcp-windows-latest
❌ [chromium] › mcp/network.spec.ts:75 › network log file is returned on snapshot @mcp-windows-latest
❌ [chromium] › mcp/network.spec.ts:118 › network log file separates requests and assets @mcp-windows-latest
❌ [chromium] › mcp/network.spec.ts:168 › network log file stores method and status @mcp-windows-latest
❌ [chromium] › mcp/network.spec.ts:209 › new network log file after navigation @mcp-windows-latest
❌ [chromium] › mcp/session-log.spec.ts:22 › session log should record tool calls @mcp-windows-latest
❌ [firefox] › mcp/console.spec.ts:198 › console log file is created on snapshot @mcp-windows-latest
❌ [firefox] › mcp/files.spec.ts:106 › clicking on download link emits download @mcp-windows-latest
❌ [firefox] › mcp/network.spec.ts:75 › network log file is returned on snapshot @mcp-windows-latest
❌ [firefox] › mcp/network.spec.ts:118 › network log file separates requests and assets @mcp-windows-latest
❌ [firefox] › mcp/network.spec.ts:168 › network log file stores method and status @mcp-windows-latest
❌ [firefox] › mcp/network.spec.ts:209 › new network log file after navigation @mcp-windows-latest
❌ [firefox] › mcp/session-log.spec.ts:22 › session log should record tool calls @mcp-windows-latest
❌ [webkit] › mcp/console.spec.ts:198 › console log file is created on snapshot @mcp-windows-latest
❌ [webkit] › mcp/network.spec.ts:75 › network log file is returned on snapshot @mcp-windows-latest
❌ [webkit] › mcp/network.spec.ts:118 › network log file separates requests and assets @mcp-windows-latest
❌ [webkit] › mcp/network.spec.ts:168 › network log file stores method and status @mcp-windows-latest
❌ [webkit] › mcp/network.spec.ts:209 › new network log file after navigation @mcp-windows-latest
❌ [msedge] › mcp/console.spec.ts:198 › console log file is created on snapshot @mcp-windows-latest
❌ [msedge] › mcp/network.spec.ts:75 › network log file is returned on snapshot @mcp-windows-latest
❌ [msedge] › mcp/network.spec.ts:118 › network log file separates requests and assets @mcp-windows-latest
❌ [msedge] › mcp/network.spec.ts:168 › network log file stores method and status @mcp-windows-latest
❌ [msedge] › mcp/network.spec.ts:209 › new network log file after navigation @mcp-windows-latest
❌ [msedge] › mcp/session-log.spec.ts:22 › session log should record tool calls @mcp-windows-latest
❌ [firefox] › mcp/files.spec.ts:106 › clicking on download link emits download @mcp-macos-15

1 flaky ⚠️ [chrome] › mcp/cdp.spec.ts:24 › cdp server `@mcp-windows-latest`

4576 passed, 135 skipped


Merge workflow run.

@cpAdm
Copy link
Contributor Author

cpAdm commented Feb 4, 2026

@cpAdm the failing test seems very much related. Looks like we need to somehow incorporate the trace identity into the key. For example via model.traceUri or model.timestamp or a combination.

It's interesting that it only fails on macOS. Is the contexid not unique there?

@Skn0tt
Copy link
Member

Skn0tt commented Feb 4, 2026

I'd guess the test is flaky, and macOS is just the flakiest of our CI runners.

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]: Since 1.58, Request can't be checked until the whole execution is done

2 participants