Skip to content

Commit 458a2d5

Browse files
authored
Fix Maven task rebuild when using jacoco for code coverage (#21472)
* remove duplicate maven run for verify phase * add FF * update condition * fix condition * add test for RemoveDuplicateMavenRun FF in enabled state * update test name
1 parent 7d3243a commit 458a2d5

File tree

5 files changed

+204
-21
lines changed

5 files changed

+204
-21
lines changed

Tasks/MavenV4/Tests/L0.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -335,6 +335,33 @@ describe("Maven L0 Suite", function () {
335335
assert(testRunner.succeeded, 'task should have succeeded');
336336
});
337337

338+
it('run maven with code coverage enabled and restore original pom.xml after (with feature flag enabled for combined maven run)', async function() {
339+
this.timeout(parseInt(process.env.TASK_TEST_TIMEOUT) || 20000);
340+
const testPath = path.join(__dirname, 'L0RestoreOriginalPomXmlCombinedRun.js');
341+
const testRunner = new MockTestRunner(testPath);
342+
343+
await testRunner.runAsync();
344+
345+
assert(testRunner.ran('/home/bin/maven/bin/mvn -version'), 'it should have run mvn -version');
346+
assert(testRunner.ran('/home/bin/maven/bin/mvn -f pom.xml help:effective-pom'), 'it should have generated effective pom');
347+
assert(testRunner.ran('/home/bin/maven/bin/mvn -f pom.xml clean verify'), 'it should have run mvn -f pom.xml clean verify');
348+
349+
const readOriginalPomXmlLogIndex = testRunner.stdout.indexOf('Reading original pom.xml');
350+
assert(readOriginalPomXmlLogIndex !== -1, 'should have read original pom.xml');
351+
const wroteModifiedPomXmlLogIndex = testRunner.stdout.indexOf('Writing modified pom.xml contents');
352+
assert(wroteModifiedPomXmlLogIndex !== -1, 'should have written modified pom.xml contents');
353+
const wroteOriginalPomXmlLogIndex = testRunner.stdout.indexOf('Writing original pom.xml contents');
354+
assert(wroteOriginalPomXmlLogIndex !== -1, 'should have written original pom.xml contents');
355+
356+
assert(readOriginalPomXmlLogIndex < wroteModifiedPomXmlLogIndex, 'it shouldn\'t have saved pom.xml before writing modified pom.xml contents');
357+
assert(wroteModifiedPomXmlLogIndex < wroteOriginalPomXmlLogIndex, 'it shouldn\'t have restored original pom.xml before writing modified pom.xml contents');
358+
359+
assert(testRunner.invokedToolCount === 3, 'should have run maven exactly 3 times (with feature flag): ' + testRunner.invokedToolCount);
360+
361+
assert(testRunner.stderr.length === 0, 'should not have written to stderr=' + testRunner.stderr);
362+
assert(testRunner.succeeded, 'task should have succeeded');
363+
});
364+
338365
it('run maven with spotbugs plugin enabled', async function() {
339366
this.timeout(parseInt(process.env.TASK_TEST_TIMEOUT) || 20000);
340367
const testPath = path.join(__dirname, 'L0SpotbugsPlugin.js');
Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
import * as path from 'path';
2+
import * as fs from 'fs';
3+
4+
import { TaskMockRunner } from 'azure-pipelines-task-lib/mock-run';
5+
import { TaskLibAnswers } from 'azure-pipelines-task-lib/mock-answer';
6+
7+
import { getTempDir, initializeTest, MavenTaskInputs, setInputs } from './TestUtils';
8+
9+
const taskPath = path.join(__dirname, '..', 'maventask.js');
10+
11+
const taskRunner = new TaskMockRunner(taskPath);
12+
13+
// Set environment variable to ENABLE the RemoveDuplicateMavenRun feature flag
14+
// getPipelineFeature() looks for DISTRIBUTEDTASK_TASKS_<FeatureName>
15+
process.env['DISTRIBUTEDTASK_TASKS_REMOVEDUPLICATEMAVENRUN'] = 'true';
16+
17+
// Common initial setup
18+
initializeTest(taskRunner);
19+
20+
// Set Inputs
21+
const inputs: MavenTaskInputs = {
22+
mavenVersionSelection: 'Default',
23+
mavenPOMFile: 'pom.xml',
24+
options: '',
25+
goals: 'package',
26+
javaHomeSelection: 'JDKVersion',
27+
jdkVersion: 'default',
28+
publishJUnitResults: true,
29+
testResultsFiles: '**/TEST-*.xml',
30+
mavenOpts: '-Xmx2048m',
31+
checkstyleAnalysisEnabled: false,
32+
pmdAnalysisEnabled: false,
33+
findbugsAnalysisEnabled: false,
34+
mavenFeedAuthenticate: true,
35+
codeCoverageTool: 'JaCoCo',
36+
restoreOriginalPomXml: true
37+
};
38+
setInputs(taskRunner, inputs);
39+
40+
// Set up environment variables (task-lib does not support mocking getVariable)
41+
// Env vars in the mock framework must replace '.' with '_'
42+
delete process.env.M2_HOME; // Remove in case process running this test has it already set
43+
44+
// Provide answers for task mock
45+
const answers: TaskLibAnswers = {
46+
which: {
47+
mvn: '/home/bin/maven/bin/mvn'
48+
},
49+
checkPath: {
50+
'/home/bin/maven/bin/mvn': true,
51+
'pom.xml': true
52+
},
53+
exec: {
54+
'/home/bin/maven/bin/mvn -version': {
55+
code: 0,
56+
stdout: 'Maven version 1.0.0'
57+
},
58+
'/home/bin/maven/bin/mvn -f pom.xml help:effective-pom': {
59+
code: 0,
60+
stdout:
61+
'<Configuration>\r\n<project test>\r\n</project>\r\n</Configuration>\r\nEffective POMs, after inheritance, interpolation, and profiles are applied:\r\n\r\n<!-- ====================================================================== -->\r\n<!-- -->\r\n<!-- Generated by Maven Help Plugin on 2017-06-29T09:43:41 -->\r\n<!-- See: http://maven.apache.org/plugins/maven-help-plugin/ -->\r\n<!-- -->\r\n<!-- ====================================================================== -->\r\n\r\n<!-- ====================================================================== -->\r\n<!-- -->\r\n<!-- Effective POM for project -->\r\n<!-- \'com.microsoft.xplatalm:xplatalmApp:jar:1.0-SNAPSHOT\' -->\r\n<!-- -->\r\n<!-- ====================================================================== -->\r\n\r\n<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">\r\n</project>\r\n\r\n[INFO] ------------------------------------------------------------------------\r\n[INFO] BUILD SUCCESS\r\n[INFO] ------------------------------------------------------------------------\r\n[INFO] Total time: 0.927 s\r\n[INFO] Finished at: 2017-06-29T09:43:41-04:00\r\n[INFO] Final Memory: 7M/18M\r\n[INFO] ------------------------------------------------------------------------'
62+
},
63+
'/home/bin/maven/bin/mvn -f pom.xml clean verify': {
64+
code: 0,
65+
stdout: 'Maven verify done'
66+
}
67+
},
68+
findMatch: {
69+
'**/TEST-*.xml': [
70+
'/user/build/fun/test-123.xml'
71+
]
72+
},
73+
exist: {
74+
[path.join(getTempDir(), '.mavenInfo')] : true,
75+
[path.join('CCReport43F6D5EF', 'jacoco.xml')]: true,
76+
'CCReportPomA4D283EG.xml': true
77+
},
78+
rmRF: {
79+
target: { success: true },
80+
CCReport43F6D5EF: { success: true },
81+
[path.join('CCReport43F6D5EF', "pom.xml")]: { success: true },
82+
'CCReportPomA4D283EG.xml': { success: true }
83+
}
84+
};
85+
taskRunner.setAnswers(answers);
86+
87+
taskRunner.registerMock('azure-pipelines-tasks-codecoverage-tools/codecoveragefactory', {
88+
CodeCoverageEnablerFactory: class {
89+
public getTool(buildTool: string, ccTool: string) {
90+
if (buildTool.toLowerCase() !== 'maven' || ccTool.toLowerCase() !== 'jacoco') {
91+
throw new Error(`Should use maven-jacoco but called ${buildTool}-${ccTool}`);
92+
}
93+
94+
return {
95+
enableCodeCoverage() {
96+
console.log('Writing modified pom.xml contents');
97+
return Promise.resolve('CCReport43F6D5EF/target/site/jacoco-aggregate');
98+
}
99+
};
100+
}
101+
}
102+
});
103+
104+
const originalPomXmlContents = 'original pom.xml contents';
105+
106+
const fsClone = Object.assign({}, fs);
107+
Object.assign(fsClone, {
108+
readFileSync(filename: string, encoding: BufferEncoding ): string {
109+
if (filename === 'pom.xml' && encoding === 'utf8') {
110+
console.log('Reading original pom.xml');
111+
return originalPomXmlContents;
112+
}
113+
114+
return fs.readFileSync(filename, encoding);
115+
},
116+
writeFileSync(filename: string, data: any): void {
117+
if (filename === 'pom.xml') {
118+
if (data === originalPomXmlContents) {
119+
console.log('Writing original pom.xml contents');
120+
return;
121+
}
122+
123+
throw new Error(`Trying to write unknown data into pom.xml; data=${data}`);
124+
}
125+
126+
fs.writeFileSync(filename, data);
127+
}
128+
});
129+
taskRunner.registerMock('fs', fsClone);
130+
131+
// We only register this mock to prevent import conflicts with already mocked fs
132+
taskRunner.registerMock('fs-extra', {
133+
mkdirpSync(dir: string): void {
134+
// This should never be called
135+
}
136+
});
137+
138+
// Run task
139+
taskRunner.run();

Tasks/MavenV4/maventask.ts

Lines changed: 36 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,19 @@ async function execBuild() {
245245
if (isCodeCoverageOpted && mavenGoals.indexOf('clean') == -1) {
246246
mvnRun.arg('clean');
247247
}
248-
mvnRun.arg(mavenGoals);
248+
249+
if (tl.getPipelineFeature('RemoveDuplicateMavenRun') && isCodeCoverageOpted && ccTool.toLowerCase() === "jacoco") {
250+
const verifyOrLaterPhases = ['verify', 'install', 'deploy'];
251+
const hasVerifyOrLater = mavenGoals.some(goal => verifyOrLaterPhases.includes(goal.toLowerCase()));
252+
253+
if (!hasVerifyOrLater) {
254+
mvnRun.arg('verify');
255+
} else {
256+
mvnRun.arg(mavenGoals);
257+
}
258+
} else {
259+
mvnRun.arg(mavenGoals);
260+
}
249261

250262
// 2. Apply any goals for static code analysis tools selected by the user.
251263
mvnRun = applySonarQubeArgs(mvnRun);
@@ -451,27 +463,32 @@ function publishCodeCoverage(isCodeCoverageOpted: boolean): Q.Promise<boolean> {
451463
var defer = Q.defer<boolean>();
452464
if (isCodeCoverageOpted) {
453465
tl.debug("Collecting code coverage reports");
454-
455-
if (ccTool.toLowerCase() == "jacoco") {
456-
var mvnReport = tl.tool(mvnExec);
457-
mvnReport.arg('-f');
458-
mvnReport.arg(mavenPOMFile);
459-
mvnReport.line(mavenOptions);
460-
mvnReport.arg("verify");
461-
mvnReport.arg("-Dmaven.test.skip=true"); // This argument added to skip tests to avoid running them twice. More about this argument: http://maven.apache.org/surefire/maven-surefire-plugin/examples/skipping-tests.html
462-
mvnReport.exec().then(function (code) {
463-
publishCCToTfs();
464-
defer.resolve(true);
465-
}).fail(function (err) {
466-
sendCodeCoverageEmptyMsg();
467-
defer.reject(err);
468-
});
466+
if (tl.getPipelineFeature('RemoveDuplicateMavenRun')) {
467+
publishCCToTfs();
468+
defer.resolve(true);
469469
}
470470
else {
471-
if (ccTool.toLowerCase() == "cobertura") {
472-
publishCCToTfs();
471+
if (ccTool.toLowerCase() == "jacoco") {
472+
var mvnReport = tl.tool(mvnExec);
473+
mvnReport.arg('-f');
474+
mvnReport.arg(mavenPOMFile);
475+
mvnReport.line(mavenOptions);
476+
mvnReport.arg("verify");
477+
mvnReport.arg("-Dmaven.test.skip=true"); // This argument added to skip tests to avoid running them twice. More about this argument: http://maven.apache.org/surefire/maven-surefire-plugin/examples/skipping-tests.html
478+
mvnReport.exec().then(function (code) {
479+
publishCCToTfs();
480+
defer.resolve(true);
481+
}).fail(function (err) {
482+
sendCodeCoverageEmptyMsg();
483+
defer.reject(err);
484+
});
485+
}
486+
else {
487+
if (ccTool.toLowerCase() == "cobertura") {
488+
publishCCToTfs();
489+
}
490+
defer.resolve(true);
473491
}
474-
defer.resolve(true);
475492
}
476493
}
477494
else {

Tasks/MavenV4/task.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
"author": "Microsoft Corporation",
1818
"version": {
1919
"Major": 4,
20-
"Minor": 264,
20+
"Minor": 266,
2121
"Patch": 0
2222
},
2323
"releaseNotes": "Configuration of the SonarQube analysis was moved to the [SonarQube](https://marketplace.visualstudio.com/items?itemName=SonarSource.sonarqube) or [SonarCloud](https://marketplace.visualstudio.com/items?itemName=SonarSource.sonarcloud) extensions, in task `Prepare Analysis Configuration`",

Tasks/MavenV4/task.loc.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
"author": "Microsoft Corporation",
1818
"version": {
1919
"Major": 4,
20-
"Minor": 264,
20+
"Minor": 266,
2121
"Patch": 0
2222
},
2323
"releaseNotes": "ms-resource:loc.releaseNotes",

0 commit comments

Comments
 (0)