From 01191b8eca9fc2ca706fb6ab34213ff448f1eedd Mon Sep 17 00:00:00 2001 From: ConnectDotz Date: Mon, 12 Aug 2024 17:11:42 -0400 Subject: [PATCH] address test suite error didn't get propagated correctly issue (#1165) --- src/test-provider/test-item-data.ts | 29 +++++- tests/test-provider/test-item-data.test.ts | 106 ++++++++++++++++++++- 2 files changed, 132 insertions(+), 3 deletions(-) diff --git a/src/test-provider/test-item-data.ts b/src/test-provider/test-item-data.ts index aa71c4c64..67c8a7e82 100644 --- a/src/test-provider/test-item-data.ts +++ b/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(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): void => { this.createChildItems(parsedRoot); if (run) { diff --git a/tests/test-provider/test-item-data.test.ts b/tests/test-provider/test-item-data.test.ts index 2b6c03114..d3ef2df92 100644 --- a/tests/test-provider/test-item-data.test.ts +++ b/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); + }); + }); });