Bug 1198257 - Better support for providing a directory name and discovering reftests under that directory, r=jmaher
authorJames Graham <james@hoppipolla.co.uk>
Mon, 20 Jul 2015 17:03:02 +0100
changeset 295627 70708e14dceb5f9a9b38cf51673218b8bd2e91a1
parent 295626 4a67faf13596e8a4e0757a65a21ca75241c921ef
child 295628 46b3878338c22fa1ba8e662fa754adb272fec08e
push id5245
push userraliiev@mozilla.com
push dateThu, 29 Oct 2015 11:30:51 +0000
treeherdermozilla-beta@dac831dc1bd0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjmaher
bugs1198257
milestone43.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1198257 - Better support for providing a directory name and discovering reftests under that directory, r=jmaher
layout/tools/reftest/reftest.js
layout/tools/reftest/runreftest.py
--- a/layout/tools/reftest/reftest.js
+++ b/layout/tools/reftest/reftest.js
@@ -64,16 +64,17 @@ var gB2GisMulet;                // bool
 // Are we using <iframe mozbrowser>?
 var gBrowserIsIframe;           // bool
 var gBrowserMessageManager;
 var gCanvas1, gCanvas2;
 // gCurrentCanvas is non-null between InitCurrentCanvasWithSnapshot and the next
 // RecordResult.
 var gCurrentCanvas = null;
 var gURLs;
+var gManifestsLoaded = {};
 // Map from URI spec to the number of times it remains to be used
 var gURIUseCounts;
 // Map from URI spec to the canvas rendered for that URI
 var gURICanvases;
 var gTestResults = {
   // Successful...
   Pass: 0,
   LoadOnly: 0,
@@ -484,21 +485,24 @@ function StartTests()
     } catch(e) {
         gDumpLog("REFTEST TEST-UNEXPECTED-FAIL | | Unable to find reftest.manifests pref.  Please ensure your profile is setup properly\n");
         DoneTests();
     }
 
     try {
         var globalFilter = manifests.hasOwnProperty("") ? new RegExp(manifests[""]) : null;
         var manifestURLs = Object.keys(manifests);
-        manifestURLs.sort();
+
+        // Ensure we read manifests from higher up the directory tree first so that we
+        // process includes before reading the included manifest again
+        manifestURLs.sort(function(a,b) {return a.length - b.length})
         manifestURLs.forEach(function(manifestURL) {
             gDumpLog("Readings manifest" + manifestURL + "\n");
-            var pathFilters = manifests[manifestURL].map(function(x) {return new RegExp(x)});
-            ReadTopManifest(manifestURL, [globalFilter, pathFilters]);
+            var filter = manifests[manifestURL] ? new RegExp(manifests[manifestURL]) : null;
+            ReadTopManifest(manifestURL, [globalFilter, filter, false]);
         });
         BuildUseCounts();
 
         // Filter tests which will be skipped to get a more even distribution when chunking
         // tURLs is a temporary array containing all active tests
         var tURLs = new Array();
         for (var i = 0; i < gURLs.length; ++i) {
             if (gURLs[i].expected == EXPECTED_DEATH)
@@ -742,46 +746,60 @@ function AddPrefSettings(aWhere, aPrefNa
         aTestPrefSettings.push(setting);
     }
     if (aWhere != "test-") {
         aRefPrefSettings.push(setting);
     }
     return true;
 }
 
-function ReadTopManifest(aFileURL, aFilters)
+function ReadTopManifest(aFileURL, aFilter)
 {
     var url = gIOService.newURI(aFileURL, null, null);
     if (!url)
         throw "Expected a file or http URL for the manifest.";
-    ReadManifest(url, EXPECTED_PASS, aFilters);
+    ReadManifest(url, EXPECTED_PASS, aFilter);
 }
 
-function AddTestItem(aTest, aFilters)
+function AddTestItem(aTest, aFilter)
 {
-    if (!aFilters)
-        aFilters = [null, []];
+    if (!aFilter)
+        aFilter = [null, [], false];
 
-    if ((aFilters[0] && !aFilters[0].test(aTest.url1.spec)) ||
-        (aFilters[1].length > 0 &&
-         !aFilters[1].some(function(filter) {return filter.test(aTest.url1.spec)})))
+    globalFilter = aFilter[0];
+    manifestFilter = aFilter[1];
+    invertManifest = aFilter[2];
+    if ((globalFilter && !globalFilter.test(aTest.url1.spec)) ||
+        (manifestFilter &&
+         !(invertManifest ^ manifestFilter.test(aTest.url1.spec))))
         return;
     if (gFocusFilterMode == FOCUS_FILTER_NEEDS_FOCUS_TESTS &&
         !aTest.needsFocus)
         return;
     if (gFocusFilterMode == FOCUS_FILTER_NON_NEEDS_FOCUS_TESTS &&
         aTest.needsFocus)
         return;
     gURLs.push(aTest);
 }
 
 // Note: If you materially change the reftest manifest parsing,
 // please keep the parser in print-manifest-dirs.py in sync.
-function ReadManifest(aURL, inherited_status, aFilters)
+function ReadManifest(aURL, inherited_status, aFilter)
 {
+    // Ensure each manifest is only read once. This assumes that manifests that are
+    // included with an unusual inherited_status or filters will be read via their
+    // include before they are read directly in the case of a duplicate
+    if (gManifestsLoaded.hasOwnProperty(aURL.spec)) {
+        if (gManifestsLoaded[aURL.spec] === null)
+            return;
+        else
+            aFilter = [aFilter[0], aFilter[1], true];
+    }
+    gManifestsLoaded[aURL.spec] = aFilter[1];
+
     var secMan = CC[NS_SCRIPTSECURITYMANAGER_CONTRACTID]
                      .getService(CI.nsIScriptSecurityManager);
 
     var listURL = aURL;
     var channel = gIOService.newChannelFromURI2(aURL,
                                                 null,      // aLoadingNode
                                                 Services.scriptSecurityManager.getSystemPrincipal(),
                                                 null,      // aTriggeringPrincipal
@@ -987,17 +1005,17 @@ function ReadManifest(aURL, inherited_st
         if (items[0] == "include") {
             if (items.length != 2)
                 throw "Error in manifest file " + aURL.spec + " line " + lineNo + ": incorrect number of arguments to include";
             if (runHttp)
                 throw "Error in manifest file " + aURL.spec + " line " + lineNo + ": use of include with http";
             var incURI = gIOService.newURI(items[1], null, listURL);
             secMan.checkLoadURIWithPrincipal(principal, incURI,
                                              CI.nsIScriptSecurityManager.DISALLOW_SCRIPT);
-            ReadManifest(incURI, expected_status, aFilters);
+            ReadManifest(incURI, expected_status, aFilter);
         } else if (items[0] == TYPE_LOAD) {
             if (items.length != 2)
                 throw "Error in manifest file " + aURL.spec + " line " + lineNo + ": incorrect number of arguments to load";
             if (expected_status != EXPECTED_PASS &&
                 expected_status != EXPECTED_DEATH)
                 throw "Error in manifest file " + aURL.spec + " line " + lineNo + ": incorrect known failure type for load test";
             var [testURI] = runHttp
                             ? ServeFiles(principal, httpDepth,
@@ -1017,17 +1035,17 @@ function ReadManifest(aURL, inherited_st
                           needsFocus: needs_focus,
                           slow: slow,
                           prefSettings1: testPrefSettings,
                           prefSettings2: refPrefSettings,
                           fuzzyMaxDelta: fuzzy_max_delta,
                           fuzzyMaxPixels: fuzzy_max_pixels,
                           url1: testURI,
                           url2: null,
-                          chaosMode: chaosMode }, aFilters);
+                          chaosMode: chaosMode }, aFilter);
         } else if (items[0] == TYPE_SCRIPT) {
             if (items.length != 2)
                 throw "Error in manifest file " + aURL.spec + " line " + lineNo + ": incorrect number of arguments to script";
             var [testURI] = runHttp
                             ? ServeFiles(principal, httpDepth,
                                          listURL, [items[1]])
                             : [gIOService.newURI(items[1], null, listURL)];
             var prettyPath = runHttp
@@ -1044,17 +1062,17 @@ function ReadManifest(aURL, inherited_st
                           needsFocus: needs_focus,
                           slow: slow,
                           prefSettings1: testPrefSettings,
                           prefSettings2: refPrefSettings,
                           fuzzyMaxDelta: fuzzy_max_delta,
                           fuzzyMaxPixels: fuzzy_max_pixels,
                           url1: testURI,
                           url2: null,
-                          chaosMode: chaosMode }, aFilters);
+                          chaosMode: chaosMode }, aFilter);
         } else if (items[0] == TYPE_REFTEST_EQUAL || items[0] == TYPE_REFTEST_NOTEQUAL) {
             if (items.length != 3)
                 throw "Error in manifest file " + aURL.spec + " line " + lineNo + ": incorrect number of arguments to " + items[0];
             var [testURI, refURI] = runHttp
                                   ? ServeFiles(principal, httpDepth,
                                                listURL, [items[1], items[2]])
                                   : [gIOService.newURI(items[1], null, listURL),
                                      gIOService.newURI(items[2], null, listURL)];
@@ -1074,17 +1092,17 @@ function ReadManifest(aURL, inherited_st
                           needsFocus: needs_focus,
                           slow: slow,
                           prefSettings1: testPrefSettings,
                           prefSettings2: refPrefSettings,
                           fuzzyMaxDelta: fuzzy_max_delta,
                           fuzzyMaxPixels: fuzzy_max_pixels,
                           url1: testURI,
                           url2: refURI,
-                          chaosMode: chaosMode }, aFilters);
+                          chaosMode: chaosMode }, aFilter);
         } else {
             throw "Error in manifest file " + aURL.spec + " line " + lineNo + ": unknown test type " + items[0];
         }
     }
 }
 
 function AddURIUseCount(uri)
 {
--- a/layout/tools/reftest/runreftest.py
+++ b/layout/tools/reftest/runreftest.py
@@ -127,56 +127,79 @@ class ReftestThread(threading.Thread):
                 yield line
 
 class ReftestResolver(object):
     def defaultManifest(self, suite):
         return {"reftest": "reftest.list",
                 "crashtest": "crashtests.list",
                 "jstestbrowser": "jstests.list"}[suite]
 
-    def findManifest(self, suite, test_file):
+    def directoryManifest(self, suite, path):
+        return os.path.join(path, self.defaultManifest(suite))
+
+    def findManifest(self, suite, test_file, subdirs=True):
         """Return a tuple of (manifest-path, filter-string) for running test_file.
 
         test_file is a path to a test or a manifest file
         """
+        rv = []
+        default_manifest = self.defaultManifest(suite)
         if not os.path.isabs(test_file):
             test_file = self.absManifestPath(test_file)
 
         if os.path.isdir(test_file):
-            return os.path.join(test_file, self.defaultManifest(suite)), None
+            for dirpath, dirnames, filenames in os.walk(test_file):
+                if default_manifest in filenames:
+                    rv.append((os.path.join(dirpath, default_manifest), None))
+                    # We keep recursing into subdirectories which means that in the case
+                    # of include directives we get the same manifest multiple times.
+                    # However reftest.js will only read each manifest once
 
-        if test_file.endswith('.list'):
-            return test_file, None
+        elif test_file.endswith('.list'):
+            if os.path.exists(test_file):
+                rv = [(test_file, None)]
+        else:
+            dirname, pathname = os.path.split(test_file)
+            found = True
+            while not os.path.exists(os.path.join(dirname, default_manifest)):
+                dirname, suffix = os.path.split(dirname)
+                pathname = os.path.join(suffix, pathname)
+                if os.path.dirname(dirname) == dirname:
+                    found = False
+                    break
+            if found:
+                rv = [(os.path.join(dirname, default_manifest),
+                       r".*(?:/|\\)%s$" % pathname)]
 
-        return (self.findManifest(suite, os.path.dirname(test_file))[0],
-                r".*(?:/|\\)%s$" % os.path.basename(test_file))
+        return rv
 
     def absManifestPath(self, path):
-        return os.path.abspath(path)
+        return os.path.normpath(os.path.abspath(path))
 
     def manifestURL(self, options, path):
         return "file://%s" % path
 
     def resolveManifests(self, options, tests):
         suite = options.suite
         manifests = {}
         for testPath in tests:
-            manifest, filter_str = self.findManifest(suite, testPath)
-            manifest = self.manifestURL(options, manifest)
-            if manifest not in manifests:
-                manifests[manifest] = set()
-            if filter_str is not None:
+            for manifest, filter_str in self.findManifest(suite, testPath):
+                manifest = self.manifestURL(options, manifest)
+                if manifest not in manifests:
+                    manifests[manifest] = set()
                 manifests[manifest].add(filter_str)
 
         for key in manifests.iterkeys():
             if os.path.split(key)[1] != self.defaultManifest(suite):
                 print >> sys.stderr, "Invalid manifest for suite %s, %s" %(options.suite, key)
                 sys.exit(1)
-            manifests[key] = sorted(list(manifests[key]))
-
+            if None in manifests[key]:
+                manifests[key] = None
+            else:
+                manifests[key] = "|".join(list(manifests[key]))
         return manifests
 
 class RefTest(object):
     oldcwd = os.getcwd()
     resolver_cls = ReftestResolver
 
     def __init__(self):
         self.update_mozinfo()