[FIXED JENKINS-14570] Make filename for warnings fixed for each parser.

jenkinsci · Aug 21, 2012 · d1cbae3 · d1cbae3
1 parent 9369004
commit d1cbae3
Show file tree

Hide file tree

Showing 3 changed files with 87 additions and 9 deletions.
diff --git a/pom.xml b/pom.xml
@@ -41,7 +41,7 @@
     <dependency>
       <groupId>org.jvnet.hudson.plugins</groupId>
       <artifactId>analysis-test</artifactId>
-      <version>1.9</version>
+      <version>1.10</version>
       <scope>test</scope>
     </dependency>
     <dependency>

diff --git a/src/main/java/hudson/plugins/warnings/WarningsResult.java b/src/main/java/hudson/plugins/warnings/WarningsResult.java
@@ -8,6 +8,8 @@
 import hudson.plugins.warnings.parser.ParserRegistry;
 import hudson.plugins.warnings.parser.Warning;
 
+import java.io.File;
+
 import com.thoughtworks.xstream.XStream;
 
 /**
@@ -17,6 +19,8 @@
  * @author Ulli Hafner
  */
 public class WarningsResult extends BuildResult {
+    /** Version < 4.0 file name of warnings. */
+    static final String ORIGINAL_COMPILER_WARNINGS_XML = "compiler-warnings.xml";
     /** Unique identifier of this class. */
     private static final long serialVersionUID = -137460587767210579L;
     /** The group of the parser. @since 4.0 */
@@ -84,12 +88,26 @@ private String getUrl() {
 
     @Override
     protected String getSerializationFileName() {
-        if (group == null) { // prior 4.0
-            return "compiler-warnings.xml";
+        FileChecker fileChecker = new FileChecker(getOwner().getRootDir());
+        return getFileName(fileChecker, ParserRegistry.getUrl(group));
+    }
+
+    String getFileName(final FileChecker fileChecker, final int groupUrl) {
+        String fileName = ORIGINAL_COMPILER_WARNINGS_XML;
+        if (fileChecker.canRead(fileName)) {
+            return fileName;
         }
-        else {
-            return "compiler-" + ParserRegistry.getUrl(group) + "-warnings.xml";
+
+        fileName = createFileName(groupUrl);
+        if (fileChecker.canRead(fileName)) {
+            return fileName;
         }
+
+        return group.replaceAll("\\W+", "") + ".xml";
+    }
+
+    String createFileName(final int groupUrl) {
+        return "compiler-" + groupUrl + "-warnings.xml";
     }
 
     /** {@inheritDoc} */
@@ -106,4 +124,21 @@ public String getDisplayName() {
     protected Class<? extends ResultAction<? extends BuildResult>> getResultActionType() {
         return WarningsResultAction.class;
     }
+
+    /**
+     * Provides a way to hide file system access during testing.
+     *
+     * @author Ulli Hafner
+     */
+    static class FileChecker {
+        private final File root;
+
+        FileChecker(final File root) {
+            this.root = root;
+        }
+
+        boolean canRead(final String fileName) {
+            return new File(root, fileName).canRead();
+        }
+    }
 }
diff --git a/src/test/java/hudson/plugins/warnings/WarningsResultTest.java b/src/test/java/hudson/plugins/warnings/WarningsResultTest.java
@@ -1,12 +1,15 @@
 package hudson.plugins.warnings;
 
+import static org.junit.Assert.*;
 import static org.mockito.Mockito.*;
 import hudson.model.AbstractBuild;
 import hudson.plugins.analysis.core.BuildHistory;
+import hudson.plugins.analysis.core.NullBuildHistory;
 import hudson.plugins.analysis.core.ParserResult;
 import hudson.plugins.analysis.test.BuildResultTest;
 import hudson.plugins.analysis.util.model.AnnotationContainer;
 import hudson.plugins.analysis.util.model.DefaultAnnotationContainer;
+import hudson.plugins.warnings.WarningsResult.FileChecker;
 import hudson.plugins.warnings.parser.Warning;
 
 import java.util.GregorianCalendar;
@@ -19,9 +22,46 @@
  * Tests the class {@link WarningsResult}.
  */
 public class WarningsResultTest extends BuildResultTest<WarningsResult> {
+    private static final int GROUP_URL = 1;
+    private static final String ORIGINAL_FILENAME = WarningsResult.ORIGINAL_COMPILER_WARNINGS_XML;
+
     @Override
     protected WarningsResult createBuildResult(final AbstractBuild<?, ?> build, final ParserResult project, final BuildHistory history) {
-        return new WarningsResult(build, history, project, "UTF-8", null, false);
+        return createResult(build, project, history, null);
+    }
+
+    private WarningsResult createResult(final AbstractBuild<?, ?> build, final ParserResult project, final BuildHistory history, final String group) {
+        return new WarningsResult(build, history, project, "UTF-8", group, false);
+    }
+
+    /**
+     * Verifies that filenames are correctly parsed.
+     *
+     * @see <a href="http://issues.jenkins-ci.org/browse/JENKINS-14570">Issue 14570</a>
+     */
+    @Test
+    public void testWarningsFileName() {
+        FileChecker stub = mock(FileChecker.class);
+
+        WarningsResult result = createResult("group1");
+        verifyFileName(result, stub, "group1");
+
+        result = createResult("charCHAR/( 2");
+        verifyFileName(result, stub, "charCHAR2");
+
+        when(stub.canRead(result.createFileName(GROUP_URL))).thenReturn(true);
+        verifyFileName(result, stub, result.createFileName(GROUP_URL).replaceFirst(".xml", ""));
+
+        when(stub.canRead(ORIGINAL_FILENAME)).thenReturn(true);
+        verifyFileName(result, stub, ORIGINAL_FILENAME.replaceFirst(".xml", ""));
+    }
+
+    private WarningsResult createResult(final String group) {
+        return createResult(createBuild(), new ParserResult(), new NullBuildHistory(), group);
+    }
+
+    private void verifyFileName(final WarningsResult result, final FileChecker stub, final String expected) {
+        assertEquals("Wrong filename selected", expected + ".xml", result.getFileName(stub, GROUP_URL));
     }
 
     /**
@@ -80,11 +120,14 @@ private WarningsResult createResult(final ParserResult newWarnings, final Annota
         BuildHistory history = mock(BuildHistory.class);
         when(history.getReferenceAnnotations()).thenReturn(oldWarnings);
 
-        @SuppressWarnings("rawtypes")
+        return createResultUnderTest(newWarnings, history, createBuild());
+    }
+
+    @SuppressWarnings("rawtypes")
+    private AbstractBuild<?, ?> createBuild() {
         AbstractBuild build = mock(AbstractBuild.class);
         when(build.getTimestamp()).thenReturn(new GregorianCalendar());
-
-        return createResultUnderTest(newWarnings, history, build);
+        return build;
     }
 
     private WarningsResult createResultUnderTest(final ParserResult newWarnings, final BuildHistory history, @SuppressWarnings("rawtypes") final AbstractBuild build) {