diff --git a/src/controller/testController.ts b/src/controller/testController.ts index a2c20713..7b734398 100644 --- a/src/controller/testController.ts +++ b/src/controller/testController.ts @@ -184,6 +184,10 @@ export const runTests: (request: TestRunRequest, option: IRunOption) => any = in return Promise.resolve(coverageProvider!.getCoverageDetails(fileCoverage.uri)); }; } + const testRunner: TestRunner | undefined = testRunnerService.getRunner(request.profile?.label, request.profile?.kind); + if (testRunner) { + enqueueTestMethods(testItems, run); + } try { await new Promise(async (resolve: () => void): Promise => { @@ -195,7 +199,6 @@ export const runTests: (request: TestRunRequest, option: IRunOption) => any = in disposables.forEach((d: Disposable) => d.dispose()); return resolve(); }); - enqueueTestMethods(testItems, run); // TODO: first group by project, then merge test methods. const queue: TestItem[][] = mergeTestMethods(testItems); for (const testsInQueue of queue) { @@ -219,7 +222,6 @@ export const runTests: (request: TestRunRequest, option: IRunOption) => any = in profile: request.profile, testConfig: await loadRunConfig(itemsPerProject, workspaceFolder), }; - const testRunner: TestRunner | undefined = testRunnerService.getRunner(request.profile?.label, request.profile?.kind); if (testRunner) { await executeWithTestRunner(option, testRunner, testContext, run, disposables); disposables.forEach((d: Disposable) => d.dispose()); diff --git a/src/controller/utils.ts b/src/controller/utils.ts index 946d4c97..535bbbf0 100644 --- a/src/controller/utils.ts +++ b/src/controller/utils.ts @@ -126,7 +126,7 @@ export function updateOrCreateTestItem(parent: TestItem, childData: IJavaTestIte function updateTestItem(testItem: TestItem, metaInfo: IJavaTestItem): void { testItem.range = asRange(metaInfo.range); - testItem.label = `${getCodiconLabel(metaInfo.testLevel)} ${metaInfo.label}`; + testItem.label = metaInfo.label; dataCache.set(testItem, { jdtHandler: metaInfo.jdtHandler, fullName: metaInfo.fullName, @@ -148,7 +148,7 @@ export function createTestItem(metaInfo: IJavaTestItem, parent?: TestItem): Test } const item: TestItem = testController.createTestItem( metaInfo.id, - `${getCodiconLabel(metaInfo.testLevel)} ${metaInfo.label}`.trim(), + metaInfo.label, metaInfo.uri ? Uri.parse(metaInfo.uri) : undefined, ); item.range = asRange(metaInfo.range); @@ -172,24 +172,6 @@ export function createTestItem(metaInfo: IJavaTestItem, parent?: TestItem): Test return item; } -/** - * Get codicon label based on the test level. - */ -function getCodiconLabel(testLevel: TestLevel): string { - switch (testLevel) { - case TestLevel.Project: - return '$(project)'; - case TestLevel.Package: - return '$(symbol-namespace)'; - case TestLevel.Class: - return '$(symbol-class)'; - case TestLevel.Method: - return '$(symbol-method)'; - default: - return ''; - } -} - let updateNodeForDocumentTimeout: NodeJS.Timer; /** * Update test item in a document with adaptive debounce enabled. diff --git a/src/runners/baseRunner/RunnerResultAnalyzer.ts b/src/runners/baseRunner/RunnerResultAnalyzer.ts index 1009b160..af6848e4 100644 --- a/src/runners/baseRunner/RunnerResultAnalyzer.ts +++ b/src/runners/baseRunner/RunnerResultAnalyzer.ts @@ -2,14 +2,10 @@ // Licensed under the MIT license. import { Location, MarkdownString, TestItem } from 'vscode'; -import { dataCache, ITestItemData } from '../../controller/testItemDataCache'; -import { IRunTestContext, TestLevel, TestResultState } from '../../java-test-runner.api'; +import { IRunTestContext } from '../../java-test-runner.api'; import { processStackTraceLine } from '../utils'; export abstract class RunnerResultAnalyzer { - // Track parent test item states to update them when all children complete - protected parentStates: Map = new Map(); - constructor(protected testContext: IRunTestContext) { } public abstract analyzeData(data: string): void; @@ -40,95 +36,4 @@ export abstract class RunnerResultAnalyzer { return stacktrace.includes(s); }); } - - /** - * Initialize parent state tracking for a test item. - * Counts how many method-level children are being tested. - */ - protected initializeParentState(item: TestItem, triggeredTestsMapping: Map): void { - const parent: TestItem | undefined = item.parent; - if (!parent) { - return; - } - - const parentData: ITestItemData | undefined = dataCache.get(parent); - if (!parentData || parentData.testLevel !== TestLevel.Class) { - return; - } - - if (!this.parentStates.has(parent)) { - // Count how many method-level children are being tested (only count triggered tests) - let childCount: number = 0; - parent.children.forEach((child: TestItem) => { - const childData: ITestItemData | undefined = dataCache.get(child); - if (childData?.testLevel === TestLevel.Method && triggeredTestsMapping.has(child.id)) { - childCount++; - } - }); - - this.parentStates.set(parent, { - started: false, - childrenTotal: childCount, - childrenCompleted: 0, - hasFailure: false, - }); - } - } - - /** - * Update parent test item when a child test starts. - * Marks the parent as "started" when the first child starts. - */ - protected updateParentOnChildStart(item: TestItem): void { - const parent: TestItem | undefined = item.parent; - if (!parent) { - return; - } - - const parentState: ParentItemState | undefined = this.parentStates.get(parent); - if (parentState && !parentState.started) { - parentState.started = true; - this.testContext.testRun.started(parent); - } - } - - /** - * Update parent test item when a child test completes. - * Marks the parent as "passed" or "failed" when all children complete. - */ - protected updateParentOnChildComplete(item: TestItem, childState: TestResultState): void { - const parent: TestItem | undefined = item.parent; - if (!parent) { - return; - } - - const parentState: ParentItemState | undefined = this.parentStates.get(parent); - if (!parentState) { - return; - } - - // Consider failed or errored tests as failures for the parent - if (childState === TestResultState.Failed || - childState === TestResultState.Errored) { - parentState.hasFailure = true; - } - - parentState.childrenCompleted++; - - // Check if all children have completed - if (parentState.childrenCompleted >= parentState.childrenTotal && parentState.childrenTotal > 0) { - if (parentState.hasFailure) { - this.testContext.testRun.failed(parent, []); - } else { - this.testContext.testRun.passed(parent); - } - } - } -} - -interface ParentItemState { - started: boolean; - childrenTotal: number; - childrenCompleted: number; - hasFailure: boolean; } diff --git a/src/runners/junitRunner/JUnitRunnerResultAnalyzer.ts b/src/runners/junitRunner/JUnitRunnerResultAnalyzer.ts index ac79b28a..252859d0 100644 --- a/src/runners/junitRunner/JUnitRunnerResultAnalyzer.ts +++ b/src/runners/junitRunner/JUnitRunnerResultAnalyzer.ts @@ -17,6 +17,8 @@ export class JUnitRunnerResultAnalyzer extends RunnerResultAnalyzer { private triggeredTestsMapping: Map = new Map(); private projectName: string; private incompleteTestSuite: ITestInfo[] = []; + private enqueuedTests: Set = new Set(); + private suiteItems: Set = new Set(); // tests may be run concurrently, so each item's current state needs to be remembered private currentStates: Map = new Map(); @@ -67,27 +69,30 @@ export class JUnitRunnerResultAnalyzer extends RunnerResultAnalyzer { if (data.startsWith(MessageId.TestTree)) { this.enlistToTestMapping(data.substring(MessageId.TestTree.length).trim()); } else if (data.startsWith(MessageId.TestStart)) { - const item: TestItem | undefined = this.getTestItem(data.substr(MessageId.TestStart.length)); - if (!item) { + const testInfo: ITestInfo | undefined = this.getTestInfo(data.substr(MessageId.TestStart.length)); + if (!testInfo?.testItem) { return; } - this.initializeParentState(item, this.triggeredTestsMapping); + const item: TestItem = testInfo.testItem; this.setCurrentState(item, TestResultState.Running, 0); this.setDurationAtStart(this.getCurrentState(item)); - setTestState(this.testContext.testRun, item, this.getCurrentState(item).resultState); - this.updateParentOnChildStart(item); + if (!testInfo.isSuite) { + setTestState(this.testContext.testRun, item, this.getCurrentState(item).resultState); + } } else if (data.startsWith(MessageId.TestEnd)) { - const item: TestItem | undefined = this.getTestItem(data.substr(MessageId.TestEnd.length)); - if (!item) { + const testInfo: ITestInfo | undefined = this.getTestInfo(data.substr(MessageId.TestEnd.length)); + if (!testInfo?.testItem) { return; } + const item: TestItem = testInfo.testItem; const currentState: CurrentItemState = this.getCurrentState(item); this.calcDurationAtEnd(currentState); this.determineResultStateAtEnd(data, currentState); - setTestState(this.testContext.testRun, item, currentState.resultState, undefined, currentState.duration); - const itemData: ITestItemData | undefined = dataCache.get(item); - if (itemData?.testLevel === TestLevel.Method) { - this.updateParentOnChildComplete(item, currentState.resultState); + const shouldReportSuite: boolean = currentState.resultState === TestResultState.Failed || + currentState.resultState === TestResultState.Errored || + (currentState.resultState === TestResultState.Skipped && testInfo.testCount === 0); + if (!testInfo.isSuite || shouldReportSuite) { + setTestState(this.testContext.testRun, item, currentState.resultState, undefined, currentState.duration); } } else if (data.startsWith(MessageId.TestFailed)) { const item: TestItem | undefined = this.getTestItem(data.substr(MessageId.TestFailed.length)); @@ -121,14 +126,18 @@ export class JUnitRunnerResultAnalyzer extends RunnerResultAnalyzer { return; } const currentResultState: TestResultState = this.getCurrentState(this.tracingItem).resultState; - if (this.assertionFailure) { - this.tryAppendMessage(this.tracingItem, this.assertionFailure, currentResultState); - } - if (this.traces?.value) { - this.tryAppendMessage(this.tracingItem, new TestMessage(this.traces), currentResultState); - } - if (currentResultState === TestResultState.Errored) { - setTestState(this.testContext.testRun, this.tracingItem, currentResultState); + const isSkippedSuite: boolean = currentResultState === TestResultState.Skipped && + this.suiteItems.has(this.tracingItem); + if (!isSkippedSuite) { + if (this.assertionFailure) { + this.tryAppendMessage(this.tracingItem, this.assertionFailure, currentResultState); + } + if (this.traces?.value) { + this.tryAppendMessage(this.tracingItem, new TestMessage(this.traces), currentResultState); + } + if (currentResultState === TestResultState.Errored) { + setTestState(this.testContext.testRun, this.tracingItem, currentResultState); + } } this.recordingType = RecordingType.None; } else if (data.startsWith(MessageId.ExpectStart)) { @@ -192,8 +201,12 @@ export class JUnitRunnerResultAnalyzer extends RunnerResultAnalyzer { } protected getTestItem(message: string): TestItem | undefined { + return this.getTestInfo(message)?.testItem; + } + + private getTestInfo(message: string): ITestInfo | undefined { const index: string = message.substring(0, message.indexOf(',')).trim(); - return this.testOutputMapping.get(index)?.testItem; + return this.testOutputMapping.get(index); } protected getTestId(message: string): string { @@ -406,6 +419,7 @@ export class JUnitRunnerResultAnalyzer extends RunnerResultAnalyzer { testId, testCount, testItem, + isSuite, }); } @@ -424,7 +438,15 @@ export class JUnitRunnerResultAnalyzer extends RunnerResultAnalyzer { testId, testCount, testItem, + isSuite, }); + if (isSuite && testItem) { + this.suiteItems.add(testItem); + } + if (!isSuite && testItem && !this.enqueuedTests.has(testItem)) { + this.enqueuedTests.add(testItem); + this.testContext.testRun.enqueued(testItem); + } } } @@ -526,6 +548,7 @@ interface ITestInfo { testId: string; testCount: number; testItem: TestItem | undefined; + isSuite: boolean; } enum RecordingType { diff --git a/src/runners/testngRunner/TestNGRunnerResultAnalyzer.ts b/src/runners/testngRunner/TestNGRunnerResultAnalyzer.ts index 0998fecb..de769da0 100644 --- a/src/runners/testngRunner/TestNGRunnerResultAnalyzer.ts +++ b/src/runners/testngRunner/TestNGRunnerResultAnalyzer.ts @@ -2,7 +2,7 @@ // Licensed under the MIT license. import { Location, MarkdownString, TestItem, TestMessage } from 'vscode'; -import { dataCache, ITestItemData } from '../../controller/testItemDataCache'; +import { dataCache } from '../../controller/testItemDataCache'; import { RunnerResultAnalyzer } from '../baseRunner/RunnerResultAnalyzer'; import { setTestState } from '../utils'; import { IRunTestContext, TestLevel, TestResultState } from '../../java-test-runner.api'; @@ -33,6 +33,7 @@ export class TestNGRunnerResultAnalyzer extends RunnerResultAnalyzer { } if (testLevel === TestLevel.Method) { this.triggeredTestsMapping.set(item.id, item); + this.testContext.testRun.enqueued(item); } else { item.children.forEach((child: TestItem) => { queue.push(child); @@ -73,10 +74,8 @@ export class TestNGRunnerResultAnalyzer extends RunnerResultAnalyzer { if (!item) { return; } - this.initializeParentState(item, this.triggeredTestsMapping); this.currentTestState = TestResultState.Running; this.testContext.testRun.started(item); - this.updateParentOnChildStart(item); } else if (outputData.name === TEST_FAIL) { const item: TestItem | undefined = this.getTestItem(id); if (!item) { @@ -114,10 +113,6 @@ export class TestNGRunnerResultAnalyzer extends RunnerResultAnalyzer { } const duration: number | undefined = this.parseDuration(attributes.duration); setTestState(this.testContext.testRun, item, this.currentTestState, undefined, duration); - const itemData: ITestItemData | undefined = dataCache.get(item); - if (itemData?.testLevel === TestLevel.Method) { - this.updateParentOnChildComplete(item, this.currentTestState); - } } } @@ -141,7 +136,9 @@ export class TestNGRunnerResultAnalyzer extends RunnerResultAnalyzer { message += `\n${attributes.trace}`; } const testMessage: TestMessage = new TestMessage(message); - for (const item of this.testContext.testItems) { + const testCases: Set = new Set(this.triggeredTestsMapping.values()); + const items: Iterable = testCases.size > 0 ? testCases : this.testContext.testItems; + for (const item of items) { this.testContext.testRun.errored(item, testMessage); } } diff --git a/test/suite/JUnitAnalyzer.test.ts b/test/suite/JUnitAnalyzer.test.ts index cae956be..cb6b7a95 100644 --- a/test/suite/JUnitAnalyzer.test.ts +++ b/test/suite/JUnitAnalyzer.test.ts @@ -407,6 +407,7 @@ org.opentest4j.AssertionFailedError: expected: <1> but was: <2> const testItem = generateTestItem(testController, 'junit@junit5.ParameterizedAnnotationTest#testMultiArguments(String, String, String)', TestKind.JUnit5, new Range(10, 0, 16, 0)); const testRunRequest = new TestRunRequest([testItem], []); const testRun = testController.createTestRun(testRunRequest); + const enqueuedSpy = sinon.spy(testRun, 'enqueued'); const startedSpy = sinon.spy(testRun, 'started'); const passedSpy = sinon.spy(testRun, 'passed'); const testRunnerOutput = `%TESTC 0 v2 @@ -434,6 +435,8 @@ org.opentest4j.AssertionFailedError: expected: <1> but was: <2> stub.returns(dummy); analyzer.analyzeData(testRunnerOutput); + assert.strictEqual(enqueuedSpy.calledWith(testItem), false); + sinon.assert.calledWith(enqueuedSpy, dummy); sinon.assert.calledWith(startedSpy, dummy); sinon.assert.calledWith(passedSpy, dummy); }); @@ -574,6 +577,7 @@ org.opentest4j.AssertionFailedError: expected: <1> but was: <2> const testRunRequest = new TestRunRequest([suiteItem], []); const testRun = testController.createTestRun(testRunRequest); + const enqueuedSpy = sinon.spy(testRun, 'enqueued'); const startedSpy = sinon.spy(testRun, 'started'); const passedSpy = sinon.spy(testRun, 'passed'); @@ -605,12 +609,59 @@ org.opentest4j.AssertionFailedError: expected: <1> but was: <2> const analyzer = new JUnitRunnerResultAnalyzer(runnerContext); analyzer.analyzeData(testRunnerOutput); - // Verify the suite item itself started and passed (the core regression in #1828) - sinon.assert.calledWith(startedSpy, suiteItem); - sinon.assert.calledWith(passedSpy, suiteItem, sinon.match.number); - // Verify the method-level child also started and passed + assert.strictEqual(enqueuedSpy.calledWith(suiteItem), false); + assert.strictEqual(enqueuedSpy.calledWith(classItem), false); + sinon.assert.calledWith(enqueuedSpy, methodItem); + assert.strictEqual(startedSpy.calledWith(suiteItem), false); + assert.strictEqual(passedSpy.calledWith(suiteItem), false); + assert.strictEqual(startedSpy.calledWith(classItem), false); + assert.strictEqual(passedSpy.calledWith(classItem), false); sinon.assert.calledWith(startedSpy, methodItem); sinon.assert.calledWith(passedSpy, methodItem, sinon.match.number); }); + test("reports an assumption-aborted suite when it has no test cases", () => { + const suiteItem = testController.createTestItem( + 'junit@junit5.AbortedSuite', + 'AbortedSuite', + Uri.file('/mock/test/AbortedSuite.java'), + ); + dataCache.set(suiteItem, { + jdtHandler: '', + fullName: 'junit5.AbortedSuite', + projectName: 'junit', + testLevel: TestLevel.Class, + testKind: TestKind.JUnit5, + }); + + const testRun = testController.createTestRun(new TestRunRequest([suiteItem], [])); + const enqueuedSpy = sinon.spy(testRun, 'enqueued'); + const startedSpy = sinon.spy(testRun, 'started'); + const skippedSpy = sinon.spy(testRun, 'skipped'); + const testRunnerOutput = `%TESTC 0 v2 +%TSTTREE1,junit5.AbortedSuite,true,0,false,-1,AbortedSuite,,[engine:junit-jupiter]/[class:junit5.AbortedSuite] +%TESTS 1,junit5.AbortedSuite +%FAILED 1,@AssumptionFailure: junit5.AbortedSuite +%TRACES +org.opentest4j.TestAbortedException: aborted +%TRACEE +%TESTE 1,@AssumptionFailure: junit5.AbortedSuite +%RUNTIME10`; + const runnerContext: IRunTestContext = { + isDebug: false, + kind: TestKind.JUnit5, + projectName: 'junit', + testItems: [suiteItem], + testRun, + workspaceFolder: workspace.workspaceFolders?.[0]!, + }; + + const analyzer = new JUnitRunnerResultAnalyzer(runnerContext); + analyzer.analyzeData(testRunnerOutput); + + assert.strictEqual(enqueuedSpy.calledWith(suiteItem), false); + assert.strictEqual(startedSpy.calledWith(suiteItem), false); + sinon.assert.calledOnceWithExactly(skippedSpy, suiteItem); + }); + }); diff --git a/test/suite/TestNGAnalyzer.test.ts b/test/suite/TestNGAnalyzer.test.ts index 511eebc6..7bd6b29b 100644 --- a/test/suite/TestNGAnalyzer.test.ts +++ b/test/suite/TestNGAnalyzer.test.ts @@ -7,8 +7,9 @@ import * as assert from 'assert'; import * as sinon from 'sinon'; import { TestController, TestMessage, TestRunRequest, tests, workspace } from 'vscode'; import { TestNGRunnerResultAnalyzer } from '../../src/runners/testngRunner/TestNGRunnerResultAnalyzer'; -import { IRunTestContext, TestKind } from '../../src/java-test-runner.api'; +import { IRunTestContext, TestKind, TestLevel } from '../../src/java-test-runner.api'; import { generateTestItem } from './utils'; +import { dataCache } from '../../src/controller/testItemDataCache'; // tslint:disable: only-arrow-functions suite('TestNG Runner Analyzer Tests', () => { @@ -70,4 +71,80 @@ suite('TestNG Runner Analyzer Tests', () => { sinon.assert.notCalled(appendOutputSpy); }); + + test('reports method results without assigning a result state to the class', () => { + const classItem = testController.createTestItem('testng@example.SampleTest', 'SampleTest'); + dataCache.set(classItem, { + jdtHandler: '', + fullName: 'example.SampleTest', + projectName: 'testng', + testLevel: TestLevel.Class, + testKind: TestKind.TestNG, + }); + const testItem = generateTestItem(testController, 'testng@example.SampleTest#test', TestKind.TestNG); + classItem.children.add(testItem); + + const testRun = testController.createTestRun(new TestRunRequest([classItem], [])); + const enqueuedSpy = sinon.spy(testRun, 'enqueued'); + const startedSpy = sinon.spy(testRun, 'started'); + const passedSpy = sinon.spy(testRun, 'passed'); + const runnerContext: IRunTestContext = { + isDebug: false, + kind: TestKind.TestNG, + projectName: 'testng', + testItems: [classItem], + testRun, + workspaceFolder: workspace.workspaceFolders?.[0]!, + }; + const analyzer = new TestNGRunnerResultAnalyzer(runnerContext); + + analyzer.processData(JSON.stringify({ + name: 'testStarted', + attributes: { name: 'example.SampleTest#test' }, + })); + analyzer.processData(JSON.stringify({ + name: 'testFinished', + attributes: { name: 'example.SampleTest#test', duration: '10' }, + })); + + sinon.assert.calledWith(enqueuedSpy, testItem); + assert.strictEqual(enqueuedSpy.calledWith(classItem), false); + sinon.assert.calledWith(startedSpy, testItem); + sinon.assert.calledWith(passedSpy, testItem, 10); + assert.strictEqual(startedSpy.calledWith(classItem), false); + assert.strictEqual(passedSpy.calledWith(classItem), false); + }); + + test('reports runner errors on method cases instead of their class', () => { + const classItem = testController.createTestItem('testng@example.SampleTest', 'SampleTest'); + dataCache.set(classItem, { + jdtHandler: '', + fullName: 'example.SampleTest', + projectName: 'testng', + testLevel: TestLevel.Class, + testKind: TestKind.TestNG, + }); + const testItem = generateTestItem(testController, 'testng@example.SampleTest#test', TestKind.TestNG); + classItem.children.add(testItem); + + const testRun = testController.createTestRun(new TestRunRequest([classItem], [])); + const erroredSpy = sinon.spy(testRun, 'errored'); + const runnerContext: IRunTestContext = { + isDebug: false, + kind: TestKind.TestNG, + projectName: 'testng', + testItems: [classItem], + testRun, + workspaceFolder: workspace.workspaceFolders?.[0]!, + }; + const analyzer = new TestNGRunnerResultAnalyzer(runnerContext); + + analyzer.processData(JSON.stringify({ + name: 'error', + attributes: { message: 'Failed to run TestNG tests' }, + })); + + sinon.assert.calledWith(erroredSpy, testItem, sinon.match.instanceOf(TestMessage)); + assert.strictEqual(erroredSpy.calledWith(classItem), false); + }); });