fix: Deduplicate diff test messages (#1521)

src/runners/baseRunner/RunnerResultAnalyzer.ts

@@ -2,9 +2,8 @@
 // Licensed under the MIT license.
 
 import * as path from 'path';
-import { Location, MarkdownString, Range, TestItem, TestMessage } from 'vscode';
+import { Location, MarkdownString, Range, TestItem } from 'vscode';
 import { IRunTestContext } from '../../types';
-import { setTestState, TestResultState } from '../utils';
 
 export abstract class RunnerResultAnalyzer {
     constructor(protected testContext: IRunTestContext) { }
@@ -21,7 +20,7 @@ export abstract class RunnerResultAnalyzer {
         return [];
     }
 
-    protected processStackTrace(data: string, traces: MarkdownString, assertionFailure: TestMessage | undefined, currentItem: TestItem | undefined, projectName: string): void {
+    protected processStackTrace(data: string, traces: MarkdownString, currentItem: TestItem | undefined, projectName: string): void {
         const traceRegExp: RegExp = /(\s?at\s+)([\w$\\.]+\/)?((?:[\w$]+\.)+[<\w$>]+)\((.*)\)/;
         const traceResults: RegExpExecArray | null = traceRegExp.exec(data);
         if (traceResults) {
@@ -55,10 +54,6 @@ export abstract class RunnerResultAnalyzer {
                             this.testMessageLocation = new Location(currentItem.uri, new Range(currentItem.range.start.line, 0, currentItem.range.start.line, 0));
                         }
                     }
-                    if (assertionFailure) {
-                        assertionFailure.location = this.testMessageLocation;
-                        setTestState(this.testContext.testRun, currentItem, TestResultState.Failed, assertionFailure);
-                    }
                 }
             }
         } else {

src/runners/junitRunner/JUnitRunnerResultAnalyzer.ts

@@ -107,13 +107,17 @@ export class JUnitRunnerResultAnalyzer extends RunnerResultAnalyzer {
             if (!this.tracingItem) {
                 return;
             }
-            const testMessage: TestMessage = new TestMessage(this.traces);
             const currentResultState: TestResultState = this.getCurrentState(this.tracingItem).resultState;
-            this.tryAppendMessage(this.tracingItem, testMessage, currentResultState);
-            this.recordingType = RecordingType.None;
+            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)) {
             this.recordingType = RecordingType.ExpectMessage;
         } else if (data.startsWith(MessageId.ExpectEnd)) {
@@ -139,8 +143,7 @@ export class JUnitRunnerResultAnalyzer extends RunnerResultAnalyzer {
                     this.assertionFailure = TestMessage.diff(`Expected [${assertionResults[1]}] but was [${assertionResults[2]}]`, assertionResults[1], assertionResults[2]);
                 }
             }
-
-            this.processStackTrace(data, this.traces, this.assertionFailure, this.tracingItem, this.projectName);
+            this.processStackTrace(data, this.traces, this.tracingItem, this.projectName);
         }
     }
 
@@ -218,6 +221,7 @@ export class JUnitRunnerResultAnalyzer extends RunnerResultAnalyzer {
         this.expectString = '';
         this.actualString = '';
         this.recordingType = RecordingType.None;
+        this.testMessageLocation = undefined;
     }
 
     protected getStacktraceFilter(): string[] {
@@ -335,7 +339,6 @@ export class JUnitRunnerResultAnalyzer extends RunnerResultAnalyzer {
     private async tryAppendMessage(item: TestItem, testMessage: TestMessage, testState: TestResultState): Promise<void> {
         if (this.testMessageLocation) {
             testMessage.location = this.testMessageLocation;
-            this.testMessageLocation = undefined;
         } else if (item.uri && item.range) {
             testMessage.location = new Location(item.uri, item.range);
         } else {

src/runners/testngRunner/TestNGRunnerResultAnalyzer.ts

@@ -45,7 +45,7 @@ export class TestNGRunnerResultAnalyzer extends RunnerResultAnalyzer {
         // tslint:disable-next-line: no-conditional-assignment
         while ((match = this.regex.exec(data)) !== null) {
             try {
-               this.processData(match[1]);
+                this.processData(match[1]);
             } catch (error) {
                 this.testContext.testRun.appendOutput(`[ERROR] Failed to parse output data: ${match[1]}\n`);
             }
@@ -80,7 +80,7 @@ export class TestNGRunnerResultAnalyzer extends RunnerResultAnalyzer {
                 markdownTrace.supportHtml = true;
 
                 for (const line of outputData.attributes.trace.split(/\r?\n/)) {
-                    this.processStackTrace(line, markdownTrace, undefined, this.currentItem, this.projectName);
+                    this.processStackTrace(line, markdownTrace, this.currentItem, this.projectName);
                 }
 
                 const testMessage: TestMessage = new TestMessage(markdownTrace);

test/suite/JUnitAnalyzer.test.ts

@@ -5,9 +5,9 @@
 
 import * as assert from 'assert';
 import * as sinon from 'sinon';
-import { IRunTestContext, TestKind } from '../../src/types';
 import { MarkdownString, Range, TestController, TestMessage, TestRunRequest, tests, workspace } from 'vscode';
 import { JUnitRunnerResultAnalyzer } from '../../src/runners/junitRunner/JUnitRunnerResultAnalyzer';
+import { IRunTestContext, TestKind } from '../../src/types';
 import { generateTestItem } from './utils';
 
 // tslint:disable: only-arrow-functions
@@ -146,7 +146,7 @@ java.lang.AssertionError: expected:<1> but was:<2>
         analyzer.analyzeData(testRunnerOutput);
 
         sinon.assert.calledWith(failedSpy, testItem, sinon.match.any, sinon.match.number);
-        const testMessage = failedSpy.getCall(0).args[1] as TestMessage;
+        const testMessage = failedSpy.getCall(1).args[1] as TestMessage;
         const stringLiteral = (testMessage.message as MarkdownString).value;
         assert.ok(stringLiteral.split('<br/>').length === 3);
     });
@@ -336,4 +336,54 @@ org.junit.ComparisonFailure: expected:<hello
 
         assert.strictEqual(testItem.description, '');
     });
+
+    test("test diff is not duplicated when failing assertion is extracted", () => {
+        const range = new Range(9, 0, 11, 0);
+        const testItem = generateTestItem(testController, 'junit@junit5.TestWithExtractedEqualityAssertion#test', TestKind.JUnit5, range, undefined, 'TestWithExtractedEqualityAssertion.java');
+        const testRunRequest = new TestRunRequest([testItem], []);
+        const testRun = testController.createTestRun(testRunRequest);
+        const startedSpy = sinon.spy(testRun, 'started');
+        const failedSpy = sinon.spy(testRun, 'failed');
+        const testRunnerOutput = `%TESTC  1 v2
+%TSTTREE2,junit5.TestWithExtractedEqualityAssertion,true,1,false,1,TestWithExtractedEqualityAssertion,,[engine:junit-jupiter]/[class:junit5.TestWithExtractedEqualityAssertion]
+%TSTTREE3,test(junit5.TestWithExtractedEqualityAssertion),false,1,false,2,test(),,[engine:junit-jupiter]/[class:junit5.TestWithExtractedEqualityAssertion]/[method:test()]
+%TESTS  3,test(junit5.TestWithExtractedEqualityAssertion)
+%FAILED 3,test(junit5.TestWithExtractedEqualityAssertion)
+%EXPECTS
+1
+%EXPECTE
+%ACTUALS
+2
+%ACTUALE
+%TRACES
+org.opentest4j.AssertionFailedError: expected: <1> but was: <2>
+    at junit5.TestWithExtractedEqualityAssertion.extracted2(TestWithExtractedEqualityAssertion.java:18)
+    at junit5.TestWithExtractedEqualityAssertion.extracted1(TestWithExtractedEqualityAssertion.java:14)
+    at junit5.TestWithExtractedEqualityAssertion.test(TestWithExtractedEqualityAssertion.java:11)
+%TRACEE
+%TESTE  3,test(junit5.TestWithExtractedEqualityAssertion)
+%RUNTIME55`;
+        const runnerContext: IRunTestContext = {
+            isDebug: false,
+            kind: TestKind.JUnit5,
+            projectName: 'junit',
+            testItems: [testItem],
+            testRun: testRun,
+            workspaceFolder: workspace.workspaceFolders?.[0]!,
+        };
+
+        const analyzer = new JUnitRunnerResultAnalyzer(runnerContext);
+        analyzer.analyzeData(testRunnerOutput);
+
+        sinon.assert.calledWith(startedSpy, testItem);
+        sinon.assert.calledWith(failedSpy, testItem, sinon.match.any);
+
+        const diffTestMessages = failedSpy.getCalls().map(call => call.args[1] as TestMessage).filter(v => v.actualOutput || v.expectedOutput);
+        assert.strictEqual(diffTestMessages.length, 1, "not more than one diff-message");
+        const testMessage = diffTestMessages[0];
+        assert.strictEqual(testMessage.expectedOutput, '1');
+        assert.strictEqual(testMessage.actualOutput, '2');
+        assert.strictEqual(testMessage.location?.range.start.line, 10); // =11 - 1, (most precise info we get from the stack trace)
+    });
+
 });

test/test-projects/junit/src/test/java/junit5/TestWithExtractedEqualityAssertion.java

@@ -0,0 +1,30 @@
+package junit5;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+class TestWithExtractedEqualityAssertion {
+
+    @Test
+    void test() {
+    }
+
+    @Test
+    void test1() {
+        extracted1();
+    }
+
+    @Test
+    void test2() {
+        extracted2();
+    }
+
+    private void extracted1() {
+        extracted2();
+    }
+
+    private void extracted2() {
+        Assertions.assertEquals(1, 2);
+    }
+
+}