Skip to content

Commit 4c93ee2

Browse files
authored
Eliminate possibility of race condition when getting participations for new ab testing framework (#14663)
* eliminate possibility of race condition for new ab-testing * update test
1 parent 4eb3afd commit 4c93ee2

File tree

4 files changed

+20
-27
lines changed

4 files changed

+20
-27
lines changed

dotcom-rendering/src/client/abTesting.ts

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,18 +29,22 @@ const getClientParticipations = (): ABParticipations => {
2929

3030
return {};
3131
};
32-
const initABTesting = (): void => {
33-
const { serverSideABTests } = window.guardian.config;
3432

35-
const clientSideABTests = getClientParticipations();
36-
37-
const participations = {
38-
...clientSideABTests,
39-
...serverSideABTests,
33+
/**
34+
* Get all AB test participations, client and server side
35+
*/
36+
const getABTestParticipations = (): ABParticipations => {
37+
return {
38+
...getClientParticipations(),
39+
...window.guardian.config.serverSideABTests,
4040
};
41+
};
42+
43+
const initWindowABTesting = (): void => {
44+
const participations = getABTestParticipations();
4145

4246
window.guardian.modules.abTests = {
43-
getParticipations: () => participations,
47+
getParticipations: getABTestParticipations,
4448
isUserInTest: (testId: string) => {
4549
return !isUndefined(participations[testId]);
4650
},
@@ -50,4 +54,4 @@ const initABTesting = (): void => {
5054
};
5155
};
5256

53-
export { initABTesting };
57+
export { initWindowABTesting, getABTestParticipations };

dotcom-rendering/src/client/main.web.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ void (async () => {
6161
'abTesting',
6262
() =>
6363
import(/* webpackMode: 'eager' */ './abTesting').then(
64-
({ initABTesting }) => initABTesting(),
64+
({ initWindowABTesting }) => initWindowABTesting(),
6565
),
6666
{ priority: 'critical' },
6767
);

dotcom-rendering/src/experiments/lib/beta-ab-tests.test.ts

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -7,21 +7,10 @@ jest.mock('@guardian/libs', () => ({
77

88
// Mock window.guardian
99
const mockGetParticipations = jest.fn();
10-
const mockWindow = {
11-
guardian: {
12-
modules: {
13-
abTests: {
14-
getParticipations: mockGetParticipations,
15-
},
16-
},
17-
},
18-
};
19-
20-
// Set up window mock
21-
Object.defineProperty(global, 'window', {
22-
value: mockWindow,
23-
writable: true,
24-
});
10+
11+
jest.mock('../../client/abTesting', () => ({
12+
getABTestParticipations: () => mockGetParticipations(),
13+
}));
2514

2615
describe('BetaABTests', () => {
2716
let betaABTests: BetaABTests;

dotcom-rendering/src/experiments/lib/beta-ab-tests.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { isUndefined } from '@guardian/libs';
2+
import { getABTestParticipations } from '../../client/abTesting';
23

34
export interface BetaABTestAPI {
45
getParticipations: () => ABParticipations;
@@ -55,8 +56,7 @@ export class BetaABTests implements BetaABTestAPI {
5556
if (isServer) {
5657
this.participations = serverSideABTests;
5758
} else {
58-
this.participations =
59-
window.guardian.modules.abTests?.getParticipations() ?? {};
59+
this.participations = getABTestParticipations();
6060
}
6161
}
6262

0 commit comments

Comments
 (0)