fix(e2e): globalSetup wrote to the SHARED instance on every run - #72
Merged
Conversation
Both of globalSetup's `occ` helpers hardcoded the container:
docker exec -u www-data nextcloud php occ openbuild:seed-hello-world-fixture
docker exec -u www-data nextcloud php occ config:system:set ratelimit… \
&& docker exec nextcloud apache2ctl graceful
`nextcloud` is the SHARED dev box on :8080. The suite is driven with
PLAYWRIGHT_BASE_URL pointing at the disposable container on :8099, so every run
seeded fixtures into — and set a system config on, and gracefully restarted
Apache in — an instance the tests were not asserting against.
A cross-instance write on every single run, and silent: it only surfaced
because :8080 went into maintenance and the seed started logging
"hello-world fixture seed failed (specs needing it will fail)" while the run
otherwise looked fine. Same class of drift as the base-URL bug that
tests/e2e/support/baseUrl.ts documents, and the same failure mode as the
incident that had a session's numbers retracted.
The container is now resolved from the PUBLISHED PORT of the instance under
test (`docker ps --filter publish=<port>`), not by name. Name matching is what
makes this trap easy: `docker ps -f name=nextcloud` is a SUBSTRING match and
will happily return `nextcloud`, `ob-vue3-e2e-nextcloud` or a stale
`nextcloud-old`. The port is the one thing that cannot disagree with the URL the
tests actually talk to.
When the container cannot be resolved, both helpers SKIP with an actionable
message (set OPENBUILD_E2E_CONTAINER, or the per-command overrides) and
deliberately do NOT fall back to a hardcoded name — the fallback is the bug.
The rate-limit helper says so explicitly, since it writes config and restarts
Apache and must never guess an instance.
globalSetup also now logs which instance and container it resolved, so a
mismatch is visible in the first line of any run rather than inferred from a
failure three specs later.
Verified: `instance under test: http://localhost:8099 (container: ob-vue3-e2e)`,
the hello-world seed succeeds against it, and applicationCard stays 4/4 green.
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 bug
Both of
globalSetup'socchelpers hardcoded the container:nextcloudis the shared dev box on :8080. The suite is driven withPLAYWRIGHT_BASE_URLpointing at the disposable container on :8099 — so every run seeded fixtures into, set a system config on, and gracefully restarted Apache in an instance the tests were not asserting against.A cross-instance write on every single run, and silent. It only surfaced because :8080 went into maintenance and the seed began logging "hello-world fixture seed failed (specs needing it will fail)" while the run otherwise looked fine. Same class of drift as the base-URL bug that
tests/e2e/support/baseUrl.tsdocuments — and the same failure mode as the incident that had a session's numbers retracted.The fix
The container is resolved from the published port of the instance under test (
docker ps --filter publish=<port>), not by name.Name matching is precisely what makes this trap easy:
docker ps -f name=nextcloudis a substring match and will happily returnnextcloud,ob-vue3-e2e-nextcloud, or a stalenextcloud-old. The port is the one thing that cannot disagree with the URL the tests actually talk to.When the container can't be resolved, both helpers skip with an actionable message (
OPENBUILD_E2E_CONTAINER, or the existing per-command overrides) and deliberately do not fall back to a hardcoded name — the fallback is the bug. The rate-limit helper says so explicitly, since it writes config and restarts Apache and must never guess an instance.globalSetupnow also logs the resolved instance and container, so a mismatch shows up in the first line of any run instead of being inferred from a failure three specs later.Verification
The seed now succeeds against the correct container (it had been failing while :8080 was in maintenance), and
applicationCardstays 4/4 green. ESLint clean apart from the repo-wide pre-existingn/no-unpublished-import.