address test suite error didn't get propagated correctly issue (#1165)

Author: connectdotz

src/test-provider/test-item-data.ts

@@ -160,6 +160,7 @@ export class WorkspaceRoot extends TestItemDataBase {
       coverage: options?.profile?.kind === vscode.TestRunProfileKind.Coverage,
     };
   }
+
   discoverTest(run: JestTestRun): void {
     const testList = this.context.ext.testResultProvider.getTestList();
     // only trigger update when testList is not empty because it's possible test-list is not available yet,
@@ -294,7 +295,9 @@ export class WorkspaceRoot extends TestItemDataBase {
         if (event.files.length === 0) {
           run.write(`No tests were run.`, `new-line`);
         } else {
-          event.files.forEach((f) => this.addTestFile(f, (testRoot) => testRoot.discoverTest(run)));
+          event.files.forEach((f) =>
+            this.addTestFile(f, (testRoot) => testRoot.onAssertionUpdate(run))
+          );
         }
         run.end({ process: event.process, delay: 1000, reason: 'assertions-updated' });
         this.preventZombieProcess(event.process);
@@ -540,6 +543,15 @@ abstract class TestResultData extends TestItemDataBase {
     super(context, name);
   }
 
+  // TODO - we should use "unknown" state when vscode supports it
+  // (see https://github.com/microsoft/vscode/issues/206139).
+  resetChildrenState(run: JestTestRun, result: TestSuiteResult): void {
+    this.forEachChild((child) => {
+      child.updateItemState(run, result);
+      child.resetChildrenState(run, result);
+    });
+  }
+
   updateItemState(
     run: JestTestRun,
     result?: TestSuiteResult | TestAssertionStatus,
@@ -625,6 +637,7 @@ abstract class TestResultData extends TestItemDataBase {
   }
 
   forEachChild(onTestData: (child: TestData) => void): void {
+    console.log(`${this.item.id} has ${this.item.children.size} children`);
     this.item.children.forEach((childItem) => {
       const child = this.context.getData<TestData>(childItem);
       if (child) {
@@ -655,6 +668,20 @@ export class TestDocumentRoot extends TestResultData {
     return item;
   }
 
+  onAssertionUpdate(run: JestTestRun): void {
+    // handle special case when the test results contains no assertions
+    // (usually due to syntax error), we will need to mark the item's children
+    // explicitly failed instead of just removing them. Due to vscode's current
+    // implementation - removing the test item will not reset the item's status,
+    // when they get added back again later (by the parsed source nodes), they
+    // will inherit the previous status, which might not be correct.
+    const suiteResult = this.context.ext.testResultProvider.getTestSuiteResult(this.item.id);
+    if (suiteResult && isContainerEmpty(suiteResult.assertionContainer)) {
+      this.resetChildrenState(run, suiteResult);
+    }
+    this.discoverTest(run);
+  }
+
   discoverTest = (run?: JestTestRun, parsedRoot?: ContainerNode<ItBlock>): void => {
     this.createChildItems(parsedRoot);
     if (run) {

tests/test-provider/test-item-data.test.ts

@@ -156,13 +156,22 @@ describe('test-item-data', () => {
 
   const createAllTestItems = () => {
     const wsRoot = new WorkspaceRoot(context);
-    const folder = new FolderData(context, 'dir', wsRoot.item);
-    const uri: any = { fsPath: 'whatever' };
+    const folder = new FolderData(context, 'tests', wsRoot.item);
+    const uri: any = { fsPath: '/ws-1/tests/a.test.ts' };
     const doc = new TestDocumentRoot(context, uri, folder.item);
     const node: any = { fullName: 'a test', attrs: {}, data: {} };
     const testItem = new TestData(context, uri, node, doc.item);
     return { wsRoot, folder, doc, testItem };
   };
+  // like createAllTestItems but with added a describe block
+  const createTestDataTree = () => {
+    const { wsRoot, folder, doc, testItem } = createAllTestItems();
+    const node1: any = { fullName: 'describe', attrs: {}, data: {} };
+    const desc = new TestData(context, doc.uri, node1, doc.item);
+    const node2: any = { fullName: 'describe test 2', attrs: {}, data: {} };
+    const test2 = new TestData(context, doc.uri, node2, desc.item);
+    return { wsRoot, folder, doc, testItem, desc, test2 };
+  };
 
   beforeEach(() => {
     controllerMock = mockController();
@@ -1834,4 +1843,97 @@ describe('test-item-data', () => {
       );
     });
   });
+  describe('onAssertionUpdate', () => {
+    let folder, doc, desc, testItem, test2;
+    beforeEach(() => {
+      ({ folder, doc, desc, testItem, test2 } = createTestDataTree());
+    });
+    describe('when test suite failed without assertions', () => {
+      it("all child items should inherit the test suite's status", () => {
+        // address https://github.com/jest-community/vscode-jest/issues/1098
+        const file = '/ws-1/tests/a.test.ts';
+        // context.ext.testResultProvider.getTestList.mockReturnValueOnce([]);
+        const runMode = new RunMode({ type: 'watch' });
+        context.ext.settings = { runMode };
+
+        // test suite failed and there is no assertions
+        const testSuiteResult: any = {
+          status: 'KnownFail',
+          message: 'test file failed',
+        };
+        context.ext.testResultProvider.getTestSuiteResult.mockReturnValue(testSuiteResult);
+
+        // doc has 2 children before the test suite event
+        expect(doc.item.children.size).toBe(2);
+
+        // triggers testSuiteChanged event listener
+        contextCreateTestRunSpy.mockClear();
+        mockedJestTestRun.mockClear();
+
+        // mock a non-watch process that is still running
+        const process = {
+          id: 'whatever',
+          request: { type: 'watch-tests' },
+        };
+        context.ext.testResultProvider.events.testSuiteChanged.event.mock.calls[0][0]({
+          type: 'assertions-updated',
+          process,
+          files: [file],
+        });
+
+        // all child items should have been removed
+        expect(doc.item.children.size).toBe(0);
+
+        // checking item status update...
+        const run = mockedJestTestRun.mock.results[0].value;
+
+        // all child items should be updated regardless before being removed
+        expect(run.failed).toHaveBeenCalledTimes(4);
+        [doc.item, testItem.item, desc.item, test2.item].forEach((item) => {
+          expect(run.failed).toHaveBeenCalledWith(item, expect.anything());
+        });
+
+        expect(run.end).toHaveBeenCalledTimes(1);
+      });
+    });
+    it('when no test suite result found, the doc and its child items should be removed without any status update', () => {
+      const file = '/ws-1/tests/a.test.ts';
+      const runMode = new RunMode({ type: 'watch' });
+      context.ext.settings = { runMode };
+
+      // test suite failed and there is no assertions
+      context.ext.testResultProvider.getTestSuiteResult.mockReturnValue(undefined);
+
+      // doc has 2 children before the test suite event
+      expect(folder.item.children.size).toBe(1);
+      expect(doc.item.children.size).toBe(2);
+
+      // triggers testSuiteChanged event listener
+      contextCreateTestRunSpy.mockClear();
+      mockedJestTestRun.mockClear();
+
+      // mock a non-watch process that is still running
+      const process = {
+        id: 'whatever',
+        request: { type: 'watch-tests' },
+      };
+      context.ext.testResultProvider.events.testSuiteChanged.event.mock.calls[0][0]({
+        type: 'assertions-updated',
+        process,
+        files: [file],
+      });
+
+      // all doc's child items should have been removed but the doc itself would remain
+      expect(folder.item.children.size).toBe(1);
+      expect(doc.item.children.size).toBe(0);
+
+      // checking item status update...
+      const run = mockedJestTestRun.mock.results[0].value;
+
+      // no update should occur
+      expect(run.failed).not.toHaveBeenCalled();
+
+      expect(run.end).toHaveBeenCalledTimes(1);
+    });
+  });
 });