cherry-pick(release-1.14): make sure we always teardown all fixtures (#…

…8171) PR #8158 SHA d32d50a Co-authored-by: Dmitry Gozman <[email protected]>
microsoft · Aug 12, 2021 · cba2f05 · cba2f05
1 parent 4bdc06c
commit cba2f05
Show file tree

Hide file tree

Showing 2 changed files with 63 additions and 2 deletions.
diff --git a/src/test/fixtures.ts b/src/test/fixtures.ts
@@ -223,14 +223,23 @@ export class FixtureRunner {
   }
 
   async teardownScope(scope: FixtureScope) {
+    let error: Error | undefined;
     // Teardown fixtures in the reverse order.
     const fixtures = Array.from(this.instanceForId.values()).reverse();
     for (const fixture of fixtures) {
-      if (fixture.registration.scope === scope)
-        await fixture.teardown();
+      if (fixture.registration.scope === scope) {
+        try {
+          await fixture.teardown();
+        } catch (e) {
+          if (error === undefined)
+            error = e;
+        }
+      }
     }
     if (scope === 'test')
       this.testScopeClean = true;
+    if (error !== undefined)
+      throw error;
   }
 
   async resolveParametersAndRunHookOrTest(fn: Function, workerInfo: WorkerInfo, testInfo: TestInfo | undefined) {

diff --git a/tests/playwright-test/hooks.spec.ts b/tests/playwright-test/hooks.spec.ts
@@ -349,3 +349,55 @@ test('beforeAll failure should prevent the test, but not afterAll', async ({ run
     '%%afterAll',
   ]);
 });
+
+test('fixture error should not prevent afterAll', async ({ runInlineTest }) => {
+  const result = await runInlineTest({
+    'a.test.js': `
+      const test = pwt.test.extend({
+        foo: async ({}, use) => {
+          await use('foo');
+          throw new Error('bad fixture');
+        },
+      });
+      test('good test', ({ foo }) => {
+        console.log('\\n%%test');
+      });
+      test.afterAll(() => {
+        console.log('\\n%%afterAll');
+      });
+    `,
+  });
+  expect(result.exitCode).toBe(1);
+  expect(result.failed).toBe(1);
+  expect(result.output).toContain('bad fixture');
+  expect(result.output.split('\n').filter(line => line.startsWith('%%'))).toEqual([
+    '%%test',
+    '%%afterAll',
+  ]);
+});
+
+test('afterEach failure should not prevent afterAll', async ({ runInlineTest }) => {
+  const result = await runInlineTest({
+    'a.test.js': `
+      const { test } = pwt;
+      test('good test', ({ }) => {
+        console.log('\\n%%test');
+      });
+      test.afterEach(() => {
+        console.log('\\n%%afterEach');
+        throw new Error('bad afterEach');
+      })
+      test.afterAll(() => {
+        console.log('\\n%%afterAll');
+      });
+    `,
+  });
+  expect(result.exitCode).toBe(1);
+  expect(result.failed).toBe(1);
+  expect(result.output).toContain('bad afterEach');
+  expect(result.output.split('\n').filter(line => line.startsWith('%%'))).toEqual([
+    '%%test',
+    '%%afterEach',
+    '%%afterAll',
+  ]);
+});