From f0bdd17b792181ef848bbcfacf47a14be41a63b2 Mon Sep 17 00:00:00 2001 From: Yulun Wu Date: Fri, 21 Aug 2026 18:04:54 -0700 Subject: [PATCH] Add per-test scoring to lib/result-trees.js Scores a test as the fraction of its subtests that passed. interop-scoring already counted passing subtests inline, so it calls the shared function now, which also closes its TODO asking for unrecognized statuses to be caught rather than counted as failures. --- interop-scoring/main.js | 61 ++++++++---------------- lib/feature-level-interop.js | 6 --- lib/result-trees.js | 40 ++++++++++++++++ test/result-trees.js | 91 ++++++++++++++++++++++++++++++++++++ 4 files changed, 151 insertions(+), 47 deletions(-) diff --git a/interop-scoring/main.js b/interop-scoring/main.js index 2da22b8b..54574857 100644 --- a/interop-scoring/main.js +++ b/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; } diff --git a/lib/feature-level-interop.js b/lib/feature-level-interop.js index 970352f5..85a77db4 100644 --- a/lib/feature-level-interop.js +++ b/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, }; diff --git a/lib/result-trees.js b/lib/result-trees.js index a3d464a7..9d50c503 100644 --- a/lib/result-trees.js +++ b/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, diff --git a/test/result-trees.js b/test/result-trees.js index 8ed124bb..45823dcd 100644 --- a/test/result-trees.js +++ b/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'/); + }); + }); });