Skip to content

Commit 8b6f76b

Browse files
authored
Don't serve ab tests set to off (#14684)
* Don't serve ab tests set to off * make test more explicit * don't upload inactive tests to fastly * remove expiry method * metrics * frontend
1 parent 911a873 commit 8b6f76b

File tree

8 files changed

+22
-14
lines changed

8 files changed

+22
-14
lines changed

ab-testing/README.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,10 @@ To add a test where there is not enough space in the default audience space (`A`
6363

6464
For example if there are already 3 25% tests in space `A` totalling 75%, and you want to run a 50% test, you can set the `audienceSpace` to `B` to allow this test to overlap with the existing tests.
6565

66+
### Test Status
67+
68+
Tests can be set to `ON` or `OFF` using the `status` field. Only tests with status `ON` will be validated and deployed.
69+
6670
## How it works
6771

6872
The AB testing framework uses Deno to run scripts that validate and deploy the tests. The `deno.json` file contains the tasks that can be run, such as `validate`, `deploy`, and `build`.

ab-testing/abTest.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,4 +19,8 @@ import type { ABTest } from './types';
1919
* - 100% Test variant MVT 500-999
2020
*/
2121

22-
export const ABTests: ABTest[] = [];
22+
const ABTests: ABTest[] = [];
23+
24+
const activeABtests = ABTests.filter((test) => test.status === 'ON');
25+
26+
export { ABTests as allABTests, activeABtests };

ab-testing/frontend/src/routes/+page.svelte

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<script lang="ts">
2-
import { ABTests } from '../../../abTest';
2+
import { allABTests, activeABtests } from '../../../abTest';
33
import Table from '$lib/components/TableFixed.svelte';
44
import AudienceBreakdown from '$lib/components/AudienceBreakdown.svelte';
55
</script>
@@ -31,8 +31,8 @@
3131
</p>
3232
</section>
3333
<section>
34-
<AudienceBreakdown tests={ABTests} />
35-
<Table tests={ABTests} />
34+
<AudienceBreakdown tests={activeABtests} />
35+
<Table tests={allABTests} />
3636
</section>
3737

3838
<style>

ab-testing/index.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
// @ts-ignore - extension is required to import this as a package in DCR
2-
import { ABTests } from './abTest.ts';
2+
import { ABTests, activeABtests } from './abTest.ts';
33

4-
export { ABTests };
4+
export { ABTests, activeABtests };

ab-testing/scripts/build/index.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
1-
import { ABTests } from '../../abTest.ts';
21
import { getMVTGroupsFromDictionary } from '../lib/fastly-api.ts';
32
import { buildABTestGroupKeyValues } from './build-ab-tests-dict.ts';
43
import { parseArgs } from 'jsr:@std/cli/parse-args';
54
import { calculateAllSpaceUpdates } from './calculate-mvt-updates.ts';
65
import { parseMVTValue, stringifyMVTValue } from '../lib/fastly-subfield.ts';
76
import { dirname } from 'jsr:@std/path';
7+
import { activeABtests } from '../../abTest.ts';
88

99
const flags = parseArgs(Deno.args, {
1010
string: ['mvts', 'ab-tests'],
@@ -26,9 +26,9 @@ const mvtGroups = new Map(
2626
}),
2727
);
2828

29-
const abTestGroupKeyValues = buildABTestGroupKeyValues(ABTests);
29+
const abTestGroupKeyValues = buildABTestGroupKeyValues(activeABtests);
3030

31-
const mvtIdKeyValues = calculateAllSpaceUpdates(mvtGroups, ABTests);
31+
const mvtIdKeyValues = calculateAllSpaceUpdates(mvtGroups, activeABtests);
3232

3333
const mvtDictArray = Array.from(
3434
mvtIdKeyValues.entries().map(([key, value]) => ({

ab-testing/scripts/validation/index.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
import { ABTests } from '../../abTest.ts';
21
import { ABTest } from '../../types.ts';
2+
import { activeABtests } from '../../abTest.ts';
33
import { enoughSpace } from './enoughSpace.ts';
44
import { limitServerSideTests } from './limitServerSide.ts';
55
import { uniqueName } from './uniqueName.ts';
@@ -19,7 +19,7 @@ function validateTests(testList: ABTest[]) {
1919
}
2020

2121
try {
22-
validateTests(ABTests);
22+
validateTests(activeABtests);
2323
console.log('AB test validations passed');
2424
} catch (err) {
2525
const error = err as Error;

ab-testing/scripts/validation/validExpiration.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ export function allExpirationsValid(tests: ABTest[]): boolean {
1212
throw new Error(
1313
`${
1414
test.name
15-
} has an expiration date in the past: ${expires.toISOString()}, has it expired?`,
15+
} has an expiration date in the past: ${expires.toISOString()}, has it expired? If it doesn't belong to you or your team, you can set the status to OFF for now.`,
1616
);
1717
});
1818
}

dotcom-rendering/src/components/Metrics.importable.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type { ABTest, ABTestAPI } from '@guardian/ab-core';
2-
import { ABTests } from '@guardian/ab-testing';
2+
import { activeABtests } from '@guardian/ab-testing';
33
import {
44
bypassCommercialMetricsSampling,
55
EventTimer,
@@ -33,7 +33,7 @@ const clientSideTestsToForceMetrics: ABTest[] = [
3333
];
3434

3535
const shouldCollectMetricsForBetaTests = (userTestParticipations: string[]) => {
36-
const userParticipationConfigs = ABTests.filter((test) =>
36+
const userParticipationConfigs = activeABtests.filter((test) =>
3737
userTestParticipations.includes(test.name),
3838
);
3939
return userParticipationConfigs.some(

0 commit comments

Comments
 (0)