fix: use the shared driver fixture for network assertions (repairs consumer-smoke) - #26
Merged
Merged
Conversation
The tests added in #24 built their own Chrome driver with a bare webdriver.Chrome(options=...), which let Selenium Manager resolve chromedriver instead of webdriver_manager. On the CI runner that produced a version mismatch - "ChromeDriver only supports Chrome 155, current browser is 152" - and all four errored. They errored in the DataSpaceFrontend deploy, not on their own PR: the smoke workflow checks out `ref: CI`, so #24's checks ran against a CI branch that did not yet contain these tests. The first run that exercised them was the next deploy, where consumer-smoke failed and promote-dev was skipped. Root cause was duplicating driver construction. conftest resolves chromedriver carefully - webdriver_manager, then locating and chmod'ing the real binary - and none of that applied to a second, hand-rolled fixture. Moves the performance log capability into the shared fixture and deletes the bespoke one, so chromedriver resolution stays in exactly one place. enable_network_logging is dropped as it no longer has a caller. Verified: tests/consumer/smoke now runs 40 passed, 0 errors.
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
1 participant
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.
Repairs
consumer-smoke, which #24 broke.What happened
The tests in #24 built their own Chrome driver:
That lets Selenium Manager resolve chromedriver, rather than
webdriver_managerasconftest.pydoes. On the CI runner it picked a mismatched driver:All four parametrised cases errored,
consumer-smokefailed, andpromote-devwas skipped — so that release was never marked last-known-good.Why #24 was green
The smoke workflow checks out
ref: CI. When #24's checks ran,CIdid not yet contain these tests, so they never executed. The first run that actually exercised them was the following DataSpaceFrontend deploy.Same pinned-checkout behaviour noted in #20 — a PR's own checks do not test that PR's content.
Fix
The real cause was duplicating driver construction.
conftest.pyresolves chromedriver carefully —ChromeDriverManager().install(), then locating andchmod-ing the real binary — and none of that applied to a hand-rolled second fixture.performancelog capability into the shareddriverfixturedriverlike every other testenable_network_logging, which no longer has a callerChromedriver resolution now lives in exactly one place.
Verified
tests/consumer/smoke→ 40 passed, 0 errors (was 32 passed, 4 errors).