X Tutup
Skip to content

fix(e2e): reload login page on failed dynamic import#22875

Open
mafredri wants to merge 9 commits intomainfrom
fix-flakes
Open

fix(e2e): reload login page on failed dynamic import#22875
mafredri wants to merge 9 commits intomainfrom
fix-flakes

Conversation

@mafredri
Copy link
Member

The login helper navigated to /login and immediately tried to fill the Email field. During parallel e2e execution, ERR_NETWORK_CHANGED can cause dynamic imports to fail, rendering the error boundary instead of the login form. This caused getByLabel('Email') to time out after 5 seconds.

This applies the same reloadPageIfDynamicImportFailed pattern already used in createWorkspace (added in d1665e4).

Fixes flakes in:

  • e2e/tests/organizations/customRoles/customRoles.spec.ts ("custom roles disabled")
  • e2e/tests/updateTemplate.spec.ts ("require latest version")

mafredri added 7 commits March 9, 2026 22:09
The bool case in fillParameters blindly toggled the switch with a
single click. If dynamic parameters hydrated after the click and
reset the switch state, the final value was wrong, causing the
'overwrite default parameters' test to flake.

Mirror the retry pattern already used for string/number parameters:
read the current aria-checked state, click only if it differs from
the desired value, then verify and retry up to 3 times.
ForkReap starts a reaper goroutine that calls Wait4(-1, WNOHANG),
which reaps any child. This raced with ForkReap's own Wait4(pid)
for the direct child, causing ECHILD errors.

Use the reapLock that go-reap already supports: hold a read lock
during our Wait4(pid) so the reaper's write lock blocks until we
have collected the child's status.
The TestReapInterrupt test sends SIGINT to its own process. Under
the race detector, the catchSignals goroutine inside ForkReap may
not have registered signal.Notify yet, so Go's default SIGINT
handler can terminate the test binary.

Register signal.Notify for os.Interrupt in the test before sending
SIGINT. This disables the default termination behavior while still
allowing the catchSignals handler to receive the signal
independently.
The lint/go recipe used $(shell) inside a recipe to extract the
golangci-lint version. When MAKE_TIMED=1, $(shell) runs through
timed-shell.sh with $@ (the target name) as the first argument.
Since the target name doesn't start with '-', the timing code
path runs and its banner output contaminates the captured value,
causing intermittent 'lint/go: No such file or directory' errors.

Replace with bash command substitution ($$(...)), which is the
correct approach under .ONESHELL and avoids the SHELL/SHELLFLAGS
interaction entirely. Also replace deprecated egrep with grep -oE.
When parallel e2e tests trigger ERR_NETWORK_CHANGED, React
Router's dynamic import for the page chunk can fail, rendering
the error boundary instead of the expected page content. This
caused createWorkspace to time out waiting for the name input.

After navigating, race the expected content against the error
boundary. If the error boundary wins, reload the page to retry
the chunk load.
The second tick used a time offset from a 'now' variable captured
early in the test. The activity bump uses the database NOW() which
advances with wall clock time. Under slow CI, the elapsed time
between the two caused the tick to land before the bumped deadline,
so the executor skipped the stop transition.

Use time.Now() when computing the second tick so it is always
relative to the actual bump time.
The login helper navigated to /login and immediately tried to
fill the Email field. During parallel e2e execution,
ERR_NETWORK_CHANGED can cause dynamic imports to fail, rendering
the error boundary instead of the login form. This caused
getByLabel('Email') to time out.

Apply the same reloadPageIfDynamicImportFailed pattern already
used in createWorkspace.
The verifyParameters helper used a one-shot isChecked() call to
verify boolean parameter state. Dynamic parameter hydration can
reset the checkbox after page load, causing a race where the
snapshot reads the stale default value instead of the expected
one. Replace with Playwright's auto-retrying toBeChecked() /
not.toBeChecked() assertions, matching the pattern already used
for string and number parameters.
Move signal.Notify call from inside the catchSignals goroutine
to ForkReap's synchronous path. This prevents a race where
SIGINT arrives before the goroutine has registered its handler,
causing the signal to never be forwarded to the child process
and TestReapInterrupt to hang indefinitely.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

X Tutup