Skip to content

Commit df9a777

Browse files
Zaimwa9Zaimwa9
authored
fix(e2e): staging E2E failures from shared-project pollution and onboarding flag race (#8027)
1 parent 2c000a2 commit df9a777

3 files changed

Lines changed: 114 additions & 84 deletions

File tree

‎frontend/e2e/tests/deep-link-test.pw.ts‎

Lines changed: 79 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { test, expect } from '../test-setup'
22
import { createHelpers, LONG_TIMEOUT, log } from '../helpers'
3-
import { E2E_USER, PASSWORD, E2E_TEST_PROJECT } from '../config'
3+
import { E2E_USER, PASSWORD } from '../config'
44
import Project from '../../common/project'
55

66
const PAGE_SIZE = 50
@@ -20,7 +20,6 @@ test.describe('Deep link to feature slideout', () => {
2020
const { login } = createHelpers(page)
2121
const api = Project.api
2222

23-
// Given - an authenticated API session and a project with > PAGE_SIZE features
2423
log('Authenticate against the API')
2524
const loginRes = await request.post(`${api}auth/login/`, {
2625
data: { email: E2E_USER, password: PASSWORD },
@@ -29,86 +28,93 @@ test.describe('Deep link to feature slideout', () => {
2928
const { key } = await loginRes.json()
3029
const headers = { Authorization: `Token ${key}` }
3130

32-
const project = unwrap<{ id: number; name: string }>(
33-
await (await request.get(`${api}projects/`, { headers })).json(),
34-
).find((p) => p.name === E2E_TEST_PROJECT)!
35-
expect(project).toBeTruthy()
31+
// Unique names per run so the flakiness check (`E2E_REPEAT` / `/e2e N`) does
32+
// not collide with entities created by a previous iteration.
33+
const runId = Date.now()
3634

37-
const environment = unwrap<{ id: number; name: string; api_key: string }>(
38-
await (
39-
await request.get(`${api}environments/?project=${project.id}`, {
40-
headers,
41-
})
42-
).json(),
43-
).find((e) => e.name === 'Development')!
44-
expect(environment).toBeTruthy()
35+
log('Create a dedicated project and environment')
36+
// The seeded organisation from e2e_seed_data.py; other orgs may appear
37+
// concurrently (e.g. the versioning test creates one), so match by name.
38+
const organisation = unwrap<{ id: number; name: string }>(
39+
await (await request.get(`${api}organisations/`, { headers })).json(),
40+
).find((o) => o.name === 'Bullet Train Ltd')!
41+
expect(organisation).toBeTruthy()
42+
43+
// The seeded org is at its subscription's project cap; the E2E auth token
44+
// header marks the request as E2E so the cap check is bypassed.
45+
const e2eToken =
46+
process.env.E2E_TEST_TOKEN ??
47+
process.env[`E2E_TEST_TOKEN_${Project.env.toUpperCase()}`] ??
48+
''
49+
const projectRes = await request.post(`${api}projects/`, {
50+
data: {
51+
name: `Deep Link Project ${runId}`,
52+
organisation: organisation.id,
53+
},
54+
headers: { ...headers, 'X-E2E-Test-Auth-Token': e2eToken.trim() },
55+
})
56+
expect(projectRes.ok()).toBeTruthy()
57+
const project = (await projectRes.json()) as { id: number }
58+
59+
const environmentRes = await request.post(`${api}environments/`, {
60+
data: { name: 'Development', project: project.id },
61+
headers,
62+
})
63+
expect(environmentRes.ok()).toBeTruthy()
64+
const environment = (await environmentRes.json()) as {
65+
id: number
66+
api_key: string
67+
}
4568

4669
log(`Create ${FEATURE_COUNT} features`)
47-
// Unique names per run so the flakiness check (`E2E_REPEAT` / `/e2e N`) does
48-
// not collide with features created by a previous iteration.
49-
const runId = Date.now()
5070
const featureName = (i: number) =>
5171
`${FEATURE_PREFIX}${runId}_${String(i).padStart(3, '0')}`
52-
const createdFeatureIds: number[] = []
53-
54-
try {
55-
const created = await Promise.all(
56-
Array.from({ length: FEATURE_COUNT }, (_, i) =>
57-
request.post(`${api}projects/${project.id}/features/`, {
58-
data: { name: featureName(i) },
59-
headers,
60-
}),
61-
),
62-
)
63-
for (const res of created) {
64-
expect(res.ok()).toBeTruthy()
65-
createdFeatureIds.push((await res.json()).id)
66-
}
72+
const created = await Promise.all(
73+
Array.from({ length: FEATURE_COUNT }, (_, i) =>
74+
request.post(`${api}projects/${project.id}/features/`, {
75+
data: { name: featureName(i) },
76+
headers,
77+
}),
78+
),
79+
)
80+
for (const res of created) {
81+
expect(res.ok()).toBeTruthy()
82+
}
6783

68-
// The list renders sorted by name ascending, so page 1 holds the first
69-
// PAGE_SIZE features and a feature on page 2 never mounts a row on page 1.
70-
const listUrl = (pageNumber: number) =>
71-
`${api}projects/${project.id}/features/?environment=${environment.id}&page=${pageNumber}&page_size=${PAGE_SIZE}&sort_field=name&sort_direction=ASC`
72-
const page1 = await (await request.get(listUrl(1), { headers })).json()
73-
const page2 = await (await request.get(listUrl(2), { headers })).json()
74-
const onPageFeature = page1.results[0] as { id: number; name: string }
75-
const offPageFeature = page2.results[0] as { id: number; name: string }
76-
expect(onPageFeature).toBeTruthy()
77-
expect(offPageFeature).toBeTruthy()
78-
log(`On-page: ${onPageFeature.name}, off-page: ${offPageFeature.name}`)
84+
// The list renders sorted by name ascending, so page 1 holds the first
85+
// PAGE_SIZE features and a feature on page 2 never mounts a row on page 1.
86+
const listUrl = (pageNumber: number) =>
87+
`${api}projects/${project.id}/features/?environment=${environment.id}&page=${pageNumber}&page_size=${PAGE_SIZE}&sort_field=name&sort_direction=ASC`
88+
const page1 = await (await request.get(listUrl(1), { headers })).json()
89+
const page2 = await (await request.get(listUrl(2), { headers })).json()
90+
const onPageFeature = page1.results[0] as { id: number; name: string }
91+
const offPageFeature = page2.results[0] as { id: number; name: string }
92+
expect(onPageFeature).toBeTruthy()
93+
expect(offPageFeature).toBeTruthy()
94+
log(`On-page: ${onPageFeature.name}, off-page: ${offPageFeature.name}`)
7995

80-
await login(E2E_USER, PASSWORD)
81-
const featuresPath = `/project/${project.id}/environment/${environment.api_key}/features`
82-
const slideout = page.locator('.create-feature-modal')
96+
await login(E2E_USER, PASSWORD)
97+
const featuresPath = `/project/${project.id}/environment/${environment.api_key}/features`
98+
const slideout = page.locator('.create-feature-modal')
8399

84-
// When/Then - this is the #7652 regression: a deep link to a feature that
85-
// is NOT on the first page previously rendered the list without opening
86-
// any modal, because the deep-link handler only fired for mounted rows.
87-
await page.goto(`${featuresPath}?feature=${offPageFeature.id}&tab=value`)
88-
await expect(slideout).toBeVisible({ timeout: LONG_TIMEOUT })
89-
await expect(slideout).toContainText(offPageFeature.name)
100+
// When/Then - this is the #7652 regression: a deep link to a feature that
101+
// is NOT on the first page previously rendered the list without opening
102+
// any modal, because the deep-link handler only fired for mounted rows.
103+
await page.goto(`${featuresPath}?feature=${offPageFeature.id}&tab=value`)
104+
await expect(slideout).toBeVisible({ timeout: LONG_TIMEOUT })
105+
await expect(slideout).toContainText(offPageFeature.name)
90106

91-
// And - the existing on-page deep link still works (no regression). A
92-
// fresh navigation reloads the page, dismissing the previous slideout.
93-
await page.goto(`${featuresPath}?feature=${onPageFeature.id}&tab=value`)
94-
await expect(slideout).toBeVisible({ timeout: LONG_TIMEOUT })
95-
await expect(slideout).toContainText(onPageFeature.name)
107+
// And - the existing on-page deep link still works (no regression). A
108+
// fresh navigation reloads the page, dismissing the previous slideout.
109+
await page.goto(`${featuresPath}?feature=${onPageFeature.id}&tab=value`)
110+
await expect(slideout).toBeVisible({ timeout: LONG_TIMEOUT })
111+
await expect(slideout).toContainText(onPageFeature.name)
96112

97-
// And - an unknown feature id degrades gracefully (no slideout, no crash).
98-
await page.goto(`${featuresPath}?feature=999999999&tab=value`)
99-
await expect(page.locator('[data-test="features-page"]')).toBeVisible({
100-
timeout: LONG_TIMEOUT,
101-
})
102-
await expect(slideout).toBeHidden()
103-
} finally {
104-
// Clean up so reruns against a persistent project don't accumulate rows.
105-
await Promise.all(
106-
createdFeatureIds.map((id) =>
107-
request.delete(`${api}projects/${project.id}/features/${id}/`, {
108-
headers,
109-
}),
110-
),
111-
)
112-
}
113+
// And - an unknown feature id degrades gracefully (no slideout, no crash).
114+
await page.goto(`${featuresPath}?feature=999999999&tab=value`)
115+
await expect(page.locator('[data-test="features-page"]')).toBeVisible({
116+
timeout: LONG_TIMEOUT,
117+
})
118+
await expect(slideout).toBeHidden()
113119
})
114120
})

‎frontend/e2e/tests/onboarding-tests.pw.ts‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,21 @@ test.describe('Onboarding', () => {
2626
.getByRole('heading', { name: /Welcome/ })
2727
.waitFor({ state: 'visible', timeout: 30000 });
2828

29+
// The features page syncs the slideout state to the URL with
30+
// history.replace, which aborts a full navigation that is still in
31+
// flight - let the page settle and retry.
32+
const gotoOnboarding = async () => {
33+
for (let attempt = 0; ; attempt++) {
34+
try {
35+
await page.goto('/getting-started?connected');
36+
return;
37+
} catch (e) {
38+
if (attempt >= 2) throw e;
39+
await page.waitForTimeout(1000);
40+
}
41+
}
42+
};
43+
2944
log('Sign up');
3045
await page.goto('/');
3146
await click(byId('jsSignup'));
@@ -62,7 +77,7 @@ test.describe('Onboarding', () => {
6277
// No real first evaluation in a test, so force the connected state via
6378
// ?connected (the #7767 stub); that unlocks the toggle and flips LIVE.
6479
log('Force the connected state');
65-
await page.goto('/getting-started?connected');
80+
await gotoOnboarding();
6681
await flowReady();
6782
await expect(page.getByText('LIVE', { exact: true })).toBeVisible();
6883

@@ -119,12 +134,12 @@ test.describe('Onboarding', () => {
119134
/\/features\?feature=\d+&tab=segment-overrides/,
120135
);
121136

122-
await page.goto('/getting-started?connected');
137+
await gotoOnboarding();
123138
await flowReady();
124139
await page.getByRole('button', { name: /Remote config/ }).click();
125140
await expect(page).toHaveURL(/\/features\?feature=\d+&tab=value/);
126141

127-
await page.goto('/getting-started?connected');
142+
await gotoOnboarding();
128143
await flowReady();
129144
await page.getByRole('button', { name: /Experiment/ }).click();
130145
await expect(page).toHaveURL(/\/experiments$/);

‎frontend/web/components/App.js‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -139,14 +139,23 @@ const App = class extends Component {
139139
// New users with no organisation go through the single-page onboarding
140140
// flow when it's enabled - it creates the organisation itself, so it
141141
// replaces the legacy /create page. Everyone else still gets /create.
142-
if (
143-
AccountStore.getUser()?.isGettingStarted &&
144-
Utils.getFlagsmithHasFeature('onboarding_quickstart_flow')
145-
) {
146-
this.props.history.replace('/getting-started')
147-
} else {
148-
this.props.history.replace(`/create${query}`)
149-
}
142+
// The flag must be evaluated for the identified user, not whatever the
143+
// SDK last held, so wait for identify to settle before routing.
144+
// Capped at 2s: a degraded flags API falls back to routing with the
145+
// already-loaded flags instead of blocking the redirect.
146+
Promise.race([
147+
Promise.resolve(API.flagsmithIdentify()).catch(() => {}),
148+
new Promise((resolve) => setTimeout(resolve, 2000)),
149+
]).then(() => {
150+
if (
151+
AccountStore.getUser()?.isGettingStarted &&
152+
Utils.getFlagsmithHasFeature('onboarding_quickstart_flow')
153+
) {
154+
this.props.history.replace('/getting-started')
155+
} else {
156+
this.props.history.replace(`/create${query}`)
157+
}
158+
})
150159
return
151160
}
152161

0 commit comments

Comments
 (0)