Skip to content

Commit

Permalink
fix(trace-viewer): don't show trace on debug
Browse files Browse the repository at this point in the history
  • Loading branch information
@ruifigueira
ruifigueira committed Aug 8, 2024
1 parent 32d319b commit 249d6cd
Show file tree
Hide file tree
Showing 4 changed files with 94 additions and 26 deletions.
16 changes: 8 additions & 8 deletions src/extension.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -457,8 +457,11 @@ export class Extension implements RunHooks {
const testItem = this._testTree.testItemForTest(test);
if (testItem) {
testRun.started(testItem);
const fullProject = ancestorProject(test);
const traceUrl = `${fullProject.outputDir}/.playwright-artifacts-${result.workerIndex}/traces/${test.id}.json`;
let traceUrl: string | undefined;
if (!isDebug) {
const fullProject = ancestorProject(test);
traceUrl = `${fullProject.outputDir}/.playwright-artifacts-${result.workerIndex}/traces/${test.id}.json`;
}
(testItem as any)[traceUrlSymbol] = traceUrl;
}

Expand All @@ -479,7 +482,7 @@ export class Extension implements RunHooks {
if (!testItem)
return;

const trace = result.attachments.find(a => a.name === 'trace')?.path || '';
const trace = result.attachments.find(a => a.name === 'trace')?.path;
// if trace viewer is currently displaying the trace file about to be replaced, it needs to be refreshed
const prevTrace = (testItem as any)[traceUrlSymbol];
(testItem as any)[traceUrlSymbol] = trace;
Expand Down Expand Up @@ -758,16 +761,13 @@ export class Extension implements RunHooks {

private _showTrace(testItem: vscodeTypes.TestItem) {
const traceUrl = (testItem as any)[traceUrlSymbol];
if (traceUrl)
this._traceViewer()?.open(traceUrl);
this._traceViewer()?.open(traceUrl);
}

private _treeItemSelected(treeItem: vscodeTypes.TreeItem | null) {
if (!treeItem)
return;
const traceUrl = (treeItem as any)[traceUrlSymbol] || '';
if (!traceUrl && !this._traceViewer()?.isStarted())
return;
const traceUrl = (treeItem as any)[traceUrlSymbol];
this._traceViewer()?.open(traceUrl);
}

Expand Down
14 changes: 5 additions & 9 deletions src/traceViewer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,6 @@ export class SpawnTraceViewer {
this._config = config;
}

isStarted() {
return !!this._traceViewerProcess;
}

currentFile() {
return this._currentFile;
}
Expand All @@ -47,10 +43,12 @@ export class SpawnTraceViewer {
await this._startIfNeeded();
}

async open(file: string) {
async open(file?: string) {
this._currentFile = file;
if (!file && !this._traceViewerProcess)
return;
await this._startIfNeeded();
this._traceViewerProcess?.stdin?.write(file + '\n');
this._currentFile = file;
}

private async _startIfNeeded() {
Expand Down Expand Up @@ -112,13 +110,11 @@ export class SpawnTraceViewer {
}

infoForTest() {
if (!this._serverUrlPrefixForTest)
return;
return {
type: 'spawn',
serverUrlPrefix: this._serverUrlPrefixForTest,
testConfigFile: this._config.configFile,
traceFile: this.currentFile(),
traceFile: this._currentFile,
};
}
}
7 changes: 7 additions & 0 deletions tests/mock/vscode.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

constructor(
readonly testController: TestController,
Expand Down Expand Up @@ -411,26 +413,31 @@ export class TestRun {

enqueued(test: TestItem) {
test.status = 'enqueued';
test._didChangeStatus.fire(test);
this._log(test, { status: 'enqueued' });
}

started(test: TestItem) {
test.status = 'started';
test._didChangeStatus.fire(test);
this._log(test, { status: 'started' });
}

skipped(test: TestItem) {
test.status = 'skipped';
test._didChangeStatus.fire(test);
this._log(test, { status: 'skipped' });
}

failed(test: TestItem, messages: TestMessage[], duration?: number) {
test.status = 'failed';
test._didChangeStatus.fire(test);
this._log(test, { status: 'failed', duration, messages });
}

passed(test: TestItem, duration?: number) {
test.status = 'passed';
test._didChangeStatus.fire(test);
this._log(test, { status: 'passed', duration });
}

Expand Down
83 changes: 74 additions & 9 deletions tests/spawn-trace-viewer.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/

import { enableConfigs, expect, selectConfig, selectTestItem, test, traceViewerInfo } from './utils';
import { TestItem } from './mock/vscode';

test.beforeEach(({ showBrowser }) => {
test.skip(showBrowser);
Expand All @@ -24,6 +25,13 @@ test.beforeEach(({ showBrowser }) => {

test.use({ showTrace: true, envRemoteName: 'ssh-remote' });

async function waitForTestItemStatus(testItem: TestItem, status: TestItem['status']) {
await new Promise<void>(resolve => testItem.onDidChangeStatus(() => {
if (testItem.status === 'started')
resolve();
}));
}

test('@smoke should open trace viewer', async ({ activate }) => {
const { vscode, testController } = await activate({
'playwright.config.js': `module.exports = { testDir: 'tests' }`,
Expand Down Expand Up @@ -83,7 +91,9 @@ test('should not open trace viewer if test did not run', async ({ activate }) =>
await testController.expandTestItems(/test.spec/);
selectTestItem(testController.findTestItems(/pass/)[0]);

await expect.poll(() => traceViewerInfo(vscode)).toBeUndefined();
await expect.poll(() => traceViewerInfo(vscode)).toMatchObject({
traceFile: undefined,
});
});

test('should refresh trace viewer while test is running', async ({ activate, createLatch }) => {
Expand All @@ -98,9 +108,13 @@ test('should refresh trace viewer while test is running', async ({ activate, cre
});

await testController.expandTestItems(/test.spec/);
selectTestItem(testController.findTestItems(/pass/)[0]);
const [testItem] = testController.findTestItems(/pass/);
selectTestItem(testItem);

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

await expect.poll(() => traceViewerInfo(vscode)).toMatchObject({
type: 'spawn',
traceFile: expect.stringMatching(/\.json$/),
Expand Down Expand Up @@ -135,7 +149,9 @@ test('should close trace viewer if test configs refreshed', async ({ activate })

await testController.refreshHandler(null);

await expect.poll(() => traceViewerInfo(vscode)).toBeUndefined();
await expect.poll(() => traceViewerInfo(vscode)).toMatchObject({
traceFile: undefined,
});
});

test('should open new trace viewer when another test config is selected', async ({ activate }) => {
Expand Down Expand Up @@ -171,7 +187,9 @@ test('should open new trace viewer when another test config is selected', async
// closes opened trace viewer
await selectConfig(vscode, 'playwright2.config.js');

await expect.poll(() => traceViewerInfo(vscode)).toBeUndefined();
await expect.poll(() => traceViewerInfo(vscode)).toMatchObject({
traceFile: undefined,
});

// opens trace viewer from selected test config
selectTestItem(testItems[0]);
Expand All @@ -186,7 +204,7 @@ test('should open new trace viewer when another test config is selected', async
expect(serverUrlPrefix2).not.toBe(serverUrlPrefix1);
});

test('should not open trace viewer when debugging', async ({ activate, createLatch }) => {
test('should not open trace viewer test item is selected while debugging', async ({ activate, createLatch }) => {
const latch = createLatch();

const { vscode, testController } = await activate({
Expand All @@ -198,13 +216,60 @@ test('should not open trace viewer when debugging', async ({ activate, createLat
});

await testController.expandTestItems(/test.spec/);
const testItems = testController.findTestItems(/pass/);
const [testItem] = testController.findTestItems(/pass/);

const debugPromise = testController.debug();
selectTestItem(testItems[0]);

await expect.poll(() => traceViewerInfo(vscode), { timeout: 20_000 }).toMatchObject({
// wait for test to start
await waitForTestItemStatus(testItem, 'started');

// select the test item and wait a bit more to give some time for trace viewer to eventually open
selectTestItem(testItem);

// ensure it didn't open
await expect.poll(() => traceViewerInfo(vscode)).toMatchObject({
traceFile: undefined,
});

latch.open();
await debugPromise;
});

test('should show empty state in trace viewer when test item is selected while debugging', async ({ activate, createLatch }) => {
const latch = createLatch();

const { vscode, testController } = await activate({
'playwright.config.js': `module.exports = { testDir: 'tests' }`,
'tests/test.spec.ts': `
import { test } from '@playwright/test';
test('should pass', async () => ${latch.blockingCode});
`,
});

await testController.expandTestItems(/test.spec/);
const [testItem] = testController.findTestItems(/pass/);

// run test without blocking, just to ensure trace viewer opens and displays a trace file
latch.open();
await testController.run();
await expect.poll(() => traceViewerInfo(vscode)).toMatchObject({
type: 'spawn',
traceFile: expect.stringMatching(/\.json$/),
traceFile: expect.stringMatching(/\.zip$/),
});

// debug the test
latch.close();
const debugPromise = testController.debug();

// wait for test to start
await waitForTestItemStatus(testItem, 'started');

// select the test item and wait a bit more to give some time for trace viewer to eventually update
selectTestItem(testItem);

// ensure it's not showing any trace file
await expect.poll(() => traceViewerInfo(vscode)).toMatchObject({
traceFile: undefined,
});

latch.open();
Expand Down

0 comments on commit 249d6cd

Please sign in to comment.