Skip to content

Commit c657ada

Browse files
rzanerzane
authored
feat: Load configuration in .initialize() (#87)
#### Problems On main, a side-effect of importing this package is that it will attempt to parse `window.TT`. If `window.TT` is not assigned, the import will explode. Importing a module should not have side-effects. Another consequence of this configuration strategy is that we're mocking `testTrackConfig` in just about every single test to avoid it blowing up. Finally, the fact that this module relies on `window.TT` is a relic from another era where we wanted to pass data from the backend to the frontend. We should just be able to pass some configuration to a function when we initialize TestTrack. That'll be fixed in a future PR. #### Solution The solution is to retrieve the config when `initialize` is called, and pass it around. Passing it around ended up creating quite a large diff. /no-platform
1 parent 32823ad commit c657ada

26 files changed

+653
-676
lines changed

‎src/abConfiguration.test.ts‎

Lines changed: 44 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,38 +1,37 @@
11
import ABConfiguration from './abConfiguration';
2-
import SplitRegistry from './splitRegistry';
3-
import TestTrackConfig from './testTrackConfig';
42
import Visitor from './visitor';
5-
import { mockSplitRegistry } from './test-utils';
3+
import { createConfig } from './test-utils';
4+
import type { Config } from './config';
65

7-
vi.mock('./testTrackConfig');
8-
9-
function createVisitor() {
10-
const visitor = new Visitor({ id: 'visitor_id', assignments: [] });
11-
visitor.logError = vi.fn();
12-
return visitor;
13-
}
14-
15-
describe('ABConfiguration', () => {
16-
beforeEach(() => {
17-
TestTrackConfig.getSplitRegistry = mockSplitRegistry({
6+
function setupConfig() {
7+
return createConfig({
8+
splits: {
189
element: {
19-
earth: 25,
20-
wind: 25,
21-
fire: 25,
22-
water: 25
10+
feature_gate: false,
11+
weights: { earth: 25, wind: 25, fire: 25, water: 25 }
2312
},
2413
button_color: {
25-
red: 50,
26-
blue: 50
14+
feature_gate: false,
15+
weights: { red: 50, blue: 50 }
2716
},
2817
new_feature: {
29-
true: 100
18+
feature_gate: false,
19+
weights: { true: 100 }
3020
}
31-
});
21+
}
3222
});
23+
}
24+
25+
function createVisitor(config: Config) {
26+
const visitor = new Visitor({ config, id: 'visitor_id', assignments: [] });
27+
visitor.logError = vi.fn();
28+
return visitor;
29+
}
3330

31+
describe('ABConfiguration', () => {
3432
it('requires a splitName', () => {
35-
const visitor = createVisitor();
33+
const config = setupConfig();
34+
const visitor = createVisitor(config);
3635

3736
expect(() => {
3837
// @ts-expect-error Testing missing required property
@@ -41,7 +40,8 @@ describe('ABConfiguration', () => {
4140
});
4241

4342
it('requires an trueVariant', () => {
44-
const visitor = createVisitor();
43+
const config = setupConfig();
44+
const visitor = createVisitor(config);
4545

4646
expect(() => {
4747
new ABConfiguration({ splitName: 'button_color', visitor: visitor });
@@ -56,7 +56,8 @@ describe('ABConfiguration', () => {
5656
});
5757

5858
it('allows a null trueVariant', () => {
59-
const visitor = createVisitor();
59+
const config = setupConfig();
60+
const visitor = createVisitor(config);
6061

6162
expect(() => {
6263
new ABConfiguration({
@@ -70,7 +71,8 @@ describe('ABConfiguration', () => {
7071

7172
describe('#getVariants()', () => {
7273
it('logs an error if the split does not have exactly two variants', () => {
73-
const visitor = createVisitor();
74+
const config = setupConfig();
75+
const visitor = createVisitor(config);
7476
const abConfiguration = new ABConfiguration({
7577
splitName: 'element',
7678
trueVariant: 'water',
@@ -83,9 +85,9 @@ describe('ABConfiguration', () => {
8385
});
8486

8587
it('does not log an error if the split registry is not loaded', () => {
86-
vi.mocked(TestTrackConfig.getSplitRegistry).mockReturnValue(new SplitRegistry(null));
88+
const config = createConfig();
8789

88-
const visitor = createVisitor();
90+
const visitor = createVisitor(config);
8991
const abConfiguration = new ABConfiguration({
9092
splitName: 'element',
9193
trueVariant: 'water',
@@ -99,32 +101,35 @@ describe('ABConfiguration', () => {
99101

100102
describe('true variant', () => {
101103
it('is true if null was passed in during instantiation', () => {
104+
const config = setupConfig();
102105
const abConfiguration = new ABConfiguration({
103106
splitName: 'button_color',
104107
// @ts-expect-error Testing null value
105108
trueVariant: null,
106-
visitor: createVisitor()
109+
visitor: createVisitor(config)
107110
});
108111

109112
expect(abConfiguration.getVariants().true).toBe('true');
110113
});
111114

112115
it('is true if only one variant in the split', () => {
116+
const config = setupConfig();
113117
const abConfiguration = new ABConfiguration({
114118
splitName: 'new_feature',
115119
// @ts-expect-error Testing null value
116120
trueVariant: null,
117-
visitor: createVisitor()
121+
visitor: createVisitor(config)
118122
});
119123

120124
expect(abConfiguration.getVariants().true).toBe('true');
121125
});
122126

123127
it('is whatever was passed in during instantiation', () => {
128+
const config = setupConfig();
124129
const abConfiguration = new ABConfiguration({
125130
splitName: 'button_color',
126131
trueVariant: 'red',
127-
visitor: createVisitor()
132+
visitor: createVisitor(config)
128133
});
129134

130135
expect(abConfiguration.getVariants().true).toBe('red');
@@ -133,43 +138,46 @@ describe('ABConfiguration', () => {
133138

134139
describe('false variant', () => {
135140
it('is the variant of the split that is not the true_variant', () => {
141+
const config = setupConfig();
136142
const abConfiguration = new ABConfiguration({
137143
splitName: 'button_color',
138144
trueVariant: 'red',
139-
visitor: createVisitor()
145+
visitor: createVisitor(config)
140146
});
141147

142148
expect(abConfiguration.getVariants().false).toBe('blue');
143149
});
144150

145151
it('is false when there is no split_registry', () => {
146-
vi.mocked(TestTrackConfig.getSplitRegistry).mockReturnValue(new SplitRegistry(null));
152+
const config = createConfig();
147153

148154
const abConfiguration = new ABConfiguration({
149155
splitName: 'button_color',
150156
trueVariant: 'red',
151-
visitor: createVisitor()
157+
visitor: createVisitor(config)
152158
});
153159

154160
expect(abConfiguration.getVariants().false).toBe('false');
155161
});
156162

157163
it('is always the same if the split has more than two variants', () => {
164+
const config = setupConfig();
158165
const abConfiguration = new ABConfiguration({
159166
splitName: 'element',
160167
trueVariant: 'earth',
161-
visitor: createVisitor()
168+
visitor: createVisitor(config)
162169
});
163170

164171
expect(abConfiguration.getVariants().false).toBe('fire');
165172
});
166173

167174
it('is false if only one variant in the split', () => {
175+
const config = setupConfig();
168176
const abConfiguration = new ABConfiguration({
169177
splitName: 'new_feature',
170178
// @ts-expect-error Testing null value
171179
trueVariant: null,
172-
visitor: createVisitor()
180+
visitor: createVisitor(config)
173181
});
174182

175183
expect(abConfiguration.getVariants().false).toBe('false');

‎src/abConfiguration.ts‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,4 @@
1-
import TestTrackConfig from './testTrackConfig';
21
import Visitor from './visitor';
3-
import SplitRegistry from './splitRegistry';
42

53
export type ABConfigurationOptions = {
64
splitName: string;
@@ -12,7 +10,6 @@ class ABConfiguration {
1210
private _splitName: string;
1311
private _trueVariant?: string;
1412
private _visitor: Visitor;
15-
private _splitRegistry: SplitRegistry;
1613

1714
constructor(options: ABConfigurationOptions) {
1815
if (!options.splitName) {
@@ -26,7 +23,6 @@ class ABConfiguration {
2623
this._splitName = options.splitName;
2724
this._trueVariant = options.trueVariant;
2825
this._visitor = options.visitor;
29-
this._splitRegistry = TestTrackConfig.getSplitRegistry();
3026
}
3127

3228
getVariants() {
@@ -68,7 +64,7 @@ class ABConfiguration {
6864
}
6965

7066
_getSplit() {
71-
return this._splitRegistry.getSplit(this._splitName);
67+
return this._visitor.config.splitRegistry.getSplit(this._splitName);
7268
}
7369

7470
_getSplitVariants() {

‎src/api.test.ts‎

Lines changed: 20 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,16 @@
11
import { http, HttpResponse } from 'msw';
2-
import { request, toSearchParams } from './api';
2+
import { request, toSearchParams, urlFor } from './api';
33
import { server } from './setupTests';
4+
import { createConfig } from './test-utils';
45

5-
vi.mock('./testTrackConfig', () => ({
6-
default: {
7-
getUrl: vi.fn(() => 'http://testtrack.dev')
8-
}
9-
}));
6+
const url = 'https://testtrack.dev/api/v1/test';
7+
8+
describe('urlFor', () => {
9+
it('appends the path to the configured URL', () => {
10+
const config = createConfig({ url: 'http://example.org' });
11+
expect(urlFor(config, '/api/v1/foo')).toEqual(new URL('http://example.org/api/v1/foo'));
12+
});
13+
});
1014

1115
describe('toSearchParams', () => {
1216
it('constructs URLSearchParams without empty values', () => {
@@ -18,19 +22,15 @@ describe('toSearchParams', () => {
1822

1923
describe('request', () => {
2024
it('sends a GET request', async () => {
21-
server.use(
22-
http.get('http://testtrack.dev/api/v1/data', () => {
23-
return HttpResponse.json({ foo: 'bar' });
24-
})
25-
);
25+
server.use(http.get(url, () => HttpResponse.json({ foo: 'bar' })));
2626

27-
const result = await request({ method: 'GET', url: '/api/v1/data' });
27+
const result = await request({ method: 'GET', url: new URL(url) });
2828
expect(result).toEqual({ data: { foo: 'bar' } });
2929
});
3030

3131
it('sends a POST request', async () => {
3232
server.use(
33-
http.post('http://testtrack.dev/api/v1/test', async ({ request }) => {
33+
http.post(url, async ({ request }) => {
3434
const params = new URLSearchParams(await request.text());
3535
expect(params.get('foo')).toEqual('bar');
3636
return HttpResponse.text('', { status: 204 });
@@ -39,7 +39,7 @@ describe('request', () => {
3939

4040
const result = await request({
4141
method: 'POST',
42-
url: '/api/v1/test',
42+
url: new URL(url),
4343
body: new URLSearchParams({ foo: 'bar' })
4444
});
4545

@@ -48,7 +48,7 @@ describe('request', () => {
4848

4949
it('performs basic authentication', async () => {
5050
server.use(
51-
http.post('http://testtrack.dev/api/v1/test', ({ request }) => {
51+
http.post(url, ({ request }) => {
5252
const authorization = request.headers.get('authorization');
5353
expect(authorization).toEqual('Basic dXNlcjpwYXNz');
5454
return HttpResponse.json({ ok: true });
@@ -57,33 +57,23 @@ describe('request', () => {
5757

5858
const result = await request({
5959
method: 'POST',
60-
url: '/api/v1/test',
60+
url: new URL(url),
6161
auth: { username: 'user', password: 'pass' }
6262
});
6363

6464
expect(result).toEqual({ data: { ok: true } });
6565
});
6666

6767
it('throws when response is not ok', async () => {
68-
server.use(
69-
http.get('http://testtrack.dev/api/v1/test', () => {
70-
return new HttpResponse(null, { status: 500 });
71-
})
72-
);
68+
server.use(http.get(url, () => new HttpResponse(null, { status: 500 })));
7369

74-
await expect(request({ url: '/api/v1/test', method: 'GET' })).rejects.toThrow(
75-
'HTTP request failed with 500 status'
76-
);
70+
await expect(request({ url: new URL(url), method: 'GET' })).rejects.toThrow('HTTP request failed with 500 status');
7771
});
7872

7973
it('aborts request after timeout', async () => {
80-
server.use(
81-
http.get('http://testtrack.dev/api/v1/slow', async () => {
82-
await new Promise(resolve => setTimeout(resolve, 100));
83-
})
84-
);
74+
server.use(http.get(url, () => new Promise(resolve => setTimeout(resolve, 100))));
8575

86-
await expect(request({ url: '/api/v1/slow', method: 'GET', timeout: 5 })).rejects.toThrow(
76+
await expect(request({ url: new URL(url), method: 'GET', timeout: 5 })).rejects.toThrow(
8777
'This operation was aborted'
8878
);
8979
});

‎src/api.ts‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1-
import TestTrackConfig from './testTrackConfig';
1+
import type { Config } from './config';
22

33
type RequestOptions = {
4-
url: `/api/${string}`;
4+
url: URL;
55
method: 'GET' | 'POST';
66
timeout?: number;
77
body?: URLSearchParams;
@@ -11,6 +11,10 @@ type RequestOptions = {
1111
// eslint-disable-next-line @typescript-eslint/no-explicit-any
1212
type Result = { data: any };
1313

14+
export function urlFor(config: Config, path: `/api/${string}`): URL {
15+
return new URL(path, config.url);
16+
}
17+
1418
export function toSearchParams(values: Record<string, string | null | undefined>): URLSearchParams {
1519
const params = new URLSearchParams();
1620

@@ -24,8 +28,6 @@ export function toSearchParams(values: Record<string, string | null | undefined>
2428
}
2529

2630
export async function request(options: RequestOptions): Promise<Result> {
27-
const url = new URL(options.url, TestTrackConfig.getUrl());
28-
2931
const controller = new AbortController();
3032
const timeout = setTimeout(() => controller.abort(), options.timeout ?? 60_000);
3133

@@ -40,7 +42,7 @@ export async function request(options: RequestOptions): Promise<Result> {
4042
headers.append('Authorization', `Basic ${credential}`);
4143
}
4244

43-
const response = await fetch(url, {
45+
const response = await fetch(options.url, {
4446
method: options.method,
4547
body: options.body,
4648
headers,

‎src/assignmentNotification.test.ts‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,10 @@ import AssignmentNotification from './assignmentNotification';
33
import Visitor from './visitor';
44
import { http, HttpResponse } from 'msw';
55
import { server, requests } from './setupTests';
6-
7-
vi.mock('./testTrackConfig', () => {
8-
return {
9-
default: {
10-
getUrl: () => 'http://testtrack.dev'
11-
}
12-
};
13-
});
6+
import { createConfig } from './test-utils';
147

158
function createVisitor(options: { trackSuccess: boolean }) {
16-
const visitor = new Visitor({ id: 'visitorId', assignments: [] });
9+
const visitor = new Visitor({ config: createConfig(), id: 'visitorId', assignments: [] });
1710

1811
visitor.setAnalytics({
1912
identify: vi.fn(),

0 commit comments

Comments
 (0)