E2E: replace login priming with an on-demand lock - #113352
Draft
lucatume wants to merge 4 commits into
Draft
Conversation
|
Looks like one of the E2E tests has failed. You can fix them following these steps:
|
Workers share one cookies file per account through COOKIES_PATH, and a CI build starts with none of them. Every worker misses at once and logs in concurrently, against a calypso.live container that has just been created. TestAccount.ensureFreshAuthCookies coordinates through a lock file beside the cookies: the worker that creates it logs in and writes the cookies, the rest poll for those cookies and skip the login. A lock older than 45s belongs to a worker Playwright tore down mid-login, and gets taken over by rename so that only one of several waiters can claim it. A waiter that has waited a minute logs in unlocked rather than burn its test timeout. authenticate() now saves the cookies it obtains, which it never did: the accounts reached only through it re-logged-in on every test.
The setup project logged in as a hand-maintained per-build-type list of accounts before the suite, so the specs would find cookies rather than all log in at once. The list drifted from what the specs use, and an account missing from it got no priming at all. The login lock covers the same ground without a list. Removes the project, the env var behind it and the four TeamCity params setting it, along with the priming half of the legacy Jest global setup, which has had no specs to prime since the last one moved to Playwright Test.
The lock could be lost without its holder noticing: a worker whose login ran past the stale window had the lock taken from under it and wrote the cookies anyway, and the takeover raced with the release. The lock is now a directory holding an ownership marker the holder checks before it writes anything, with a heartbeat keeping the mtime current so a slow login isn't mistaken for a dead worker. Every change to that directory runs under a proper-lockfile guard, so acquire, takeover and release can't interleave. A failed login gets one retry while the lock is still held. The account fixtures no longer log in when they're built; they hand back a TestAccount and authenticate() does the work, so a spec that only wants credentials or the REST client pays nothing for them. That left get-account with nothing to do.
The guard that serializes changes to the lock directory is now built the same way as the lock itself, out of mkdir and an owner directory whose mtime a heartbeat keeps current. One less dependency for a few lines that already existed next to it. Taking over a stale lock now tolerates losing the mkdir race rather than throwing. The account fixtures authenticate again on first use, through getAccount. Handing back an unauthenticated TestAccount pushed the call into every spec that took a fixture and left the ones that didn't make it silently logged out.
lucatume
force-pushed
the
update/e2e-login-lock
branch
from
August 6, 2026 17:20
c59c927 to
f428704
Compare
Contributor
|
WordPress.com
Automattic for Agencies
|
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
2 participants
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.
Proposed Changes
TestAccount.ensureFreshAuthCookies()logs an account in behind a lock file inCOOKIES_PATH, so a run logs each account in once rather than once per worker.authenticate()saves the cookies it obtains. It never did, so the ~70 specs that build aTestAccountdirectly re-logged-in on every test.prime-loginssetup project,AUTHENTICATE_ACCOUNTS, the four TeamCity params setting it, and the priming half of the legacy Jest global setup.Why are these changes being made?
The priming list was hand-maintained per build type and drifted from what the specs use. An account missing from it got no priming at all and every worker logged it in separately.
Testing Instructions
Against
CALYPSO_BASE_URL=$CALYPSO_STAGING_URL, withtest/e2e/cookiesremoved:Expect one
Logging in via Login Pageline and oneSaving auth cookies, the restFound fresh cookies, skipping log in, and every test green. Run again with the cookies in place: no login at all.yarn jest --config packages/calypso-e2e/jest.config.js --rootDir packages/calypso-e2ecovers the lock itself: 8 concurrent workers producing one login, release on success and on throw, stale-lock takeover, and the wait ending on freshness rather than on the file existing.What a reviewer should check: the waiter's exit condition is
hasFreshAuthCookies(), not the cookies file existing. A file left by an earlier run can be there and expired, and waiting on existence hands every waiter a dead session.Pre-merge Checklist