diff --git a/.github/workflows/generate-test-seed.yml b/.github/workflows/generate-test-seed.yml index 53ad4e0c..58e42f5e 100644 --- a/.github/workflows/generate-test-seed.yml +++ b/.github/workflows/generate-test-seed.yml @@ -2,12 +2,14 @@ name: Generate Test Seed on: pull_request: + types: [opened, synchronize, reopened, labeled] paths: - '.github/workflows/generate-test-seed.yml' - 'studies/**' jobs: build: + if: github.event.action != 'labeled' || github.event.label.name == 'generate-seed' runs-on: ubuntu-latest env: ACTION_RUN_URL: '${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}' diff --git a/src/finch_tracker/tracker_lib.test.ts b/src/finch_tracker/tracker_lib.test.ts index 8036cde5..d2cf1641 100644 --- a/src/finch_tracker/tracker_lib.test.ts +++ b/src/finch_tracker/tracker_lib.test.ts @@ -9,6 +9,7 @@ import * as os from 'os'; import { describe, expect, test } from '@jest/globals'; import JSON5 from 'json5'; import path from 'path'; +import { asPosix } from '../base/path_utils'; import { StudyPriority } from '../core/study_processor'; import { ItemAction, makeSummary, summaryToJson } from '../core/summary'; import { Study, Study_Channel, Study_Platform } from '../proto/generated/study'; @@ -18,11 +19,12 @@ import { storeDataToDirectory } from './tracker_lib'; function readDirectory(dir: string): Record { const files = fs .readdirSync(dir, { recursive: true, encoding: 'utf-8' }) - .sort(); + .sort() + .map(asPosix); const result: Record = {}; for (const file of files) { - const filePath = path.join(dir, file); + const filePath = `${dir}/${file}`; if (!file.endsWith('.json5')) { continue; } diff --git a/src/seed_tools/utils/studies_to_seed.ts b/src/seed_tools/utils/studies_to_seed.ts index b6712d55..a4df49f2 100644 --- a/src/seed_tools/utils/studies_to_seed.ts +++ b/src/seed_tools/utils/studies_to_seed.ts @@ -6,7 +6,7 @@ import { execSync } from 'child_process'; import { promises as fs } from 'fs'; import * as path from 'path'; -import { wsPath } from 'src/base/path_utils'; +import { asPosix, wsPath } from 'src/base/path_utils'; import { Study_ActivationType, Study_Consistency, @@ -36,6 +36,10 @@ export async function readStudiesToSeed( layers: [], }; + // Keep country filters in both uppercase and lowercase. This is a workaround + // to support x-country header returning uppercase values. + duplicateCountriesAsLowercase(variationsSeed); + errors.push( ...seed_validation.getSeedErrors(variationsSeed, studyFileBaseNameMap), ); @@ -72,7 +76,7 @@ async function readStudiesAtRevision( errors: string[]; }> { const basePath = wsPath('//'); - studiesDir = path.relative(basePath, studiesDir); + studiesDir = asPosix(path.relative(basePath, studiesDir)); // Validate revision format. if (!/^[a-z0-9]+$/.test(revision) && revision !== 'HEAD') { @@ -190,3 +194,24 @@ function setStudyDefaultParameters(study: Study) { study.consistency = Study_Consistency.PERMANENT; } } + +function duplicateCountriesAsLowercase(variationsSeed: VariationsSeed) { + // For now duplicate the country filters to lowercase. + const duplicate = (countries: string[]) => { + const countrySet = new Set(countries); + const allCountries = [...countries]; + for (const country of countries) { + if (!countrySet.has(country.toLowerCase())) { + allCountries.push(country.toLowerCase()); + } + } + return allCountries; + }; + for (const study of variationsSeed.study) { + if (study.filter === undefined) { + continue; + } + study.filter.country = duplicate(study.filter.country); + study.filter.exclude_country = duplicate(study.filter.exclude_country); + } +} diff --git a/src/seed_tools/utils/study_validation.ts b/src/seed_tools/utils/study_validation.ts index 8f27905d..828bffea 100644 --- a/src/seed_tools/utils/study_validation.ts +++ b/src/seed_tools/utils/study_validation.ts @@ -21,6 +21,7 @@ export function getStudyErrors(study: Study, fileBaseName: string): string[] { checkVersionRange, checkOsVersionRange, checkChannelAndPlatform, + checkCountry, ]; for (const validator of validators) { try { @@ -343,3 +344,27 @@ export function validateName( } return true; } + +function checkCountry(study: Study): string[] { + const errors: string[] = []; + if (study.filter === undefined) { + return errors; + } + + // check if country in uppercase also exists in lowercase + const checkCountriesCase = (countries: string[]) => { + const countrySet = new Set(countries); + for (const country of countries) { + if (!countrySet.has(country.toLowerCase())) { + errors.push( + `Country ${country} in lowercase is required for study ${study.name}`, + ); + } + } + }; + + checkCountriesCase(study.filter.country); + checkCountriesCase(study.filter.exclude_country); + + return errors; +} diff --git a/src/test/data/perf_seeds/legacy_seed/expected_seed.json b/src/test/data/perf_seeds/legacy_seed/expected_seed.json index 436cf9ac..acaf7232 100644 --- a/src/test/data/perf_seeds/legacy_seed/expected_seed.json +++ b/src/test/data/perf_seeds/legacy_seed/expected_seed.json @@ -604,7 +604,18 @@ "MX", "BR", "AR", - "IN" + "IN", + "ca", + "de", + "fr", + "gb", + "us", + "at", + "es", + "mx", + "br", + "ar", + "in" ] } }, @@ -637,7 +648,17 @@ "MX", "BR", "AR", - "IN" + "IN", + "ca", + "de", + "fr", + "gb", + "at", + "es", + "mx", + "br", + "ar", + "in" ] } }, diff --git a/src/test/data/valid_seeds/country_case/expected_seed.json b/src/test/data/valid_seeds/country_case/expected_seed.json new file mode 100644 index 00000000..1daee9a2 --- /dev/null +++ b/src/test/data/valid_seeds/country_case/expected_seed.json @@ -0,0 +1,63 @@ +{ + "serial_number": "1", + "study": [ + { + "name": "TestStudy", + "consistency": "PERMANENT", + "experiment": [ + { + "name": "Enabled", + "probability_weight": 100, + "feature_association": { + "enable_feature": [ + "TestFeature" + ] + } + } + ], + "filter": { + "min_version": "92.1.30.57", + "channel": [ + "CANARY" + ], + "platform": [ + "PLATFORM_WINDOWS" + ], + "country": [ + "US", + "us" + ] + }, + "activation_type": "ACTIVATE_ON_STARTUP" + }, + { + "name": "TestStudy", + "consistency": "PERMANENT", + "experiment": [ + { + "name": "Enabled", + "probability_weight": 100, + "feature_association": { + "enable_feature": [ + "TestFeature" + ] + } + } + ], + "filter": { + "min_version": "92.1.30.57", + "channel": [ + "BETA" + ], + "platform": [ + "PLATFORM_WINDOWS" + ], + "country": [ + "us" + ] + }, + "activation_type": "ACTIVATE_ON_STARTUP" + } + ], + "version": "1" +} diff --git a/src/test/data/valid_seeds/country_case/studies/TestStudy.json5 b/src/test/data/valid_seeds/country_case/studies/TestStudy.json5 new file mode 100644 index 00000000..b4e89e6f --- /dev/null +++ b/src/test/data/valid_seeds/country_case/studies/TestStudy.json5 @@ -0,0 +1,54 @@ +[ + { + name: 'TestStudy', + experiment: [ + { + name: 'Enabled', + probability_weight: 100, + feature_association: { + enable_feature: [ + 'TestFeature', + ], + }, + }, + ], + filter: { + min_version: '92.1.30.57', + channel: [ + 'NIGHTLY', + ], + platform: [ + 'WINDOWS', + ], + country: [ + 'US', + ], + }, + }, + { + name: 'TestStudy', + experiment: [ + { + name: 'Enabled', + probability_weight: 100, + feature_association: { + enable_feature: [ + 'TestFeature', + ], + }, + }, + ], + filter: { + min_version: '92.1.30.57', + channel: [ + 'BETA', + ], + platform: [ + 'WINDOWS', + ], + country: [ + 'us', + ], + }, + }, +]