Skip to content

Commit 5da4821

Browse files
authored
Fix view=test scoring (#1239)
This change is essentially a port of these two PRs: - web-platform-tests/wpt.fyi#4290 - web-platform-tests/wpt.fyi#4294 The logic to increment TestPass comes from the earlier PRs. And missing tests should not increment subtest counts or TestPass count. It should only increment TotalTests by one for a given feature. Other fixes: - This also fixes a problem where crashed tests were being counted as passing. The unit test reduced the count for feature6. - Skip counting subtests if the test file is missing. - Add pretty printing of the output to help with debugging.
1 parent eebba80 commit 5da4821

File tree

2 files changed

+38
-12
lines changed

2 files changed

+38
-12
lines changed

workflows/steps/services/wpt_consumer/pkg/workflow/score_webfeature.go

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ const (
3737
WPTStatusCrash WPTStatusAbbreviation = "C"
3838
WPTStatusTimeout WPTStatusAbbreviation = "T"
3939
WPTStatusPreconditionFailed WPTStatusAbbreviation = "PF"
40+
WPTStatusEmpty WPTStatusAbbreviation = ""
4041
)
4142

4243
// Score calculates web feature metrics from a V2 results summary file.
@@ -90,6 +91,12 @@ func (s ResultsSummaryFileV2) scoreSubtests(
9091
if webFeatures, found = (*testToWebFeatures)[test]; !found {
9192
return
9293
}
94+
95+
// If the test has missing results, skip it.
96+
if isTestMissing(testStatus, numberofSubtests) {
97+
return
98+
}
99+
93100
// In the event of a crash, wpt records the incorrect number of subtests (but not tests).
94101
// Ignore subtest metrics for now. And mark the web feature as a whole as one to not update the subtest metrics
95102
if WPTStatusAbbreviation(testStatus) == WPTStatusCrash {
@@ -147,6 +154,24 @@ func getScoreForFeature(
147154
return score
148155
}
149156

157+
func isPassingViewTest(testStatus string, subtests, total int) bool {
158+
return isPassingViewTestStatus(testStatus) &&
159+
subtests == total && !isTestMissing(testStatus, total)
160+
}
161+
162+
func isTestMissing(testStatus string, total int) bool {
163+
return WPTStatusAbbreviation(testStatus) == WPTStatusEmpty &&
164+
total == 0
165+
}
166+
167+
func isPassingViewTestStatus(testStatus string) bool {
168+
return WPTStatusAbbreviation(testStatus) == WPTStatusOK ||
169+
WPTStatusAbbreviation(testStatus) == WPTStatusPass ||
170+
// Should handle the status == empty case.
171+
// Background: https://github.com/web-platform-tests/wpt.fyi/pull/4290
172+
WPTStatusAbbreviation(testStatus) == WPTStatusEmpty
173+
}
174+
150175
// scoreTest updates web feature metrics for a single test
151176
// based on provided subtest results and web features data.
152177
func (s ResultsSummaryFileV2) scoreTest(
@@ -165,16 +190,8 @@ func (s ResultsSummaryFileV2) scoreTest(
165190
return
166191
}
167192
// Calculate the value early so we can re-use for multiple web features.
168-
// Logic for zero subtests
169-
var countsAsPassing bool
170-
if numberofSubtests == 0 {
171-
// Determine the appropriate logic based on the status, as done in JavaScript
172-
if WPTStatusAbbreviation(testStatus) == WPTStatusOK || WPTStatusAbbreviation(testStatus) == WPTStatusPass {
173-
countsAsPassing = true // Treat as passing if status is OK or Pass
174-
}
175-
} else {
176-
countsAsPassing = numberOfSubtestPassing == numberofSubtests
177-
}
193+
countsAsPassing := isPassingViewTest(testStatus, numberOfSubtestPassing, numberofSubtests)
194+
178195
for webFeature := range webFeatures {
179196
webFeatureScore := getScoreForFeature(webFeature, webFeatureScoreMap)
180197
*webFeatureScore.TotalTests++

workflows/steps/services/wpt_consumer/pkg/workflow/score_webfeature_test.go

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,9 @@ func getComplexWebFeaturesData() shared.WebFeaturesData {
9999
"dir/tentative/test10.html": {
100100
"feature1": nil,
101101
},
102+
"test11-missing.html": {
103+
"feature1": nil,
104+
},
102105
}
103106
}
104107

@@ -159,6 +162,12 @@ func getComplexSummary() ResultsSummaryFileV2 {
159162
Status: string(WPTStatusPass),
160163
Counts: []int{111, 111},
161164
},
165+
// Missing tests should not increment subtest counts or TestPass count.
166+
// It should only increment TotalTests by one for a given feature.
167+
"test11-missing.html": query.SummaryResult{
168+
Status: string(WPTStatusEmpty),
169+
Counts: []int{0, 0},
170+
},
162171
}
163172
}
164173

@@ -189,7 +198,7 @@ func TestScore(t *testing.T) {
189198
summary: getComplexSummary(),
190199
expectedOutput: map[string]wptconsumertypes.WPTFeatureMetric{
191200
"feature1": {
192-
TotalTests: valuePtr[int64](2),
201+
TotalTests: valuePtr[int64](3),
193202
TestPass: valuePtr[int64](2),
194203
TotalSubtests: valuePtr[int64](101),
195204
SubtestPass: valuePtr[int64](101),
@@ -225,7 +234,7 @@ func TestScore(t *testing.T) {
225234
},
226235
"feature6": {
227236
TotalTests: valuePtr[int64](5),
228-
TestPass: valuePtr[int64](4),
237+
TestPass: valuePtr[int64](2),
229238
TotalSubtests: nil,
230239
SubtestPass: nil,
231240
FeatureRunDetails: map[string]interface{}{

0 commit comments

Comments
 (0)