Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] trace viewer opens in empty state while debugging #515

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ruifigueira
Copy link
Contributor

Bug detected in #513 (comment), but it also happens with public releases of playwright / playwright-vscode.

To reproduce:

  • initialize a playwright test project with npm init playwright@latest
  • in Testing, ensure Show trace viewer is selected
  • in Testing Explorer, expand and select a leaf test item, e.g. has title
  • click the Debug Test button

Eventually, it will open the trace viewer app showing Select test to see the trace.

This PR tries to fix it by ensuring that trace paths are only associated with corresponding test items when they are not running in debug.

Environment:

  System:
    OS: Windows 11 10.0.22631
    CPU: (8) x64 11th Gen Intel(R) Core(TM) i5-1135G7 @ 2.40GHz
    Memory: 11.99 GB / 31.74 GB
  Binaries:
    Node: 20.15.0 - ~\scoop\apps\nvm\current\nodejs\nodejs\node.EXE
    npm: 10.7.0 - ~\scoop\apps\nvm\current\nodejs\nodejs\npm.CMD
    pnpm: 8.15.4 - ~\AppData\Local\pnpm\pnpm.EXE
  IDEs:
    VSCode: 1.92.0 - C:\Users\rui.figueira\scoop\apps\vscode\current\bin\code.CMD
  npmPackages:
    @playwright/test: ^1.46.0 => 1.46.0

Playwright Test for VSCode: v1.1.7

@pavelfeldman
Copy link
Member

I did not realize this was intended to land first.

@ruifigueira
Copy link
Contributor Author

I'll rebase it, no worries.

@ruifigueira ruifigueira force-pushed the fix/trace-viewer-debug branch 2 times, most recently from 9d1a7a9 to 143e22f Compare August 13, 2024 23:28

const testRunPromise = testController.run();
// wait for test to start
await waitForTestItemStatus(testItem, 'started');
Copy link
Member

Choose a reason for hiding this comment

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

sounds like the next line should take care of it since you are polling?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My problem is that the polling fails with timeout, at least in my computer, and that's because tests are taking more than 5 seconds to start.

That waitForTestItemStatus(testItem, 'started') is just to ensure test is in a state where I have confidence I won't get a timeout while polling.


test.skip(({ traceViewerMode }) => !traceViewerMode);

async function waitForTestItemStatus(testItem: TestItem, status: TestItem['status']) {
Copy link
Member

Choose a reason for hiding this comment

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

It does not sound like we should need it, it is better to make the assertion that follows a waiting assertion.

@@ -193,6 +193,8 @@ export class TestItem {
tags: readonly TestTag[] = [];
canResolveChildren = false;
status: 'none' | 'enqueued' | 'started' | 'skipped' | 'failed' | 'passed' = 'none';
_didChangeStatus = new EventEmitter<TestItem>();
onDidChangeStatus = this._didChangeStatus.event;
Copy link
Member

Choose a reason for hiding this comment

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

I don't see this on vscode.TestItem,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll use TestController.onDidCreateTestRun > TestRun.onDidChange to achieve the same goal.

@ruifigueira
Copy link
Contributor Author

Rebased it again, and adapted it to new changes.

@pavelfeldman I noticed the embeddedTraceViewer setting is not materialized in the actual VS Code, and I found no path to enable it except in the mocked vscode.

I had to change the value directly in the code to be able to test it in VS Code. Is there any other way?

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