Skip to content
Permalink
Browse files

Implement Oleg's code review suggestions:

- replace extraneous tab characters with spaces
- follow Oracle coding standards
- remove unnecessary null check
- add test for JENKINS-31258 'detect test Mojo identified by Jenkins Maven Property'
- add test for JENKINS-31524 'ensure updated existing results are counted'
  • Loading branch information
demonfiddler committed Jan 15, 2016
1 parent f4f1692 commit 45712ad8938580c1ae1a8c76095917f119d3f0be
@@ -138,7 +138,7 @@ public boolean postExecute(MavenBuildProxy build, MavenProject pom, MojoInfo moj
fileSet = Iterables.filter(fileSet, new Predicate<File>() {
@Override
public boolean apply(File input) {
return !parsedFiles.containsKey(input) || parsedFiles.get(input) < input.lastModified();
return !parsedFiles.containsKey(input) || parsedFiles.get(input) != input.lastModified();
}
});

@@ -9,6 +9,7 @@
import java.util.Iterator;

import javax.annotation.CheckForNull;
import javax.annotation.Nonnull;

import org.apache.maven.project.MavenProject;
import org.apache.tools.ant.types.FileSet;
@@ -169,8 +170,9 @@ public boolean canRunTests(MojoInfo mojo) {

@CheckForNull public Iterable<File> getReportFiles(MavenProject pom, MojoInfo mojo) throws ComponentConfigurationException {
File reportsDir = getReportsDirectory(pom, mojo);
if (reportsDir != null && reportsDir.exists())
if (reportsDir.exists()) {
return getReportFiles(reportsDir, getFileSet(reportsDir));
}

return null;
}
@@ -182,11 +184,11 @@ public boolean canRunTests(MojoInfo mojo) {
* @return The directory containing the test reports.
* @throws ComponentConfigurationException if unable to retrieve the report directory from the MOJO configuration.
*/
protected File getReportsDirectory(MavenProject pom, MojoInfo mojo) throws ComponentConfigurationException {
// [JENKINS-31258] Allow unknown MOJOs to contribute test results in arbitrary locations by setting a Maven property.
String reportsDirectoryOverride = getReportsDirectoryOverride(mojo);
if (reportsDirectoryOverride != null)
return mojo.expressionEvaluator.alignToBaseDirectory(new File(reportsDirectoryOverride));
@Nonnull protected File getReportsDirectory(MavenProject pom, MojoInfo mojo) throws ComponentConfigurationException {
// [JENKINS-31258] Allow unknown MOJOs to contribute test results in arbitrary locations by setting a Maven property.
String reportsDirectoryOverride = getReportsDirectoryOverride(mojo);
if (reportsDirectoryOverride != null)
return mojo.expressionEvaluator.alignToBaseDirectory(new File(reportsDirectoryOverride));

if (this.reportDirectoryConfigKey != null) {
File reportsDir = mojo.getConfigurationValue(this.reportDirectoryConfigKey, File.class);
@@ -247,7 +249,7 @@ public static TestMojo lookup(MojoInfo mojo) {
TestMojo testMojo = lookup(mojo.pluginName.groupId, mojo.pluginName.artifactId, mojo.getGoal());

if (testMojo == null && getReportsDirectoryOverride(mojo) != null) {
testMojo = FALLBACK;
testMojo = FALLBACK;
}

if (testMojo != null && testMojo.canRunTests(mojo)) {
@@ -262,10 +264,12 @@ public static TestMojo lookup(MojoInfo mojo) {
* @param mojo An unknown MOJO.
* @return the value of the expression <code>${jenkins.&lt;mojo-execution-id&gt;.reportsDirectory}</code>.
*/
private static String getReportsDirectoryOverride(MojoInfo mojo) {
@CheckForNull private static String getReportsDirectoryOverride(MojoInfo mojo) {
// TODO: if and when this MOJO ceases to be tested against earlier Maven versions lacking MojoExecution.getLifecyclePhase(),
// Reinstate the following validity check:
// Validity of the override property should be constrained to "test" and "integration-test" phases but there seems to be no
// way to check this, as MojoExecution.getLifecyclePhase() (added 2009-05-12) causes tests to throw NoSuchMethodError!!!
// So, we're obliged to assume that the property is configured for a valid test life cycle phase.
// Until then, we're obliged to assume that the property is configured for a valid test life cycle phase.
// String phase = mojo.mojoExecution.getLifecyclePhase();
// if ("test".equals(phase) || "integration-test".equals(phase)) {
try {
@@ -274,7 +278,8 @@ private static String getReportsDirectoryOverride(MojoInfo mojo) {
if (result != null && !result.equals(reportsDirExpr) && !result.toString().trim().isEmpty())
return result.toString().trim();
} catch (ExpressionEvaluationException e) {
// Ignore evaluation exceptions.
// Note: no access to private MavenProject.logger.
e.printStackTrace();
}
// }
return null;
@@ -3,11 +3,18 @@
import static hudson.maven.MojoInfoBuilder.mojoBuilder;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;

import java.io.File;

import org.codehaus.plexus.component.configurator.expression.ExpressionEvaluationException;
import org.codehaus.plexus.component.configurator.expression.ExpressionEvaluator;

import hudson.maven.MojoInfo;
import hudson.maven.MojoInfoBuilder;

import org.junit.Before;
import org.junit.Test;
import org.jvnet.hudson.test.Issue;

/**
* Regression test for the detection of test mojos in {@link SurefireArchiver}.
@@ -197,6 +204,28 @@ public void shouldDetectAnyMojoWithATestGoal() {
assertTrue(this.surefireArchiver.isTestMojo(mojo));
}

@Test
@Issue("JENKINS-31258")
public void shouldDetectAnyMojoWithAJenkinsReportsDirectoryProperty() {
MojoInfoBuilder builder = mojoBuilder("some.weird.internal", "xxx-mojo", "xxx-goal")
.executionId("xxx-execution-id").evaluator(new ExpressionEvaluator() {
@Override
public Object evaluate(String expression) throws ExpressionEvaluationException {
if ("${jenkins.xxx-execution-id.reportsDirectory}".equals(expression))
return "target/xxx-test-reports";
return expression;
}

@Override
public File alignToBaseDirectory(File path) {
return path;
}
});

MojoInfo mojo = builder.build();
assertTrue(this.surefireArchiver.isTestMojo(mojo));
}

@Test
public void shouldNotDetectNonTestGoal() {
MojoInfoBuilder builder = mojoBuilder("some.weird.internal","test-mojo", "verify");
@@ -36,6 +36,7 @@
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
import org.jvnet.hudson.test.Issue;
import org.jvnet.hudson.test.RandomlyFails;
import org.mockito.Matchers;
import org.mockito.invocation.InvocationOnMock;
@@ -153,6 +154,33 @@ public void testResultsAreNotCountedTwice() throws InterruptedException, IOExcep
assertEquals(2658, result.getTotalCount());
}

@Test
@Issue("JENKINS-31524")
public void testUpdatedExistingResultsAreCounted() throws InterruptedException, IOException, URISyntaxException, ComponentConfigurationException {
URL resource = SurefireArchiverUnitTest.class.getResource("/surefire-archiver-test2");
File reportsDir = new File(resource.toURI().getPath());
doReturn(reportsDir).when(this.mojoInfo).getConfigurationValue("reportsDirectory", File.class);

File report1 = new File(reportsDir, "junit-report-1233.xml");
File report2 = new File(reportsDir, "junit-report-1472.xml");
long startTime = this.mojoInfo.getStartTime();

// Current report files should be counted and stale ones ignored.
report1.setLastModified(startTime);
report2.setLastModified(startTime - (60 * 1000));
this.archiver.postExecute(buildProxy, null, this.mojoInfo, new NullBuildListener(), null);
SurefireReport action = this.build.getAction(SurefireReport.class);
TestResult result = action.getResult();
assertEquals(5, result.getTotalCount());

// Updated existing report files should now be included.
report2.setLastModified(startTime);
this.archiver.postExecute(buildProxy, null, this.mojoInfo, new NullBuildListener(), null);
action = this.build.getAction(SurefireReport.class);
result = action.getResult();
assertEquals(2658, result.getTotalCount());
}

@Test
public void testMultiThreaded() throws InterruptedException, IOException, URISyntaxException, ComponentConfigurationException {
File reportsDir2 = new File(SurefireArchiverUnitTest.class.getResource("/surefire-archiver-test2").toURI().getPath());

0 comments on commit 45712ad

Please sign in to comment.
You can’t perform that action at this time.