Bulk file check on slaves

src/main/java/com/uber/jenkins/phabricator/coverage/CoberturaXMLParser.java

@@ -118,25 +118,34 @@ private Map<String, List<Integer>> parse(Map<NodeList, List<String>> coverageDat
             NodeList classes = entry.getKey();
             List<String> sourceDirs = entry.getValue();
 
-            // Loop over all files in the coverage report
+            // Collect all filenames in coverage report
+            List<String> fileNames = new ArrayList<String>();
+            List<NodeList> childNodes = new ArrayList<NodeList>();
             for (int i = 0; i < classes.getLength(); i++) {
                 Node classNode = classes.item(i);
                 String fileName = classNode.getAttributes().getNamedItem(NODE_FILENAME).getTextContent();
 
                 if (includeFileNames != null && !includeFileNames.contains(FilenameUtils.getName(fileName))) {
                     continue;
                 }
+                fileNames.add(fileName);
+                childNodes.add(classNode.getChildNodes());
+            }
+
+            // Make multiple guesses on which of the `sourceDirs` contains files in question
+            Map<String, String> detectedSourceRoots = new PathResolver(workspace, sourceDirs).choose(fileNames);
 
-                // Make a guess on which of the `sourceDirs` contains the file in question
-                String detectedSourceRoot = new PathResolver(workspace, sourceDirs).choose(fileName);
-                fileName = join(detectedSourceRoot, fileName);
+            // Loop over all files which are needed for coverage report
+            for (int i = 0; i < fileNames.size(); i++) {
+                String fileName = fileNames.get(i);
+                fileName = join(detectedSourceRoots.get(fileName), fileName);
 
                 SortedMap<Integer, Integer> hitCounts = internalCounts.get(fileName);
                 if (hitCounts == null) {
                     hitCounts = new TreeMap<Integer, Integer>();
                 }
 
-                NodeList children = classNode.getChildNodes();
+                NodeList children = childNodes.get(i);
                 for (int j = 0; j < children.getLength(); j++) {
                     Node child = children.item(j);
 

src/main/java/com/uber/jenkins/phabricator/coverage/PathResolver.java

@@ -21,9 +21,15 @@
 package com.uber.jenkins.phabricator.coverage;
 
 import hudson.FilePath;
+import hudson.remoting.VirtualChannel;
+import jenkins.MasterToSlaveFileCallable;
 
 import java.io.IOException;
+import java.io.File;
+import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 
 public class PathResolver {
     private final FilePath root;
@@ -35,26 +41,49 @@ public PathResolver(FilePath root, List<String> candidates) {
     }
 
     /**
-     * Using the workspace's root FilePath and a file that is presumed to exist on the node running the tests,
+     * Using the workspace's root FilePath and files that is presumed to exist on the node running the tests,
      * recurse over the `sources` provided by Cobertura and look for a combination where the file exists.
      *
      * This is a heuristic, and not perfect, to overcome changes to Python's coverage.py module which introduced
      * additional `source` directories in version 4.0.3
+     *
+     * Returns map where key is filename and value - sourceDirectory
      */
-    public String choose(String filename) {
-        for (String sourceDir : candidates) {
-            FilePath candidate = new FilePath(root, sourceDir);
-            candidate = new FilePath(candidate, filename);
-            try {
-                if (candidate.exists()) {
-                    return sourceDir;
+
+    public Map<String, String> choose(List<String> filenames) {
+        try {
+            if (candidates.size() > 0) {
+                return root.act(new PathResolverChooseMultiCallable(candidates, filenames));
+            }
+        } catch (IOException e) {
+            e.printStackTrace();
+        } catch (InterruptedException e) {
+            e.printStackTrace();
+        }
+        return Collections.emptyMap();
+    }
+
+    private static final class PathResolverChooseMultiCallable extends MasterToSlaveFileCallable<Map<String, String>> {
+        private final List<String> candidates;
+        private final List<String> filenames;
+
+        private PathResolverChooseMultiCallable(List<String> candidates, List<String> filenames) {
+            this.candidates = candidates;
+            this.filenames = filenames;
+        }
+
+        public Map<String, String> invoke(File f, VirtualChannel channel) {
+            Map<String, String> res = new HashMap<String, String>(filenames.size());
+            for (String filename : filenames) {
+                for (String sourceDir : candidates) {
+                    File candidate = new File(f, sourceDir);
+                    candidate = new File(candidate, filename);
+                    if (candidate.exists()) {
+                        res.put(filename, sourceDir);
+                    }
                 }
-            } catch (IOException e) {
-                e.printStackTrace();
-            } catch (InterruptedException e) {
-                e.printStackTrace();
             }
+            return res;
         }
-        return null;
     }
 }

src/test/java/com/uber/jenkins/phabricator/coverage/CoberturaCoverageProviderTest.java

@@ -19,6 +19,7 @@
 import java.nio.file.Paths;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.TimeUnit;
 
 import javax.xml.parsers.ParserConfigurationException;
 
@@ -106,9 +107,9 @@ public void testGetMetricsWithBuildActionResult() throws Exception {
 
     @Test
     public void testGetMetricsWithoutBuildActionResult() throws Exception {
-        FreeStyleBuild build = getBuild();
+        FreeStyleBuild build = getExecutedBuild();
         Path testCoverageFile = Paths.get(getClass().getResource(TEST_COVERAGE_FILE).toURI());
-        FileUtils.copyFile(testCoverageFile.toFile(), new File(build.getRootDir(), "coverage.xml"));
+        FileUtils.copyFile(testCoverageFile.toFile(), new File(build.getWorkspace().getRemote(), "coverage.xml"));
         provider.setBuild(build);
         assertTrue(provider.hasCoverage());
 
@@ -119,33 +120,30 @@ public void testGetMetricsWithoutBuildActionResult() throws Exception {
 
     @Test
     public void testGetMetricsWithoutBuildActionResultDeletesFilesFromMasterAfter() throws Exception {
-        FreeStyleBuild build = getBuild();
+        FreeStyleProject project = j.createFreeStyleProject();
+        FreeStyleBuild remoteBuild = project.scheduleBuild2(0).get(100, TimeUnit.MINUTES);
         Path testCoverageFile = Paths.get(getClass().getResource(TEST_COVERAGE_FILE).toURI());
-        File workspaceCopy = new File(build.getRootDir(), "coverage.xml");
-        FileUtils.copyFile(testCoverageFile.toFile(), workspaceCopy);
-        provider.setBuild(build);
+        File cov = new File(remoteBuild.getWorkspace().getRemote(), "coverage.xml");
+        FileUtils.copyFile(testCoverageFile.toFile(), cov);
+        provider.setBuild(remoteBuild);
         assertTrue(provider.hasCoverage());
 
         CodeCoverageMetrics metrics = provider.getMetrics();
         assertNotNull(metrics);
-        assertFalse(workspaceCopy.exists());
+        assertFalse(new File(project.getLastBuild().getRootDir(), "coverage.xml").exists());
     }
 
     @Test
     public void testGetLineCoverageNull() throws Exception {
-        FreeStyleProject project = j.createFreeStyleProject();
-        FreeStyleBuild build = new FreeStyleBuild(project);
-        provider.setBuild(build);
+        provider.setBuild(getExecutedBuild());
         assertNull(provider.readLineCoverage());
     }
 
     @Test
     public void testGetLineCoverageWithFile() throws Exception {
-        FreeStyleProject project = j.createFreeStyleProject();
-        FreeStyleBuild build = new FreeStyleBuild(project);
+        FreeStyleBuild build = getExecutedBuild();
 
-        File coverageFile = new File(build.getRootDir(), "coverage0.xml");
-        assertTrue(build.getRootDir().mkdirs());
+        File coverageFile = new File(build.getWorkspace().getRemote(), "coverage0.xml");
         InputStream in = getClass().getResourceAsStream("go-torch-coverage.xml");
         OutputStream out = new BufferedOutputStream(new FileOutputStream(coverageFile));
         IOUtils.copy(in, out);
@@ -189,4 +187,8 @@ private CoverageResult getMockResult() {
     private FreeStyleBuild getBuild() throws IOException {
         return new FreeStyleBuild(j.createFreeStyleProject());
     }
+
+    private FreeStyleBuild getExecutedBuild() throws Exception {
+        return j.createFreeStyleProject().scheduleBuild2(0).get(100, TimeUnit.MINUTES);
+    }
 }

src/test/java/com/uber/jenkins/phabricator/coverage/PathResolverTest.java

@@ -20,7 +20,9 @@
 
 package com.uber.jenkins.phabricator.coverage;
 
+import com.google.common.collect.ImmutableMap;
 import com.google.common.io.Files;
+
 import hudson.FilePath;
 import org.junit.After;
 import org.junit.Before;
@@ -36,11 +38,15 @@
 @RunWith(Parameterized.class)
 public class PathResolverTest {
     public List<String> candidates;
+    public List<String> filenames;
+    public Map<String, String> expectedResult;
 
     private Stack<File> cleanupPaths;
 
-    public PathResolverTest(List<String> candidates) {
+    public PathResolverTest(List<String> candidates, List<String> filenames, Map<String, String> expectedResult) {
         this.candidates = candidates;
+        this.filenames = filenames;
+        this.expectedResult = expectedResult;
     }
 
     @Test
@@ -62,12 +68,10 @@ public void testChoose() throws Exception {
             cleanupPaths.push(file);
         }
 
-        String chosen = new PathResolver(new FilePath(tmpDir), dirs).choose("dir/file");
+        Map<String, String> chosen = new PathResolver(new FilePath(tmpDir), dirs).choose(filenames);
 
-        if (candidates.isEmpty()) {
-            assertNull(chosen);
-        } else {
-            assertEquals("workspace/", chosen);
+        for (String filename  : filenames) {
+            assertEquals(expectedResult.get(filename), chosen.get(filename));
         }
     }
 
@@ -86,9 +90,10 @@ public void tearDown() {
     @Parameterized.Parameters
     public static Collection<Object[]> data() {
         Collection<Object[]> result = new ArrayList();
-        result.add(new Object[] { Collections.emptyList() });
-        result.add(new Object[] { Arrays.asList("workspace/", "workspace/dir/", "workspace/dir/file") });
-        result.add(new Object[] { Arrays.asList("workspace/", "workspace/dir/", "workspace/dir/dir/", "workspace/dir/file") });
+        result.add(new Object[] { Collections.emptyList(), Arrays.asList("dir/file"), ImmutableMap.<String, String>of()});
+        result.add(new Object[] { Arrays.asList("workspace/", "workspace/dir/", "workspace/dir/file"), Arrays.asList("dir/file"), ImmutableMap.<String, String>of("dir/file", "workspace/") });
+        result.add(new Object[] { Arrays.asList("workspace/", "workspace/dir/", "workspace/dir/dir/", "workspace/dir/file"), Arrays.asList("dir/file"), ImmutableMap.<String, String>of("dir/file", "workspace/")});
+        result.add(new Object[] { Arrays.asList("workspace/", "workspace/dir/", "workspace/dir/dir/", "workspace/dir/file", "workspace2/dir/", "workspace2/dir/file2"), Arrays.asList("dir/file", "file2"), ImmutableMap.<String, String>of("dir/file", "workspace/", "file2", "workspace2/dir/") });
         return result;
     }
 }