fix(e2e): reload login page on failed dynamic import#22875
Open
fix(e2e): reload login page on failed dynamic import#22875
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The login helper navigated to
/loginand immediately tried to fill the Email field. During parallel e2e execution,ERR_NETWORK_CHANGEDcan cause dynamic imports to fail, rendering the error boundary instead of the login form. This causedgetByLabel('Email')to time out after 5 seconds.This applies the same
reloadPageIfDynamicImportFailedpattern already used increateWorkspace(added in d1665e4).Fixes flakes in:
e2e/tests/organizations/customRoles/customRoles.spec.ts("custom roles disabled")e2e/tests/updateTemplate.spec.ts("require latest version")