Enhance the forbidOnly mode message to guide the user towards the configuration option (#23146)

Hi, I am putting this PR out as a feeler to see if there's interested in
improving this error message, but the copy is by no means final and I am
open to improvement suggestions.

My intention here is to:
- Explain what a "focused item" is - that we're talking about a test and
it being focused is most likely down it using `only`
  
Are there other types of "items"? Are there other ways to make them
focused other than `only`?

- Explain why we're even in focused mode and how to control it

The default scaffolded Playwright config file includes a forbidMode
expression driven by whether `CI=1` is set.
I ran into this when trying to reproduce a CI issue locally so I had it
set and unknowingly entered focus only mode.
I wasn't aware this mode was a thing because I was using the default
configuration from `npm init` and did not familiarize myself with all
the options in it.

Is there a way to tell if we're in a TypeScript or JavaScript project in
this function? I would use that to display the configuration file name
with the right extension.

---------

Signed-off-by: Tomáš Hübelbauer <tomas@hubelbauer.net>
This commit is contained in:
Tomáš Hübelbauer 2023-05-25 22:32:49 +02:00 committed by GitHub
parent 1e64fa4f7c
commit e550df060e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 26 additions and 7 deletions

View File

@ -150,14 +150,24 @@ These are integration tests, making sure public API methods and events work as e
- To run all tests: - To run all tests:
```bash ```bash
npx playwright install
npm run test npm run test
``` ```
Be sure to run `npm run build` or let `npm run watch` run before you re-run the
tests after making your changes to check them.
- To run all tests in Chromium - To run all tests in Chromium
```bash ```bash
npm run ctest # also `ftest` for firefox and `wtest` for WebKit npm run ctest # also `ftest` for firefox and `wtest` for WebKit
``` ```
- To run the Playwright test runner tests
```bash
npm run ttest
npm run ttest -- --grep "specific test"
```
- To run a specific test, substitute `it` with `it.only`, or use the `--grep 'My test'` CLI parameter: - To run a specific test, substitute `it` with `it.only`, or use the `--grep 'My test'` CLI parameter:
```js ```js

View File

@ -148,8 +148,10 @@ export async function createRootSuite(testRun: TestRun, errors: TestError[], sho
// Complain about only. // Complain about only.
if (config.config.forbidOnly) { if (config.config.forbidOnly) {
const onlyTestsAndSuites = rootSuite._getOnlyItems(); const onlyTestsAndSuites = rootSuite._getOnlyItems();
if (onlyTestsAndSuites.length > 0) if (onlyTestsAndSuites.length > 0) {
errors.push(...createForbidOnlyErrors(onlyTestsAndSuites)); const configFilePath = config.config.configFile ? path.relative(config.config.rootDir, config.config.configFile) : undefined;
errors.push(...createForbidOnlyErrors(onlyTestsAndSuites, config.configCLIOverrides.forbidOnly, configFilePath));
}
} }
// Filter only for top-level projects. // Filter only for top-level projects.
@ -220,13 +222,15 @@ async function createProjectSuite(fileSuites: Suite[], project: FullProjectInter
return projectSuite; return projectSuite;
} }
function createForbidOnlyErrors(onlyTestsAndSuites: (TestCase | Suite)[]): TestError[] { function createForbidOnlyErrors(onlyTestsAndSuites: (TestCase | Suite)[], forbidOnlyCLIFlag: boolean | undefined, configFilePath: string | undefined): TestError[] {
const errors: TestError[] = []; const errors: TestError[] = [];
for (const testOrSuite of onlyTestsAndSuites) { for (const testOrSuite of onlyTestsAndSuites) {
// Skip root and file. // Skip root and file.
const title = testOrSuite.titlePath().slice(2).join(' '); const title = testOrSuite.titlePath().slice(2).join(' ');
const configFilePathName = configFilePath ? `'${configFilePath}'` : 'the Playwright configuration file';
const forbidOnlySource = forbidOnlyCLIFlag ? `'--forbid-only' CLI flag` : `'forbidOnly' option in ${configFilePathName}`;
const error: TestError = { const error: TestError = {
message: `Error: focused item found in the --forbid-only mode: "${title}"`, message: `Error: item focused with '.only' is not allowed due to the ${forbidOnlySource}: "${title}"`,
location: testOrSuite.location!, location: testOrSuite.location!,
}; };
errors.push(error); errors.push(error);

View File

@ -547,7 +547,7 @@ test('should report forbid-only error to reporter', async ({ runInlineTest }) =>
}, { 'reporter': '', 'forbid-only': true }); }, { 'reporter': '', 'forbid-only': true });
expect(result.exitCode).toBe(1); expect(result.exitCode).toBe(1);
expect(result.output).toContain(`%%got error: Error: focused item found in the --forbid-only mode`); expect(result.output).toContain(`%%got error: Error: item focused with '.only' is not allowed due to the '--forbid-only' CLI flag: \"a.test.ts pass\"`);
}); });
test('should report no-tests error to reporter', async ({ runInlineTest }) => { test('should report no-tests error to reporter', async ({ runInlineTest }) => {

View File

@ -59,13 +59,18 @@ test('it should not allow multiple tests with the same name in multiple files',
test('it should not allow a focused test when forbid-only is used', async ({ runInlineTest }) => { test('it should not allow a focused test when forbid-only is used', async ({ runInlineTest }) => {
const result = await runInlineTest({ const result = await runInlineTest({
'playwright.config.ts': `
module.exports = {
forbidOnly: true,
};
`,
'tests/focused-test.spec.js': ` 'tests/focused-test.spec.js': `
import { test, expect } from '@playwright/test'; import { test, expect } from '@playwright/test';
test.only('i-am-focused', async () => {}); test.only('i-am-focused', async () => {});
` `
}, { 'forbid-only': true }); });
expect(result.exitCode).toBe(1); expect(result.exitCode).toBe(1);
expect(result.output).toContain('Error: focused item found in the --forbid-only mode'); expect(result.output).toContain(`Error: item focused with '.only' is not allowed due to the 'forbidOnly' option in 'playwright.config.ts': \"tests${path.sep}focused-test.spec.js i-am-focused\"`);
expect(result.output).toContain(`test.only('i-am-focused'`); expect(result.output).toContain(`test.only('i-am-focused'`);
expect(result.output).toContain(`tests${path.sep}focused-test.spec.js:3`); expect(result.output).toContain(`tests${path.sep}focused-test.spec.js:3`);
}); });