feat(trace-viewer): Added test results to status line #35742
Conversation
This comment has been minimized.
This comment has been minimized.
playwright/tests/playwright-test/{test}.tsx
packages/trace-viewer/src/ui/uiModeView.css
+ 100 ~ + 103
|
|
|
@pengooseDev Sorry, wasn't ready for a review just yet. Didn't expect the tests to run in draft mode, and just playing around with it. Wanted to ask you how to get HMR to work? Not sure I understood the docs. |
Just testPW_HMR=1 npm run watch # wait to build end
PW_HMR=1 npm run ctest -- --uietcPW_HMR=1 npx playwright show-trace
PW_HMR=1 npx playwright codegen
PW_HMR=1 npx playwright show-report |
|
@pengooseDev I'm sorry, I'm not familiar with that syntax. Is this for the terminal? I tried that and it didn't get recognized |
|
@andreaslarssen It is for the terminal :) Docs |
This comment has been minimized.
This comment has been minimized.
|
@microsoft-github-policy-service agree |
|
@pengooseDev What's the script for running the tests locally? |
This comment has been minimized.
This comment has been minimized.
|
All the information is included in the Docs and the script field of the |
Test results for "tests 1"10 failed 4 flaky39077 passed, 803 skipped Merge workflow run. |
|
@andreaslarssen we need to update the tests in Additionally, since a new feature has been added, we should add regression tests to cover it. Example of a currently failing test: test('should show running progress', async ({ runUITest }) => {
const { page } = await runUITest({
'a.test.ts': `
import { test, expect } from '@playwright/test';
test('test 1', async () => {});
test('test 2', async () => new Promise(() => {}));
test('test 3', async () => {});
test('test 4', async () => {});
`,
});
await page.getByTitle('Run all').click();
await expect(page.getByTestId('status-line')).toHaveText('Running 1/4 passed (25%)');
await page.getByTitle('Stop').click();
await expect(page.getByTestId('status-line')).toHaveText('1/4 passed (25%)');
await page.getByTitle('Reload').click();
await expect(page.getByTestId('status-line')).toBeHidden();
});test('should queue watches', async ({ runUITest, writeFiles, createLatch }) => {
const latch = createLatch();
const { page } = await runUITest({
'a.test.ts': `import { test } from '@playwright/test'; test('test', () => {});`,
'b.test.ts': `import { test } from '@playwright/test'; test('test', async () => {
${latch.blockingCode}
});`,
'c.test.ts': `import { test } from '@playwright/test'; test('test', () => {});`,
'd.test.ts': `import { test } from '@playwright/test'; test('test', () => {});`,
});
await expect.poll(dumpTestTree(page)).toBe(`
▼ ◯ a.test.ts
◯ test
▼ ◯ b.test.ts
◯ test
▼ ◯ c.test.ts
◯ test
▼ ◯ d.test.ts
◯ test
`);
await page.getByTitle('Watch all').click();
await page.getByTitle('Run all').click();
await expect(page.getByTestId('status-line')).toHaveText('Running 1/4 passed (25%)');
await writeFiles({
'a.test.ts': `import { test } from '@playwright/test'; test('test', () => {});`,
'b.test.ts': `import { test } from '@playwright/test'; test('test', () => {});`,
'c.test.ts': `import { test } from '@playwright/test'; test('test', () => {});`,
});
// Now watches should not kick in.
await new Promise(f => setTimeout(f, 1000));
await expect(page.getByTestId('status-line')).toHaveText('Running 1/4 passed (25%)');
// Allow test to finish and new watch to kick in.
latch.open();
await expect(page.getByTestId('status-line')).toHaveText('3/3 passed (100%)');
}); |
|
@pengooseDev Yeah, I'm working on this in-between other stuff, so it's going to take some time. Took a quick look at one of the failing tests yesterday, but couldn't figure out (just by looking at code) why it would fail. I struggle to run the test files locally, so that affects my effectivity. |
|
What do you think about matching the text color with the test icon color, as shown in the image below?
|
|
Yeah, looks good, will fix. |
1. Icon sizenit: It looks like the size of the passed icon is different from the failed and skipped icons.
2.
You can run local tests using the following terminal commands: Run all core testsnpm ci
npm run build
npm run ttestRun a specific testnpm run ttest -- --grep "{name}"Example: npm run ttest -- --grep "should queue watches"👍 |
|
When writing commit messages, please make sure to follow the convention! Example: git commit -m "test: add regression tests" |
Not sure I fully understand the convention. The examples seems to indicate that every commit is a complete task? I also checked the commits of other PR's, and they didn't seem to follow the convention either, so I thought I might be the PR and not the commit messages that should follow the convention. |
|
@pengooseDev Also: I changed the UI a bit so that you don't have to wait until the tests have finished before seeing the result of the tests. I don't see why you would need to wait to after all the tests have run to see that. Also, it seems like there was a duplicate test-id (both the progress line and the tests result line had test id 'status-line'), and this change fixed that. The only downside of this, I guess, is that this change is (probably) why the last tests are failing. So before fixing them, I just wanted to check if you thought the UI changes I did was ok, or if I'm going to be asked to change it back after finishing the PR. |
This adds test results to the status line, showing how many tests passed, failed and was skipped.
It also changes the UI logic in that it show the same UI while running tests and after.
Fixes #34444