Add per-test scoring

interop-scoring/main.js

@@ -225,26 +225,24 @@ const KNOWN_TEST_STATUSES = new Set([
 // subtest results. Due to some missing subtests, this score skewed lower than the
 // current implementation. Neither is without its drawbacks, and the hope is that
 // the current approach will score runs more optimistically and avoid subtest matching.
-function aggregateInteropTestScores(testPassCounts, numBrowsers) {
-  if (testPassCounts.size === 0) return 0;
+function aggregateInteropTestScores(testScores, numBrowsers) {
+  if (testScores.size === 0) return 0;
   let aggregateScore = 0;
-  for (const testResults of testPassCounts.values()) {
+  for (const browserScores of testScores.values()) {
     let minTestScore = 1;
     // If a test result value is missing from any browser, the interop score is 0.
-    if (testResults['subtestTotal'].length !== numBrowsers) {
+    if (browserScores.length !== numBrowsers) {
       minTestScore = 0;
     } else {
       // Find the lowest score for the test among all browser runs.
-      for (let i = 0; i < numBrowsers; i++) {
-        const testScore = (
-          testResults['subtestPasses'][i] / testResults['subtestTotal'][i]);
-        minTestScore = Math.min(minTestScore, testScore);
+      for (const browserScore of browserScores) {
+        minTestScore = Math.min(minTestScore, browserScore);
       }
     }
     // Add the minimum test score to the aggregate interop score.
     aggregateScore += Math.floor(1000 * minTestScore);
   }
-  return Math.floor(aggregateScore / testPassCounts.size) || 0;
+  return Math.floor(aggregateScore / testScores.size) || 0;
 }
 
 // Score a set of runs (independently) on a set of tests. The runs are presumed
@@ -279,7 +277,7 @@ function aggregateInteropTestScores(testPassCounts, numBrowsers) {
 //   than if we used rational numbers.
 function scoreRuns(runs, allTestsSet) {
   const scores = [];
-  const testPassCounts = new Map();
+  const testScores = new Map();
   const unexpectedNonOKTests = new Set();
 
   try {
@@ -292,42 +290,23 @@ function scoreRuns(runs, allTestsSet) {
           return;
         }
 
-        // TODO: Validate the data by checking that all statuses are recognized.
-
-        let subtestPasses = 0;
-        let subtestTotal = 1;
-
-        // Keep subtest data for every test in order to calculate interop scores.
-        // A test entry is created the first time each test is encountered.
-        if (!testPassCounts.has(testname)) {
-          testPassCounts.set(testname, {});
-          testPassCounts.get(testname)['subtestPasses'] = [];
-          testPassCounts.get(testname)['subtestTotal'] = [];
+        // Keep each browser's score for every test in order to calculate
+        // interop scores. A test entry is created the first time each test is
+        // encountered.
+        if (!testScores.has(testname)) {
+          testScores.set(testname, []);
         }
-        if ('subtests' in results) {
-          if (results['status'] != 'OK' && !KNOWN_TEST_STATUSES.has(testname)) {
-            unexpectedNonOKTests.add(testname);
-          }
-          subtestTotal = results['subtests'].length;
-          for (const subtest of results['subtests']) {
-            if (subtest['status'] == 'PASS') {
-              subtestPasses += 1;
-            }
-          }
-        } else {
-          if (results['status'] == 'PASS') {
-            subtestPasses = 1;
-          }
+        if ('subtests' in results &&
+            results['status'] != 'OK' && !KNOWN_TEST_STATUSES.has(testname)) {
+          unexpectedNonOKTests.add(testname);
         }
 
-        // Add an entry to subtest passes and total for calculating the interop score.
-        const subtestCounts = testPassCounts.get(testname);
-        subtestCounts['subtestPasses'].push(subtestPasses);
-        subtestCounts['subtestTotal'].push(subtestTotal);
+        const testScore = lib.resultTrees.scoreTestResults(results);
+        testScores.get(testname).push(testScore);
 
         // A single test is scored 0-1000 based on how many of its subtests
         // pass, rounding down so that 1000 always means fully passing.
-        score += Math.floor(1000 * subtestPasses / subtestTotal);
+        score += Math.floor(1000 * testScore);
       });
       // We always normalize against the number of tests we are looking for,
       // rather than the total number of tests we found. The trade-off is all
@@ -363,7 +342,7 @@ function scoreRuns(runs, allTestsSet) {
   }
   // Calculate the interop scores that have been saved and add
   // the interop score to the end of the browsers' scores array.
-  scores.push(aggregateInteropTestScores(testPassCounts, runs.length));
+  scores.push(aggregateInteropTestScores(testScores, runs.length));
   return scores;
 }
 

lib/feature-level-interop.js

@@ -5,11 +5,6 @@
  * interop).
  */
 
-// Scores one test as the fraction of it that passed, in [0, 1].
-function scoreTestResults(results) {
-  throw new Error('scoreTestResults is not implemented');
-}
-
 // Scores one feature for each browser in |expectedBrowsers|, or undefined if no
 // run has any of its |tests|.
 function scoreFeature(runs, expectedBrowsers, tests) {
@@ -33,5 +28,4 @@ function scoreRuns(runs, expectedBrowsers, featureTestMap) {
 module.exports = {
   scoreFeature,
   scoreRuns,
-  scoreTestResults,
 };

lib/result-trees.js

@@ -10,6 +10,15 @@ const SUBTEST_PASS_STATUSES = ['PASS'];
 const SUBTEST_FAIL_STATUSES = ['FAIL', 'ERROR', 'TIMEOUT', 'NOTRUN'];
 const SUBTEST_NEUTRAL_STATUSES = ['PRECONDITION_FAILED', 'SKIP'];
 
+// Statuses a test can report at the top level; OK appears for a test with
+// subtests, and SKIP counts against the browser rather than being neutral.
+const KNOWN_TEST_STATUSES = new Set(['OK'].concat(
+    TEST_PASS_STATUSES, TEST_FAIL_STATUSES, TEST_NEUTRAL_STATUSES));
+
+// Statuses a subtest can report; only PASS counts as a pass.
+const KNOWN_SUBTEST_STATUSES = new Set(SUBTEST_PASS_STATUSES.concat(
+    SUBTEST_FAIL_STATUSES, SUBTEST_NEUTRAL_STATUSES));
+
 function splitTestPath(path) {
   // Complexity to handle /foo/bar/test.html?a/b, which can occur especially
   // with variants. decodeURIComponent needs to be used when reading.
@@ -63,6 +72,36 @@ function walkTests(tree, visitor, path='') {
   }
 }
 
+// Scores one test as the fraction of it that passed, in [0, 1].
+function scoreTestResults(results) {
+  const status = results['status'];
+  if (!KNOWN_TEST_STATUSES.has(status)) {
+    throw new Error(`Unknown test status: '${status}'`);
+  }
+
+  if (!('subtests' in results)) {
+    return status === 'PASS' ? 1 : 0;
+  }
+
+  const subtests = results['subtests'];
+  if (subtests.length === 0) {
+    return 0;
+  }
+
+  let passes = 0;
+  for (const subtest of subtests) {
+    const subtestStatus = subtest['status'];
+    if (!KNOWN_SUBTEST_STATUSES.has(subtestStatus)) {
+      throw new Error(`Unknown subtest status for '${subtest['name']}': ` +
+          `'${subtestStatus}'`);
+    }
+    if (subtestStatus === 'PASS') {
+      passes += 1;
+    }
+  }
+  return passes / subtests.length;
+}
+
 module.exports = {
   SUBTEST_FAIL_STATUSES,
   SUBTEST_NEUTRAL_STATUSES,
@@ -71,6 +110,7 @@ module.exports = {
   TEST_NEUTRAL_STATUSES,
   TEST_PASS_STATUSES,
   findTestResults,
+  scoreTestResults,
   splitTestPath,
   splitTestPathEncodedName,
   walkTests,

test/result-trees.js

@@ -91,4 +91,95 @@ describe('result-trees.js', () => {
       assert.isUndefined(resultTrees.findTestResults(tree, '/css/A.html'));
     });
   });
+
+  describe('scoreTestResults', () => {
+    it('scores a reftest that passed as one and one that timed out as zero',
+        () => {
+          assert.equal(
+              resultTrees.scoreTestResults({status: 'PASS'}), 1);
+          assert.equal(
+              resultTrees.scoreTestResults({status: 'TIMEOUT'}), 0);
+        });
+
+    it('scores a test with subtests as the fraction that passed', () => {
+      const results = {status: 'OK', subtests: [
+        {name: 'test 1', status: 'PASS'},
+        {name: 'test 2', status: 'PASS'},
+        {name: 'test 3', status: 'FAIL'},
+        {name: 'test 4', status: 'FAIL'},
+      ]};
+
+      assert.equal(resultTrees.scoreTestResults(results), 0.5);
+    });
+
+    it('ignores the harness status when the test has subtests', () => {
+      const results = {status: 'ERROR', subtests: [
+        {name: 'test 1', status: 'PASS'},
+        {name: 'test 2', status: 'FAIL'},
+      ]};
+
+      assert.equal(resultTrees.scoreTestResults(results), 0.5);
+    });
+
+    it('scores a test reporting an empty subtests array as zero', () => {
+      assert.equal(resultTrees.scoreTestResults(
+          {status: 'OK', subtests: []}), 0);
+    });
+
+    it('scores a test whose harness reported OK with no subtests as zero',
+        () => {
+          assert.equal(resultTrees.scoreTestResults({status: 'OK'}), 0);
+        });
+
+    it('counts duplicate subtest names once each, as the run reported them',
+        () => {
+          const results = {status: 'OK', subtests: [
+            {name: 'test 1', status: 'PASS'},
+            {name: 'test 1', status: 'FAIL'},
+          ]};
+
+          assert.equal(resultTrees.scoreTestResults(results), 0.5);
+        });
+
+    it('counts NOTRUN, SKIP and PRECONDITION_FAILED subtests as failures',
+        () => {
+          const results = {status: 'OK', subtests: [
+            {name: 'test 1', status: 'PASS'},
+            {name: 'test 2', status: 'NOTRUN'},
+            {name: 'test 3', status: 'SKIP'},
+            {name: 'test 4', status: 'PRECONDITION_FAILED'},
+          ]};
+
+          assert.equal(resultTrees.scoreTestResults(results), 0.25);
+        });
+
+    it('counts a top-level SKIP as a failure', () => {
+      assert.equal(resultTrees.scoreTestResults({status: 'SKIP'}), 0);
+    });
+
+    it('scores a test that timed out after its one passing subtest as a full ' +
+        'pass', () => {
+      // The harness died early, so the browser is credited with the single
+      // subtest it managed to report; see the note on scoreTestResults.
+      const results = {status: 'TIMEOUT', subtests: [
+        {name: 'test 1', status: 'PASS'},
+      ]};
+
+      assert.equal(resultTrees.scoreTestResults(results), 1);
+    });
+
+    it('throws for a test status it does not know', () => {
+      assert.throws(() => {
+        resultTrees.scoreTestResults({status: 'FOO'});
+      }, /Unknown test status: 'FOO'/);
+    });
+
+    it('throws for a subtest status it does not know', () => {
+      assert.throws(() => {
+        resultTrees.scoreTestResults({status: 'OK', subtests: [
+          {name: 'test 1', status: 'FOO'},
+        ]});
+      }, /Unknown subtest status for 'test 1': 'FOO'/);
+    });
+  });
 });