Skip to content

Commit 1cb07ff

Browse files
author
DanielRyanSmith
committed
Changes suggested by @jcscottiii
1 parent a4042eb commit 1cb07ff

File tree

3 files changed

+12
-30
lines changed

3 files changed

+12
-30
lines changed

backend/pkg/httpserver/list_missing_one_implementation_counts_test.go

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -325,29 +325,15 @@ func TestListMissingOneImplementationCounts(t *testing.T) {
325325
"startAt=2000-01-01&endAt=2000-01-10&page_token"+*badPageToken, nil),
326326
},
327327
{
328-
name: "400 case - no matching mobile browser",
329-
mockConfig: &MockListMissingOneImplCountsConfig{
330-
expectedTargetBrowsers: []string{"chrome"},
331-
expectedOtherBrowsers: [][]string{
332-
{"edge"},
333-
{"firefox"},
334-
{"safari"},
335-
},
336-
expectedStartAt: time.Date(2000, time.January, 1, 0, 0, 0, 0, time.UTC),
337-
expectedEndAt: time.Date(2000, time.January, 10, 0, 0, 0, 0, time.UTC),
338-
expectedPageSize: 100,
339-
expectedPageToken: badPageToken,
340-
page: nil,
341-
pageToken: nil,
342-
err: backendtypes.ErrInvalidPageToken,
343-
},
328+
name: "400 case - no matching mobile browser",
329+
mockConfig: nil,
344330
expectedGetCalls: []*ExpectedGetCall{
345331
{
346332
Key: `ListMissingOneImplementationCounts-{"browser":"edge","Params":{"startAt":"2000-01-01",` +
347333
`"endAt":"2000-01-10","include_baseline_mobile_browsers":true,` +
348334
`"browser":["chrome","firefox","safari"]}}`,
349335
Value: nil,
350-
Err: ErrNoMatchingMobileBrowser,
336+
Err: cachetypes.ErrCachedDataNotFound,
351337
},
352338
},
353339
expectedCacheCalls: nil,

lib/gcpspanner/missing_one_implementation.go

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -104,9 +104,9 @@ SELECT releases.EventReleaseDate,
104104
AND obsf.EventReleaseDate = tbuf.EventReleaseDate
105105
WHERE
106106
tbuf.EventReleaseDate = releases.EventReleaseDate
107-
{{ range $releaseDateFilter := .ReleaseDateFilters }}
107+
{{ range $browserParamName := .OtherBrowsersParamNames }}
108108
AND
109-
({{ $releaseDateFilter }})
109+
@{{ $browserParamName }} IN UNNEST(obsf.SupportedBrowsers)
110110
{{ end }}
111111
) AS Count
112112
FROM (
@@ -163,7 +163,7 @@ LIMIT @limit;
163163

164164
type missingOneImplTemplateData struct {
165165
ReleaseDateParam string
166-
ReleaseDateFilters []string
166+
OtherBrowsersParamNames []string
167167
TargetBrowserFilter string
168168
OtherBrowsersFilters []string
169169
ExcludedFeatureFilter string
@@ -219,7 +219,7 @@ func buildMissingOneImplTemplate(
219219

220220
targetBrowserConditions := make([]string, 0, len(targetBrowsers))
221221
for i, browserName := range targetBrowsers {
222-
paramName := fmt.Sprintf("targetBrowserParam%d", i) // Create a unique param name, e.g., targetBrowserParam0
222+
paramName := fmt.Sprintf("targetBrowserParam%d", i)
223223
targetBrowserConditions[i] = fmt.Sprintf("bfse.TargetBrowserName = @%s", paramName)
224224
params[paramName] = browserName
225225
}
@@ -233,22 +233,18 @@ func buildMissingOneImplTemplate(
233233
params[releaseDateParamName] = cursor.ReleaseDate
234234
}
235235

236+
var allOtherBrowsersParams []string
236237
otherBrowsersFilters := make([]string, 0, len(otherBrowsers))
237-
releaseDateFilters := make([]string, 0, len(otherBrowsers))
238238
for i, otherBrowserGroup := range otherBrowsers {
239+
allOtherBrowsersParams = append(
240+
allOtherBrowsersParams, otherBrowserGroup...)
239241
otherBrowsersConditions := make([]string, 0, len(otherBrowserGroup))
240-
releaseDateConditions := make([]string, 0, len(otherBrowserGroup))
241242
for j, browserName := range otherBrowserGroup {
242243
paramName := fmt.Sprintf("otherBrowsersParam%d-%d", i, j)
243244
otherBrowsersConditions[j] = fmt.Sprintf("bfse_other.TargetBrowserName = @%s", paramName)
244245
params[paramName] = browserName
245-
246-
paramName = fmt.Sprintf("releaseDateParam%d-%d", i, j)
247-
releaseDateConditions[j] = fmt.Sprintf("%s IN UNNEST(obsf.SupportedBrowsers)", paramName)
248-
params[paramName] = browserName
249246
}
250247
otherBrowsersFilters[i] = fmt.Sprintf("(%s)", strings.Join(otherBrowsersConditions, " AND "))
251-
releaseDateFilters[i] = fmt.Sprintf("(%s)", strings.Join(releaseDateConditions, " AND "))
252248
}
253249

254250
var excludedFeatureFilter, otherExcludedFeatureFilter string
@@ -263,7 +259,7 @@ func buildMissingOneImplTemplate(
263259

264260
tmplData := missingOneImplTemplateData{
265261
ReleaseDateParam: releaseDateParamName,
266-
ReleaseDateFilters: releaseDateFilters,
262+
OtherBrowsersParamNames: allOtherBrowsersParams,
267263
TargetBrowserFilter: targetBrowserFilter,
268264
OtherBrowsersFilters: otherBrowsersFilters,
269265
ExcludedFeatureFilter: excludedFeatureFilter,

lib/gcpspanner/missing_one_implementation_feature_list.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ func buildMissingOneImplFeatureListTemplate(
113113
params := map[string]interface{}{}
114114
targetBrowserConditions := make([]string, 0, len(targetBrowsers))
115115
for i, browserName := range targetBrowsers {
116-
paramName := fmt.Sprintf("targetBrowserParam%d", i) // Create a unique param name, e.g., targetBrowserParam0
116+
paramName := fmt.Sprintf("targetBrowserParam%d", i)
117117
targetBrowserConditions[i] = fmt.Sprintf("bfse.TargetBrowserName = @%s", paramName)
118118
params[paramName] = browserName
119119
}

0 commit comments

Comments
 (0)