From 03ebe213232ca308c001519fdce9cc83a3f22d0c Mon Sep 17 00:00:00 2001
From: Dmitry Gozman <dgozman@gmail.com>
Date: Thu, 22 Jul 2021 12:34:37 -0700
Subject: [PATCH] faet(test runner): help when describe() is misused (#7753)

---
 src/test/testType.ts                | 28 +++++++++++++++++++---------
 src/test/util.ts                    |  4 ++++
 tests/playwright-test/basic.spec.ts | 15 +++++++++++++++
 3 files changed, 38 insertions(+), 9 deletions(-)

diff --git a/src/test/testType.ts b/src/test/testType.ts
index d715f2a142..9370a48dd7 100644
--- a/src/test/testType.ts
+++ b/src/test/testType.ts
@@ -19,6 +19,7 @@ import { currentlyLoadingFileSuite, currentTestInfo, setCurrentlyLoadingFileSuit
 import { TestCase, Suite } from './test';
 import { wrapFunctionWithLocation } from './transform';
 import { Fixtures, FixturesWithLocation, Location, TestType } from './types';
+import { errorWithLocation } from './util';
 
 const countByFile = new Map<string, number>();
 
@@ -47,7 +48,7 @@ export class TestTypeImpl {
     test.fixme = wrapFunctionWithLocation(this._modifier.bind(this, 'fixme'));
     test.fail = wrapFunctionWithLocation(this._modifier.bind(this, 'fail'));
     test.slow = wrapFunctionWithLocation(this._modifier.bind(this, 'slow'));
-    test.setTimeout = this._setTimeout.bind(this);
+    test.setTimeout = wrapFunctionWithLocation(this._setTimeout.bind(this));
     test.use = wrapFunctionWithLocation(this._use.bind(this));
     test.extend = wrapFunctionWithLocation(this._extend.bind(this));
     test.declare = wrapFunctionWithLocation(this._declare.bind(this));
@@ -58,7 +59,7 @@ export class TestTypeImpl {
     throwIfRunningInsideJest();
     const suite = currentlyLoadingFileSuite();
     if (!suite)
-      throw new Error(`test() can only be called in a test file`);
+      throw errorWithLocation(location, `test() can only be called in a test file`);
 
     const ordinalInFile = countByFile.get(suite._requireFile) || 0;
     countByFile.set(suite._requireFile, ordinalInFile + 1);
@@ -75,7 +76,16 @@ export class TestTypeImpl {
     throwIfRunningInsideJest();
     const suite = currentlyLoadingFileSuite();
     if (!suite)
-      throw new Error(`describe() can only be called in a test file`);
+      throw errorWithLocation(location, `describe() can only be called in a test file`);
+
+    if (typeof title === 'function') {
+      throw errorWithLocation(location, [
+        'It looks like you are calling describe() without the title. Pass the title as a first argument:',
+        `test.describe('my test group', () => {`,
+        `  // Declare tests here`,
+        `});`,
+      ].join('\n'));
+    }
 
     const child = new Suite(title);
     child._requireFile = suite._requireFile;
@@ -93,7 +103,7 @@ export class TestTypeImpl {
   private _hook(name: 'beforeEach' | 'afterEach' | 'beforeAll' | 'afterAll', location: Location, fn: Function) {
     const suite = currentlyLoadingFileSuite();
     if (!suite)
-      throw new Error(`${name} hook can only be called in a test file`);
+      throw errorWithLocation(location, `${name} hook can only be called in a test file`);
     suite._hooks.push({ type: name, fn, location });
   }
 
@@ -113,13 +123,13 @@ export class TestTypeImpl {
 
     const testInfo = currentTestInfo();
     if (!testInfo)
-      throw new Error(`test.${type}() can only be called inside test, describe block or fixture`);
+      throw errorWithLocation(location, `test.${type}() can only be called inside test, describe block or fixture`);
     if (typeof modifierArgs[0] === 'function')
-      throw new Error(`test.${type}() with a function can only be called inside describe block`);
+      throw errorWithLocation(location, `test.${type}() with a function can only be called inside describe block`);
     testInfo[type](...modifierArgs as [any, any]);
   }
 
-  private _setTimeout(timeout: number) {
+  private _setTimeout(location: Location, timeout: number) {
     const suite = currentlyLoadingFileSuite();
     if (suite) {
       suite._timeout = timeout;
@@ -128,14 +138,14 @@ export class TestTypeImpl {
 
     const testInfo = currentTestInfo();
     if (!testInfo)
-      throw new Error(`test.setTimeout() can only be called from a test file`);
+      throw errorWithLocation(location, `test.setTimeout() can only be called from a test file`);
     testInfo.setTimeout(timeout);
   }
 
   private _use(location: Location, fixtures: Fixtures) {
     const suite = currentlyLoadingFileSuite();
     if (!suite)
-      throw new Error(`test.use() can only be called in a test file`);
+      throw errorWithLocation(location, `test.use() can only be called in a test file`);
     suite._fixtureOverrides = { ...suite._fixtureOverrides, ...fixtures };
   }
 
diff --git a/src/test/util.ts b/src/test/util.ts
index 7410236b71..e85ed09920 100644
--- a/src/test/util.ts
+++ b/src/test/util.ts
@@ -162,3 +162,7 @@ export function formatLocation(location: Location) {
 export function errorWithFile(file: string, message: string) {
   return new Error(`${relativeFilePath(file)}: ${message}`);
 }
+
+export function errorWithLocation(location: Location, message: string) {
+  return new Error(`${formatLocation(location)}: ${message}`);
+}
diff --git a/tests/playwright-test/basic.spec.ts b/tests/playwright-test/basic.spec.ts
index a982d2543b..ea7c131a8d 100644
--- a/tests/playwright-test/basic.spec.ts
+++ b/tests/playwright-test/basic.spec.ts
@@ -351,3 +351,18 @@ test('should work with test helper', async ({ runInlineTest }) => {
     '%%suite2.test2',
   ]);
 });
+
+test('should help with describe() misuse', async ({ runInlineTest }) => {
+  const result = await runInlineTest({
+    'a.spec.js': `
+      pwt.test.describe(() => {});
+    `,
+  });
+  expect(result.exitCode).toBe(1);
+  expect(result.output).toContain([
+    'Error: a.spec.js:5:16: It looks like you are calling describe() without the title. Pass the title as a first argument:',
+    `test.describe('my test group', () => {`,
+    `  // Declare tests here`,
+    `});`,
+  ].join('\n'));
+});