Skip to content

Commit bcf2dce

Browse files
bryanbcookbryanbcook
authored
Add support for "Test outcome settings" for duplicate test case references (#138)
* minor tsconfig change to address missing types in VSCode IDE for tests * changed logic and logging details for when duplicates are detected to rely on "sync outcomes across test suites" The "sync outcomes across test suites" setting of the test plan controls how to handle duplicates. When not enabled and the test plan contains duplicates, only the first reference of the test case will be used; warnings are logged and instructions are provided to enable the sync outcomes setting --------- Co-authored-by: bryan cook <3217452+bryancook@users.noreply.github.com>
1 parent ff1329a commit bcf2dce

9 files changed

Lines changed: 137 additions & 31 deletions

File tree

‎PublishTestPlanResultsV1/context/TestResultContext.ts‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { TestResultContextParameters } from "./TestResultContextParameters";
33
import { TestResultContextBuilder } from "./TestResultContextBuilder";
44
import { configAlias } from "./configAlias";
55
import { TestPoint2 } from "../services/AdoWrapper";
6+
import { ILogger, getLogger } from "../services/Logger";
67

78
export class TestResultContext {
89

@@ -11,7 +12,6 @@ export class TestResultContext {
1112
return await builder.build();
1213
}
1314

14-
// logger
1515
public readonly projectId: string;
1616
public readonly projectName: string;
1717
public readonly testPlan: TestPlan;
@@ -20,15 +20,17 @@ export class TestResultContext {
2020
private readonly testPoints: Map<number, TestPoint>;
2121
private readonly testCases: Set<string>;
2222
private readonly syncOutcomeAcrossSuites: boolean = false;
23+
private readonly logger: ILogger;
2324

24-
constructor(projectId: string, projectName: string, testPlan: TestPlan) {
25+
constructor(projectId: string, projectName: string, testPlan: TestPlan, logger : ILogger) {
2526
this.projectId = projectId;
2627
this.projectName = projectName;
2728
this.testPlan = testPlan;
2829
this.supportedTestConfigs = new Map<string, TestConfiguration>();
2930
this.testPoints = new Map<number, TestPoint>();
3031
this.testCases = new Set<string>();
3132
this.syncOutcomeAcrossSuites = this.testPlan.testOutcomeSettings?.syncOutcomeAcrossSuites ?? false;
33+
this.logger = logger;
3234
}
3335

3436
addConfig(config: TestConfiguration) {
@@ -61,13 +63,16 @@ export class TestResultContext {
6163
addTestPoint(point: TestPoint) {
6264

6365
// as performance optimization, we can exclude test points that are duplicate test case references
64-
// when the test plan has the "sync outcome across suites" option enabled
65-
if (this.syncOutcomeAcrossSuites) {
66+
// when the test plan doesn't have the "sync outcome across suites" option enabled.
67+
// duplicates when found will be logged as warnings
68+
if (!this.syncOutcomeAcrossSuites) {
69+
6670
// logic to handle duplicate test points
6771
const testCaseId = (point as TestPoint2).testCaseReference.id;
6872
if (testCaseId) {
6973
if (this.testCases.has(testCaseId.toString())) {
7074
// skip adding this test point as it's a duplicate reference to the same test case
75+
this.logger.warn(`Test Plan contains duplicates for test case: ${(point as TestPoint2).testCaseReference.name} (id: ${testCaseId}). This test case will be matched to the first test point reference found, and duplicate references will be ignored.`);
7176
return;
7277
}
7378
// add the test case ID to the set to track it

‎PublishTestPlanResultsV1/context/TestResultContextBuilder.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ export class TestResultContextBuilder {
4444

4545
this.log.info(`Using Test Plan: ${testPlan.name}`);
4646

47-
let ctx = new TestResultContext(projectId, (this.projectName as string), testPlan);
47+
let ctx = new TestResultContext(projectId, (this.projectName as string), testPlan, this.log);
4848
let configs = await this.getTestConfigurations();
4949
if (configs) {
5050
this.log.debug(`${configs.length} test configurations available.`);
@@ -65,10 +65,11 @@ export class TestResultContextBuilder {
6565
ctx.addTestPoints(points);
6666
this.log.info(`Available Test Points: ${points.length}`);
6767
if (ctx.hasSyncTestOutcomeEnabled()) {
68-
this.log.info(`Test Plan has 'sync outcomes across suites' enabled. Duplicate test case references will be removed.`);
68+
this.log.info(`Test Plan has 'sync outcomes across suites' enabled.`);
69+
} else {
6970
let uniqueTestLength = ctx.getTestPoints().length;
7071
if (points.length != uniqueTestLength) {
71-
this.log.info(`Removed ${points.length - uniqueTestLength} duplicate test case reference(s).`);
72+
this.log.warn(`Removed ${points.length - uniqueTestLength} duplicate test case reference(s). Consider enabling 'sync outcomes across suites' on the Test Plan settings to allow duplicate test case references.`);
7273
}
7374
}
7475

‎PublishTestPlanResultsV1/processing/TestResultProcessor.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ export class TestResultProcessor {
129129
const allMatch = matches.map( p => p as TestPoint2).every( p => p.testCaseReference.id === firstTestCaseId);
130130

131131
if (allMatch && matches.length > 1) {
132-
this.logger.warn(`Test Plan contains duplicates for test case: ${firstTestCaseId}.`);
132+
this.logger.debug(`Found multiple test points for test case: ${firstTestCaseId}.`);
133133
}
134134

135135
return allMatch;

‎PublishTestPlanResultsV1/test/TestResultContextBuilder.specs.ts‎

Lines changed: 101 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import * as testUtil from './testUtil';
44
import { newTestConfig, newTestPlan, newTestPoint, newTestCase } from './testUtil';
55
import { TestPlan, TestConfiguration, TestPoint, SuiteTestCase } from 'azure-devops-node-api/interfaces/TestInterfaces';
66
import { AdoWrapper, TestPoint2 } from '../services/AdoWrapper';
7-
import { ILogger, NullLogger } from '../services/Logger';
7+
import { ILogger, Logger } from '../services/Logger';
88
import { TestResultContextBuilder } from '../context/TestResultContextBuilder';
99
import { configAlias } from '../context/configAlias';
1010

@@ -16,7 +16,7 @@ describe("TestResultContextBuilder", () => {
1616
var subject : TestResultContextBuilder;
1717

1818
beforeEach(() => {
19-
logger = new NullLogger();
19+
logger = sinon.createStubInstance(Logger);
2020
ado = sinon.createStubInstance(AdoWrapper);
2121
subject = new TestResultContextBuilder(logger, ado);
2222
});
@@ -216,7 +216,7 @@ describe("TestResultContextBuilder", () => {
216216
expect((points[1] as TestPoint2).testCaseReference.name).to.eq("Test Case 4");
217217
});
218218

219-
it("Should recognize when test config filter is not a valid config", async () =>{
219+
it("Should recognize when test config filter is not a valid config", async () => {
220220
// arrange
221221
setupTestConfigurations([
222222
newTestConfig(1, "Config 1"),
@@ -257,47 +257,134 @@ describe("TestResultContextBuilder", () => {
257257
});
258258
});
259259

260-
context("Sync outcomes across test suites enabled", () => {
261-
it("Should not include duplicate test points that reference the same test case", async () => {
262-
// arrange
260+
context("Sync outcomes across test suites enabled (duplicates present)", () => {
261+
262+
beforeEach(() => {
263263
setupTestPlans([
264264
newTestPlan(1, "ValidPlan", undefined, true /*sync outcomes across suites*/)
265265
]);
266266
setupTestPoints([
267267
newTestPoint(1, "Test Case 1", "1", "150"),
268-
newTestPoint(2, "Test Case 1 - duplicate", "1", "150"),
268+
newTestPoint(2, "Test Case 1", "1", "150"), // duplicate
269269
newTestPoint(3, "Test Case 2", "1", "151"),
270270
]);
271+
});
272+
273+
it("Should include duplicate test points that reference the same test case", async () => {
274+
// arrange
275+
271276

272277
// act
273278
var result = await subject.build();
274279

275280
// assert
276281
let points = result.getTestPoints();
277-
expect(points.length).to.eq(2);
282+
expect(points.length).to.eq(3);
278283
});
279-
});
280284

281-
context("Sync outcomes across test suites not set", () => {
282-
it("Should not duplicate test points that reference the same test case", async () => {
285+
it("Should log that 'sync outcomes across suites' is enabled", async () => {
283286
// arrange
287+
// act
288+
var result = await subject.build();
289+
290+
// assert
291+
const infoSpy = logger.info as sinon.SinonSpy;
292+
sinon.assert.called(infoSpy);
293+
const loggedMessages = testUtil.getLoggedMessages(infoSpy);
294+
expect(loggedMessages.some(msg => msg.includes("Test Plan has 'sync outcomes across suites' enabled"))).to.eq(true);
295+
});
296+
});
297+
298+
context("Sync outcomes across test suites not set (duplicates present)", () => {
299+
300+
beforeEach(() => {
301+
// arrange setup a test plan that contains duplicates
284302
setupTestPlans([
285303
newTestPlan(1, "ValidPlan")
286304
]);
287305
setupTestPoints([
288306
newTestPoint(1, "Test Case 1", "1", "150"),
289-
newTestPoint(2, "Test Case 1 - duplicate", "1", "150"),
307+
newTestPoint(2, "Test Case 1", "1", "150"), // duplicate
290308
newTestPoint(3, "Test Case 2", "1", "151"),
291309
]);
310+
});
311+
312+
it("Should not include duplicate test points that reference the same test case", async () => {
313+
// arrange
292314

293315
// act
294316
var result = await subject.build();
295317

296318
// assert
297319
let points = result.getTestPoints();
298-
expect(points.length).to.eq(3);
320+
expect(points.length).to.eq(2);
321+
});
322+
323+
it("Should log a warning that the test plan contains duplicates", async () => {
324+
// arrange
325+
326+
// act
327+
var result = await subject.build();
328+
329+
// assert
330+
const warnSpy = logger.warn as sinon.SinonSpy;
331+
sinon.assert.called(warnSpy);
332+
const loggedMessages = testUtil.getLoggedMessages(warnSpy);
333+
expect(loggedMessages.some(msg => msg.includes("Test Plan contains duplicates for test case: Test Case 1 (id: 150)"))).to.eq(true);
334+
});
335+
336+
it("Should log the number of duplicates that were removed", async () => {
337+
// arrange
338+
339+
// act
340+
var result = await subject.build();
341+
342+
// assert
343+
const warnSpy = logger.warn as sinon.SinonSpy;
344+
sinon.assert.called(warnSpy);
345+
const loggedMessages = testUtil.getLoggedMessages(warnSpy);
346+
expect(loggedMessages.some(msg => msg.includes("Removed 1 duplicate test case reference(s)."))).to.eq(true);
347+
});
348+
349+
it("Should inform user about 'sync outcomes across suites' should be enabled", async () => {
350+
// arrange
351+
352+
// act
353+
var result = await subject.build();
354+
355+
// assert
356+
const warnSpy = logger.warn as sinon.SinonSpy;
357+
sinon.assert.called(warnSpy);
358+
const loggedMessages = testUtil.getLoggedMessages(warnSpy);
359+
expect(loggedMessages.some(msg => msg.includes("Consider enabling 'sync outcomes across suites' on the Test Plan settings to allow duplicate test case references."))).to.eq(true);
299360
});
300361
});
362+
363+
context("Sync outcomes across test suites not set (no duplicates)", () => {
364+
365+
beforeEach(() => {
366+
// arrange setup a test plan that does not contain duplicates
367+
setupTestPlans([
368+
newTestPlan(1, "ValidPlan") // sync outcomes across suites not enabled
369+
]);
370+
setupTestPoints([
371+
newTestPoint(1, "Test Case 1", "1", "150"),
372+
newTestPoint(2, "Test Case 2", "1", "151"),
373+
]);
374+
});
375+
376+
it("Should not log a warning about duplicates", async () => {
377+
// arrange
378+
379+
// act
380+
var result = await subject.build();
381+
382+
// assert
383+
const warnSpy = logger.warn as sinon.SinonSpy;
384+
sinon.assert.notCalled(warnSpy);
385+
})
386+
387+
});
301388
})
302389

303390
context("Load Test Case Meta", () => {
@@ -377,6 +464,6 @@ describe("TestResultContextBuilder", () => {
377464
function setupTestCases(testCases : any[]) {
378465
ado.getTestCasesForSuite.returns(new Promise((resolve) => {
379466
resolve(testCases);
380-
}))
467+
}));
381468
}
382469
});

‎PublishTestPlanResultsV1/test/TestResultProcessor.specs.ts‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -130,17 +130,19 @@ describe('TestResultProcessor', () => {
130130
]
131131
})
132132

133-
it("Should log a warning that the test plan contains duplicates", async () => {
133+
it("Should log a debug message that the test plan contains duplicates", async () => {
134134
// arrange
135135
subject.logger = sinon.createStubInstance(Logger);
136136

137137
// act
138138
var result = await subject.process(testresults);
139139

140140
// assert
141-
sinon.assert.called(subject.logger.warn as sinon.SinonSpy);
142-
var loggedMessage = (subject.logger.warn as sinon.SinonSpy).getCall(0).args[0] as string;
143-
expect(loggedMessage).to.contain("Test Plan contains duplicates for test case: 1234");
141+
142+
const debugSpy = subject.logger.debug as sinon.SinonSpy;
143+
sinon.assert.called(debugSpy);
144+
const loggedMessages = testUtil.getLoggedMessages(debugSpy);
145+
expect(loggedMessages.some(msg => msg.includes("Found multiple test points for test case: 1234"))).to.eq(true);
144146
});
145147

146148
it ("Should map test result to multiple test points", async () => {

‎PublishTestPlanResultsV1/test/TestResultProcessorFactory.specs.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,16 @@ import { TestResultContext } from '../context/TestResultContext';
55
import { TestResultProcessorParameters } from '../processing/TestResultProcessorParameters';
66

77
import * as subject from '../processing/TestResultProcessorFactory';
8+
import { ILogger, NullLogger } from '../services/Logger';
89

910
describe('TestResultProcessorFactory', () => {
1011

1112
var ctx : TestResultContext;
1213
var parameters : TestResultProcessorParameters;
13-
14+
1415
beforeEach( () => {
1516
let dummyPlan = <TestPlan>{id:1};
16-
ctx = new TestResultContext("projectId","projectName", dummyPlan);
17+
ctx = new TestResultContext("projectId","projectName", dummyPlan, new NullLogger());
1718
});
1819

1920
it('Should configure all matchers when set to auto', async () => {

‎PublishTestPlanResultsV1/test/testUtil.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { ShallowReference, TestPlan, TestConfiguration, WorkItemReference, Suite
44
import { TestPoint2 } from '../services/AdoWrapper';
55
import { TestFrameworkResult } from '../framework/TestFrameworkResult';
66
import FeatureFlags from '../services/FeatureFlags';
7+
import Sinon = require('sinon');
78

89
export function setSystemVariable(name: string, val: string) {
910
let key: string = im._getVariableKey(name);
@@ -112,7 +113,13 @@ export function newTestPlan(id : number = 0, name? : string, endDate? : Date, sy
112113
};
113114
}
114115

116+
var testCaseIdCounter : number = 0;
117+
115118
export function newTestPoint(id : number = 0, name : string = "Test 1", configId : string = "0", testCaseId : string = "0" ) {
119+
if (testCaseId === "0") {
120+
// use a unique value if test case id isn't specified
121+
testCaseId = (++testCaseIdCounter).toString();
122+
}
116123
return <TestPoint2>{
117124
id: id,
118125
testCaseReference: <WorkItemReference>{ /*TestPoint has testCase, but it should be testCaseReference*/
@@ -165,3 +172,6 @@ export function newTestFrameworkResult(name : string = "Test1", outcome : string
165172
return result;
166173
}
167174

175+
export function getLoggedMessages( spy : Sinon.SinonSpy<any[], any> ) : string[] {
176+
return Array.from({ length: spy.callCount }, (_, i) => spy.getCall(i).args[0] as string);
177+
}

‎PublishTestPlanResultsV1/tsconfig.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131
// "paths": {}, /* Specify a set of entries that re-map imports to additional lookup locations. */
3232
// "rootDirs": [], /* Allow multiple folders to be treated as one when resolving modules. */
3333
// "typeRoots": [], /* Specify multiple folders that act like `./node_modules/@types`. */
34-
// "types": [], /* Specify type package names to be included without being referenced in a source file. */
34+
"types": ["node", "mocha", "chai"], /* Specify type package names to be included without being referenced in a source file. */
3535
// "allowUmdGlobalAccess": true, /* Allow accessing UMD globals from modules. */
3636
// "resolveJsonModule": true, /* Enable importing .json files */
3737
// "noResolve": true, /* Disallow `import`s, `require`s or `<reference>`s from expanding the number of files TypeScript should add to a project. */

‎devops/pipelines/marketplace-extension/regression-test.yml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ parameters:
4343
buildId: 2436
4444
expectedResults:
4545
- outcome: Passed
46-
count: 2
46+
count: 1
4747
- name: TestDuplicates_wTestOutcomeSettingEnabled
4848
format: xunit
4949
testResultsFile: '$(Build.SourcesDirectory)/examples/dotnet/xUnitResults/TestResults-adotestplan-prj.xml'
@@ -52,7 +52,7 @@ parameters:
5252
buildId: 2436
5353
expectedResults:
5454
- outcome: Passed
55-
count: 1
55+
count: 2
5656

5757
steps:
5858
- pwsh: |

0 commit comments

Comments
 (0)