first draft to deduplicate diff test messages

fladdimir · fladdimir · commit 48512db7e5be · 2023-01-29T15:58:18.000+01:00
diff --git a/src/runners/baseRunner/RunnerResultAnalyzer.ts b/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 {
diff --git a/src/runners/junitRunner/JUnitRunnerResultAnalyzer.ts b/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 {
diff --git a/src/runners/testngRunner/TestNGRunnerResultAnalyzer.ts b/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);
diff --git a/test/suite/JUnitAnalyzer.test.ts b/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);
     });
@@ -338,7 +338,8 @@ org.junit.ComparisonFailure: expected:<hello
     });
 
     test("test diff is not duplicated when failing assertion is extracted", () => {
-        const testItem = generateTestItem(testController, 'junit@junit5.TestWithExtractedEqualityAssertion#test', TestKind.JUnit5, new Range(2, 0, 2, 0), undefined, 'TestWithExtractedEqualityAssertion.java');
+        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');
@@ -356,16 +357,9 @@ org.junit.ComparisonFailure: expected:<hello
 %ACTUALE
 %TRACES
 org.opentest4j.AssertionFailedError: expected: <1> but was: <2>
-    at org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:55)
-    at org.junit.jupiter.api.AssertionUtils.failNotEqual(AssertionUtils.java:62)
-    at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:150)
-    at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:145)
-    at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:510)
     at junit5.TestWithExtractedEqualityAssertion.extracted2(TestWithExtractedEqualityAssertion.java:18)
     at junit5.TestWithExtractedEqualityAssertion.extracted1(TestWithExtractedEqualityAssertion.java:14)
-    at junit5.TestWithExtractedEqualityAssertion.test(TestWithExtractedEqualityAssertion.java:10)
-    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
-    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:210)
+    at junit5.TestWithExtractedEqualityAssertion.test(TestWithExtractedEqualityAssertion.java:11)
 %TRACEE
 %TESTE  3,test(junit5.TestWithExtractedEqualityAssertion)
 %RUNTIME55`;
@@ -384,16 +378,12 @@ org.opentest4j.AssertionFailedError: expected: <1> but was: <2>
         sinon.assert.calledWith(startedSpy, testItem);
         sinon.assert.calledWith(failedSpy, testItem, sinon.match.any);
 
-        const testMessage = failedSpy.getCall(0).args[1] as TestMessage;
+        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, 2);
-
-        const testMessages = failedSpy.getCalls().map(call => call.args[1] as TestMessage).filter(v => v.actualOutput || v.expectedOutput);
-
-        assert.strictEqual(testMessages.length, 1, "not more than one diff-message");
-        // assert.strictEqual(testMessages[0].location?.range.start, 10); // todo
+        assert.strictEqual(testMessage.location?.range.start.line, 10); // =11 - 1, (most precise info we get from the stack trace)
     });
 
-
 });
diff --git a/test/test-projects/junit/src/test/java/junit5/TestWithExtractedEqualityAssertion.java b/test/test-projects/junit/src/test/java/junit5/TestWithExtractedEqualityAssertion.java
@@ -7,9 +7,18 @@ class TestWithExtractedEqualityAssertion {
 
     @Test
     void test() {
+    }
+
+    @Test
+    void test1() {
         extracted1();
     }
 
+    @Test
+    void test2() {
+        extracted2();
+    }
+
     private void extracted1() {
         extracted2();
     }

Original file line number	Diff line number	Diff line change
`@@ -2,9 +2,8 @@`
`2`	`2`	`// Licensed under the MIT license.`
`3`	`3`
`4`	`4`	`import * as path from 'path';`
`5`		`-import { Location, MarkdownString, Range, TestItem, TestMessage } from 'vscode';`
	`5`	`+import { Location, MarkdownString, Range, TestItem } from 'vscode';`
`6`	`6`	`import { IRunTestContext } from '../../types';`
`7`		`-import { setTestState, TestResultState } from '../utils';`
`8`	`7`
`9`	`8`	`export abstract class RunnerResultAnalyzer {`
`10`	`9`	`constructor(protected testContext: IRunTestContext) { }`
`@@ -21,7 +20,7 @@ export abstract class RunnerResultAnalyzer {`
`21`	`20`	`return [];`
`22`	`21`	`}`
`23`	`22`
`24`		`- protected processStackTrace(data: string, traces: MarkdownString, assertionFailure: TestMessage \| undefined, currentItem: TestItem \| undefined, projectName: string): void {`
	`23`	`+ protected processStackTrace(data: string, traces: MarkdownString, currentItem: TestItem \| undefined, projectName: string): void {`
`25`	`24`	`const traceRegExp: RegExp = /(\s?at\s+)([\w$\\.]+\/)?((?:[\w$]+\.)+[<\w$>]+)\((.*)\)/;`
`26`	`25`	`const traceResults: RegExpExecArray \| null = traceRegExp.exec(data);`
`27`	`26`	`if (traceResults) {`
`@@ -55,10 +54,6 @@ export abstract class RunnerResultAnalyzer {`
`55`	`54`	`this.testMessageLocation = new Location(currentItem.uri, new Range(currentItem.range.start.line, 0, currentItem.range.start.line, 0));`
`56`	`55`	`}`
`57`	`56`	`}`
`58`		`- if (assertionFailure) {`
`59`		`- assertionFailure.location = this.testMessageLocation;`
`60`		`- setTestState(this.testContext.testRun, currentItem, TestResultState.Failed, assertionFailure);`
`61`		`- }`
`62`	`57`	`}`
`63`	`58`	`}`
`64`	`59`	`} else {`