From d3c4323021e777947356c15af49d22fedaed511d Mon Sep 17 00:00:00 2001
From: Dmitry Gozman <dgozman@gmail.com>
Date: Fri, 18 Feb 2022 18:25:18 -0800
Subject: [PATCH] fix(test runner): improve error message for unexpected calls
 (#12240)

---
 packages/playwright-test/src/testType.ts | 38 +++++++++++++-----------
 tests/playwright-test/config.spec.ts     |  2 +-
 tests/playwright-test/hooks.spec.ts      |  2 +-
 tests/playwright-test/test-use.spec.ts   |  2 +-
 4 files changed, 23 insertions(+), 21 deletions(-)

diff --git a/packages/playwright-test/src/testType.ts b/packages/playwright-test/src/testType.ts
index 3b3f795815..720decb19f 100644
--- a/packages/playwright-test/src/testType.ts
+++ b/packages/playwright-test/src/testType.ts
@@ -63,12 +63,24 @@ export class TestTypeImpl {
     this.test = test;
   }
 
+  private _ensureCurrentSuite(location: Location, title: string): Suite {
+    const suite = currentlyLoadingFileSuite();
+    if (!suite) {
+      throw errorWithLocation(location, [
+        `Playwright Test did not expect ${title} to be called here.`,
+        `Most common reasons include:`,
+        `- You are calling ${title} in a configuration file.`,
+        `- You are calling ${title} in a file that is imported by the configuration file.`,
+        `- You have two different versions of @playwright/test. This usually happens`,
+        `  when one of the dependenices in your package.json depends on @playwright/test.`,
+      ].join('\n'));
+    }
+    return suite;
+  }
+
   private _createTest(type: 'default' | 'only' | 'skip' | 'fixme', location: Location, title: string, fn: Function) {
     throwIfRunningInsideJest();
-    const suite = currentlyLoadingFileSuite();
-    if (!suite)
-      throw errorWithLocation(location, `test() can only be called in a test file`);
-
+    const suite = this._ensureCurrentSuite(location, 'test()');
     const test = new TestCase('test', title, fn, nextOrdinalInFile(suite._requireFile), this, location);
     test._requireFile = suite._requireFile;
     suite._addTest(test);
@@ -81,10 +93,7 @@ export class TestTypeImpl {
 
   private _describe(type: 'default' | 'only' | 'serial' | 'serial.only' | 'parallel' | 'parallel.only', location: Location, title: string, fn: Function) {
     throwIfRunningInsideJest();
-    const suite = currentlyLoadingFileSuite();
-    if (!suite)
-      throw errorWithLocation(location, `describe() can only be called in a test file`);
-
+    const suite = this._ensureCurrentSuite(location, 'test.describe()');
     if (typeof title === 'function') {
       throw errorWithLocation(location, [
         'It looks like you are calling describe() without the title. Pass the title as a first argument:',
@@ -120,9 +129,7 @@ export class TestTypeImpl {
   }
 
   private _hook(name: 'beforeEach' | 'afterEach' | 'beforeAll' | 'afterAll', location: Location, fn: Function) {
-    const suite = currentlyLoadingFileSuite();
-    if (!suite)
-      throw errorWithLocation(location, `${name} hook can only be called in a test file`);
+    const suite = this._ensureCurrentSuite(location, `test.${name}()`);
     if (name === 'beforeAll' || name === 'afterAll') {
       const sameTypeCount = suite.hooks.filter(hook => hook._type === name).length;
       const suffix = sameTypeCount ? String(sameTypeCount) : '';
@@ -136,10 +143,7 @@ export class TestTypeImpl {
 
   private _configure(location: Location, options: { mode?: 'parallel' | 'serial' }) {
     throwIfRunningInsideJest();
-    const suite = currentlyLoadingFileSuite();
-    if (!suite)
-      throw errorWithLocation(location, `describe.configure() can only be called in a test file`);
-
+    const suite = this._ensureCurrentSuite(location, `test.describe.configure()`);
     if (!options.mode)
       return;
     if (suite._parallelMode !== 'default')
@@ -196,9 +200,7 @@ export class TestTypeImpl {
   }
 
   private _use(location: Location, fixtures: Fixtures) {
-    const suite = currentlyLoadingFileSuite();
-    if (!suite)
-      throw errorWithLocation(location, `test.use() can only be called in a test file and can only be nested in test.describe()`);
+    const suite = this._ensureCurrentSuite(location, `test.use()`);
     suite._use.push({ fixtures, location });
   }
 
diff --git a/tests/playwright-test/config.spec.ts b/tests/playwright-test/config.spec.ts
index 7ba0045950..58f26b914f 100644
--- a/tests/playwright-test/config.spec.ts
+++ b/tests/playwright-test/config.spec.ts
@@ -218,7 +218,7 @@ test('should throw when test() is called in config file', async ({ runInlineTest
       });
     `,
   });
-  expect(result.output).toContain('test() can only be called in a test file');
+  expect(result.output).toContain('Playwright Test did not expect test() to be called here');
 });
 
 test('should filter by project, case-insensitive', async ({ runInlineTest }) => {
diff --git a/tests/playwright-test/hooks.spec.ts b/tests/playwright-test/hooks.spec.ts
index df58600d48..b16429c99e 100644
--- a/tests/playwright-test/hooks.spec.ts
+++ b/tests/playwright-test/hooks.spec.ts
@@ -197,7 +197,7 @@ test('beforeAll from a helper file should throw', async ({ runInlineTest }) => {
     `,
   });
   expect(result.exitCode).toBe(1);
-  expect(result.output).toContain('beforeAll hook can only be called in a test file');
+  expect(result.output).toContain('Playwright Test did not expect test.beforeAll() to be called here');
 });
 
 test('beforeAll hooks are skipped when no tests in the suite are run', async ({ runInlineTest }) => {
diff --git a/tests/playwright-test/test-use.spec.ts b/tests/playwright-test/test-use.spec.ts
index 1892676167..9a058c3b7b 100644
--- a/tests/playwright-test/test-use.spec.ts
+++ b/tests/playwright-test/test-use.spec.ts
@@ -176,6 +176,6 @@ test('test.use() should throw if called from beforeAll ', async ({ runInlineTest
     `,
   });
   expect(result.exitCode).toBe(1);
-  expect(result.output).toContain('test.use() can only be called in a test file and can only be nested in test.describe()');
+  expect(result.output).toContain('Playwright Test did not expect test.use() to be called here');
 });