Skip to content

Commit 2b3b9d5

Browse files
authored
ci: cache e2e prod preparation to run once instead of per-shard, various improvements and flake fixes (#15368)
Moves `prepare-run-test-against-prod:ci` into a dedicated `e2e-prep` job that runs once and caches the result. Previously this 2-minute step ran for every E2E shard. Saves ~2 minutes per shard (~140+ minutes total across all E2E jobs). Additionally, this entails a few other test suite improvements and flakes fixes. All together, **this reduces the amount of time our entire CI takes to run from > 1 hour if you factor in manual retries, to about 15 minutes.** - about 2x faster `waitForCompilation` checking, by checking more frequently and without waiting for the load event. This previously took about 2 seconds in CI, often running before every single test. After `waitForCompilation` we usually navigate to a different page, so waiting for a full page load was a waste of time. - Reduce test timeouts. Quicker failures => quicker retries => quicker test suite passes. The previous config was excessive - Introduces a shorter navigation timeout. By default, it used to be equal to `TEST_TIMEOUT`. `TEST_TIMEOUT` is high to accommodate long e2e tests with multiple steps, not to allow page.goto to try for 3 minutes before it fails. - Reduces console spam due to `waitForCompilation` - Reduce retries of `waitForCompilation`. If the dev server fails for some reason, we shouldn't have to wait 50 (!!) minutes until the e2e test finally fails and is ready to be retried - Lots of flake fixes for individual tests and test suites - Fixes flaky behavior of checkbox component. This **fixes flakiness EVERYWHERE any checkbox is clicked**. - Fixes upload directory clearing and seed restoration not working correctly if there were multiple upload dirs
1 parent 50c087f commit 2b3b9d5

File tree

43 files changed

+877
-515
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

43 files changed

+877
-515
lines changed

.github/workflows/e2e.config.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ export default createE2EConfig([
9090
{ file: 'queues', shards: 1 },
9191
{ file: 'sort', shards: 1 },
9292
{ file: 'server-url', shards: 1 },
93-
{ file: 'trash', shards: 1 },
93+
{ file: 'trash', shards: 2 },
9494
{ file: 'versions', shards: 3 },
9595
{ file: 'uploads', shards: 3 },
9696
])

.github/workflows/main.yml

Lines changed: 51 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -251,9 +251,42 @@ jobs:
251251
matrix=$(node .github/workflows/e2e.config.ts)
252252
echo "matrix=$matrix" >> $GITHUB_OUTPUT
253253
254+
# Prepare prod test environment once, shared by all E2E shards
255+
e2e-prep:
256+
name: E2E Prep
257+
runs-on: ubuntu-24.04
258+
needs: [e2e-matrix]
259+
steps:
260+
- uses: actions/checkout@v5
261+
262+
- name: Node setup
263+
uses: ./.github/actions/setup
264+
with:
265+
pnpm-run-install: false
266+
pnpm-restore-cache: false
267+
268+
- name: Restore build
269+
uses: actions/cache@v4
270+
with:
271+
path: ./*
272+
key: ${{ github.sha }}
273+
274+
- name: Prepare prod test environment
275+
run: pnpm prepare-run-test-against-prod:ci
276+
277+
- name: Cache prepared test environment
278+
uses: actions/cache/save@v4
279+
with:
280+
path: |
281+
test/packed
282+
test/node_modules
283+
test/package.json
284+
test/pnpm-lock.yaml
285+
key: e2e-prep-${{ github.sha }}
286+
254287
tests-e2e:
255288
runs-on: ubuntu-24.04
256-
needs: e2e-matrix
289+
needs: [e2e-matrix, e2e-prep]
257290
name: E2E - ${{ matrix.suite }}${{ matrix.total-shards > 1 && format(' ({0}/{1})', matrix.shard, matrix.total-shards) || '' }}
258291
timeout-minutes: 45
259292
strategy:
@@ -276,6 +309,17 @@ jobs:
276309
path: ./*
277310
key: ${{ github.sha }}
278311

312+
- name: Restore prepared test environment
313+
uses: actions/cache/restore@v4
314+
with:
315+
path: |
316+
test/packed
317+
test/node_modules
318+
test/package.json
319+
test/pnpm-lock.yaml
320+
key: e2e-prep-${{ github.sha }}
321+
fail-on-cache-miss: true
322+
279323
- name: Start LocalStack
280324
run: pnpm docker:start
281325
if: matrix.suite == 'plugin-cloud-storage'
@@ -333,7 +377,10 @@ jobs:
333377
# However, --fully-parallel would also run all tests in parallel within each shard.
334378
# To maintain serial execution for most suites, we pass --workers=1 when parallel=false.
335379
# Suites with parallel=true (e.g. LexicalFullyFeatured) use the default 16 workers.
336-
run: PLAYWRIGHT_JSON_OUTPUT_NAME=results_${{ matrix.suite }}_${{ matrix.shard }}.json pnpm test:e2e:prod:ci:noturbo ${{ matrix.suite }} --shard=${{ matrix.shard }}/${{ matrix.total-shards }} --fully-parallel${{ matrix.parallel == false && ' --workers=1' || '' }}
380+
#
381+
# Note: The e2e-prep job runs prepare-run-test-against-prod:ci once and caches the result.
382+
# This job restores that cache, so we use test:e2e:prod:run which skips preparation.
383+
run: PLAYWRIGHT_JSON_OUTPUT_NAME=results_${{ matrix.suite }}_${{ matrix.shard }}.json pnpm test:e2e:prod:run:noturbo ${{ matrix.suite }} --shard=${{ matrix.shard }}/${{ matrix.total-shards }} --fully-parallel${{ matrix.parallel == false && ' --workers=1' || '' }}
337384
env:
338385
PLAYWRIGHT_JSON_OUTPUT_NAME: results_${{ matrix.suite }}_${{ matrix.shard }}.json
339386
NEXT_TELEMETRY_DISABLED: 1
@@ -555,7 +602,7 @@ jobs:
555602
- name: Start PostgreSQL
556603
uses: CasperWA/postgresql-action@v1.2
557604
with:
558-
postgresql version: '14' # See https://hub.docker.com/_/postgres for available versions
605+
postgresql version: "14" # See https://hub.docker.com/_/postgres for available versions
559606
postgresql db: ${{ env.POSTGRES_DB }}
560607
postgresql user: ${{ env.POSTGRES_USER }}
561608
postgresql password: ${{ env.POSTGRES_PASSWORD }}
@@ -720,4 +767,4 @@ jobs:
720767
if: github.event.pull_request.head.repo.fork == false
721768
uses: exoego/esbuild-bundle-analyzer@v1
722769
with:
723-
metafiles: 'packages/payload/meta_index.json,packages/payload/meta_shared.json,packages/ui/meta_client.json,packages/ui/meta_shared.json,packages/next/meta_index.json,packages/richtext-lexical/meta_client.json'
770+
metafiles: "packages/payload/meta_index.json,packages/payload/meta_shared.json,packages/ui/meta_client.json,packages/ui/meta_shared.json,packages/next/meta_index.json,packages/richtext-lexical/meta_client.json"

package.json

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -129,9 +129,11 @@
129129
"test:e2e:debug": "cross-env NODE_OPTIONS=\"--no-deprecation --no-experimental-strip-types\" NODE_NO_WARNINGS=1 PWDEBUG=1 DISABLE_LOGGING=true playwright test",
130130
"test:e2e:headed": "cross-env NODE_OPTIONS=\"--no-deprecation --no-experimental-strip-types\" NODE_NO_WARNINGS=1 DISABLE_LOGGING=true playwright test --headed",
131131
"test:e2e:noturbo": "pnpm runts ./test/runE2E.ts --no-turbo",
132-
"test:e2e:prod": "pnpm prepare-run-test-against-prod && pnpm runts ./test/runE2E.ts --prod",
133-
"test:e2e:prod:ci": "pnpm prepare-run-test-against-prod:ci && pnpm runts ./test/runE2E.ts --prod",
134-
"test:e2e:prod:ci:noturbo": "pnpm prepare-run-test-against-prod:ci && pnpm runts ./test/runE2E.ts --prod --no-turbo",
132+
"test:e2e:prod": "pnpm prepare-run-test-against-prod && pnpm test:e2e:prod:run",
133+
"test:e2e:prod:ci": "pnpm prepare-run-test-against-prod:ci && pnpm test:e2e:prod:run",
134+
"test:e2e:prod:ci:noturbo": "pnpm prepare-run-test-against-prod:ci && pnpm test:e2e:prod:run:noturbo",
135+
"test:e2e:prod:run": "pnpm runts ./test/runE2E.ts --prod",
136+
"test:e2e:prod:run:noturbo": "pnpm runts ./test/runE2E.ts --prod --no-turbo",
135137
"test:int": "cross-env NODE_OPTIONS=\"--no-deprecation --no-experimental-strip-types\" NODE_NO_WARNINGS=1 DISABLE_LOGGING=true vitest --project int",
136138
"test:int:firestore": "cross-env NODE_OPTIONS=\"--no-deprecation --no-experimental-strip-types\" NODE_NO_WARNINGS=1 PAYLOAD_DATABASE=firestore DISABLE_LOGGING=true vitest --project int",
137139
"test:int:postgres": "cross-env NODE_OPTIONS=\"--no-deprecation --no-experimental-strip-types\" NODE_NO_WARNINGS=1 PAYLOAD_DATABASE=postgres DISABLE_LOGGING=true vitest --project int",

packages/ui/src/elements/EditMany/DrawerContent.tsx

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ import { useConfig } from '../../providers/Config/index.js'
2828
import { DocumentInfoProvider } from '../../providers/DocumentInfo/index.js'
2929
import { useLocale } from '../../providers/Locale/index.js'
3030
import { OperationContext } from '../../providers/Operation/index.js'
31-
import { useRouteCache } from '../../providers/RouteCache/index.js'
3231
import { useServerFunctions } from '../../providers/ServerFunctions/index.js'
3332
import { useTranslation } from '../../providers/Translation/index.js'
3433
import { abortAndIgnore, handleAbortRef } from '../../utilities/abortAndIgnore.js'
@@ -180,7 +179,6 @@ export const EditManyDrawerContent: React.FC<EditManyDrawerContentProps> = (prop
180179

181180
const router = useRouter()
182181
const abortFormStateRef = React.useRef<AbortController>(null)
183-
const { clearRouteCache } = useRouteCache()
184182
const collectionPermissions = permissions?.collections?.[collection.slug]
185183
const searchParams = useSearchParams()
186184

@@ -273,12 +271,12 @@ export const EditManyDrawerContent: React.FC<EditManyDrawerContentProps> = (prop
273271
qs.stringify(
274272
{
275273
...parseSearchParams(searchParams),
274+
_r: Date.now(), // Cache buster to force fresh data fetch. Prevents an e2e race condition where sometimes the data is not updated.
276275
page: selectAll ? '1' : undefined,
277276
},
278277
{ addQueryPrefix: true },
279278
),
280279
)
281-
clearRouteCache()
282280
closeModal(drawerSlug)
283281

284282
if (typeof onSuccessFromProps === 'function') {

packages/ui/src/elements/Pill/index.tsx

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
'use client'
22
import type { ElementType, HTMLAttributes } from 'react'
33

4-
import React from 'react' // TODO: abstract this out to support all routers
4+
import React, { useEffect, useState } from 'react' // TODO: abstract this out to support all routers
55

66
import { Link } from '../Link/index.js'
77

@@ -95,6 +95,14 @@ const StaticPill: React.FC<PillProps> = (props) => {
9595
to,
9696
} = props
9797

98+
const [isHydrated, setIsHydrated] = useState(false)
99+
100+
useEffect(() => {
101+
setIsHydrated(true)
102+
}, [])
103+
104+
const isButton = onClick && !to
105+
98106
const classes = [
99107
baseClass,
100108
`${baseClass}--style-${pillStyle}`,
@@ -112,7 +120,7 @@ const StaticPill: React.FC<PillProps> = (props) => {
112120

113121
let Element: ElementType | React.FC<RenderedTypeProps> = 'div'
114122

115-
if (onClick && !to) {
123+
if (isButton) {
116124
Element = 'button'
117125
}
118126

@@ -128,10 +136,11 @@ const StaticPill: React.FC<PillProps> = (props) => {
128136
aria-expanded={ariaExpanded}
129137
aria-label={ariaLabel}
130138
className={classes}
139+
disabled={isButton ? !isHydrated : undefined}
131140
href={to || null}
132141
id={id}
133142
onClick={onClick}
134-
type={Element === 'button' ? 'button' : undefined}
143+
type={isButton ? 'button' : undefined}
135144
>
136145
<span className={`${baseClass}__label`}>{children}</span>
137146
{Boolean(icon) && <span className={`${baseClass}__icon`}>{icon}</span>}

packages/ui/src/fields/Checkbox/Input.tsx

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
'use client'
22
import type { StaticLabel } from 'payload'
33

4-
import React, { useId } from 'react'
4+
import React, { useEffect, useId, useState } from 'react'
55

66
import { RenderCustomComponent } from '../../elements/RenderCustomComponent/index.js'
77
import { FieldLabel } from '../../fields/FieldLabel/index.js'
@@ -40,11 +40,19 @@ export const CheckboxInput: React.FC<CheckboxInputProps> = ({
4040
localized,
4141
onToggle,
4242
partialChecked,
43-
readOnly,
43+
readOnly: readOnlyFromProps,
4444
required,
4545
}) => {
46+
const [isHydrated, setIsHydrated] = useState(false)
4647
const fallbackID = useId()
4748
const id = idFromProps || fallbackID
49+
50+
useEffect(() => {
51+
setIsHydrated(true)
52+
}, [])
53+
54+
const readOnly = readOnlyFromProps || !isHydrated
55+
4856
return (
4957
<div
5058
className={[
@@ -61,11 +69,11 @@ export const CheckboxInput: React.FC<CheckboxInputProps> = ({
6169
<input
6270
aria-label=""
6371
aria-labelledby={name}
64-
defaultChecked={Boolean(checked)}
72+
checked={Boolean(checked)}
6573
disabled={readOnly}
6674
id={id}
6775
name={name}
68-
onInput={onToggle}
76+
onChange={onToggle}
6977
ref={inputRef}
7078
required={required}
7179
title={name}

packages/ui/src/providers/Selection/index.tsx

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,11 +105,24 @@ export const SelectionProvider: React.FC<Props> = ({ children, docs = [], totalD
105105
) {
106106
setSelectAll(SelectAllStatus.None)
107107
} else {
108+
const shouldSelect = selectAll !== SelectAllStatus.Some
109+
108110
docs.forEach(({ id, _isLocked, _userEditing }) => {
109111
if (!_isLocked || _userEditing?.id === user?.id) {
110-
rows.set(id, selectAll !== SelectAllStatus.Some)
112+
rows.set(id, shouldSelect)
111113
}
112114
})
115+
116+
// Set selectAll synchronously to avoid race conditions with useEffect
117+
if (shouldSelect && rows.size > 0) {
118+
if (rows.size === docs.length) {
119+
setSelectAll(SelectAllStatus.AllInPage)
120+
} else {
121+
setSelectAll(SelectAllStatus.Some)
122+
}
123+
} else {
124+
setSelectAll(SelectAllStatus.None)
125+
}
113126
}
114127

115128
setSelected(rows)

test/access-control/e2e.spec.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -410,7 +410,6 @@ describe('Access Control', () => {
410410

411411
await page.goto(restrictedUrl.list)
412412

413-
414413
expect(errors).not.toHaveLength(0)
415414
})
416415

@@ -704,6 +703,9 @@ describe('Access Control', () => {
704703
const adminUserRow = page.locator('.table tr').filter({ hasText: devUser.email })
705704
const nonAdminUserRow = page.locator('.table tr').filter({ hasText: nonAdminEmail })
706705

706+
// Wait for hydration
707+
await wait(1000)
708+
707709
// Ensure admin user cannot unlock other users
708710
await adminUserRow.locator('.cell-id a').click()
709711
await page.waitForURL(`**/collections/users/**`)
@@ -715,6 +717,9 @@ describe('Access Control', () => {
715717

716718
await page.goto(usersUrl.list)
717719

720+
// Wait for hydration
721+
await wait(1000)
722+
718723
// Ensure non-admin user cannot see unlock button
719724
await nonAdminUserRow.locator('.cell-id a').click()
720725
await page.waitForURL(`**/collections/users/**`)

test/admin/e2e/document-view/e2e.spec.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,8 @@ describe('Document View', () => {
123123
describe('API view', () => {
124124
test('collection — should not show API tab when disabled in config', async () => {
125125
await page.goto(postsUrl.collection(noApiViewCollectionSlug))
126+
// Wait for hydration
127+
await wait(1000)
126128
await page.locator('.collection-list .table a').click()
127129
await expect(page.locator('.doc-tabs__tabs-container')).not.toContainText('API')
128130
})

test/admin/e2e/general/e2e.spec.ts

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -477,6 +477,8 @@ describe('General', () => {
477477

478478
test('dashboard — should navigate to collection', async () => {
479479
await page.goto(postsUrl.admin)
480+
// Wait for hydration - otherwise playwright clicks the card early and nothing happens
481+
await wait(1000)
480482
const anchor = page.locator(`.card-${postsCollectionSlug} a.card__click`)
481483
const anchorHref = await anchor.getAttribute('href')
482484
await anchor.click()
@@ -617,6 +619,8 @@ describe('General', () => {
617619

618620
test('should replace history when adding query params to the URL and not push a new entry', async () => {
619621
await page.goto(postsUrl.admin)
622+
// Wait for hydration - otherwise playwright clicks the card early and nothing happens
623+
await wait(1000)
620624
await page.locator('.collections__card-list .card__click').first().click()
621625
// flaky
622626
// eslint-disable-next-line playwright/no-wait-for-timeout
@@ -860,6 +864,10 @@ describe('General', () => {
860864
})
861865

862866
test('should allow custom translation of locale labels', async () => {
867+
await page.goto(postsUrl.account)
868+
// Wait for hydration - otherwise playwright clicks the localizer early and nothing happens
869+
await wait(1000)
870+
863871
const selectOptionClass = '.popup__content .popup-button-list__button'
864872
const localizerButton = page.locator('.localizer .popup-button')
865873
const localeListItem1 = page.locator(selectOptionClass).nth(0)
@@ -1035,10 +1043,33 @@ describe('General', () => {
10351043
})
10361044

10371045
describe('progress bar', () => {
1038-
test('should show progress bar on page navigation', async () => {
1039-
await page.goto(postsUrl.admin)
1046+
test.fixme('should show progress bar on page navigation', async () => {
1047+
// TODO: This test is extremely flaky in CI. Not a surprise, the progress bar only shows if the timing is right. Need to fix this and make extra sure it passes in CI without retries.
1048+
// eslint-disable-next-line playwright/no-networkidle
1049+
await page.goto(postsUrl.admin, { waitUntil: 'networkidle' })
1050+
// Wait for hydration - otherwise playwright clicks the card early and nothing happens
1051+
await wait(1000)
1052+
1053+
// Throttle network to ensure navigation takes > 500ms so progress bar is visible
1054+
// Progress bar has 150ms initial delay before showing, so fast navigations won't show it
1055+
const client = await page.context().newCDPSession(page)
1056+
await client.send('Network.emulateNetworkConditions', {
1057+
downloadThroughput: (500 * 1024) / 8, // 500 kbps
1058+
latency: 400, // 400ms latency
1059+
offline: false,
1060+
uploadThroughput: (500 * 1024) / 8,
1061+
})
1062+
10401063
await page.locator('.collections__card-list .card').first().click()
10411064
await expect(page.locator('.progress-bar')).toBeVisible()
1065+
1066+
// Reset network conditions
1067+
await client.send('Network.emulateNetworkConditions', {
1068+
downloadThroughput: -1,
1069+
latency: 0,
1070+
offline: false,
1071+
uploadThroughput: -1,
1072+
})
10421073
})
10431074
})
10441075
})

0 commit comments

Comments
 (0)