Skip to content

fix(nimbus): Approve only our own Kinto review in the selenium teardown - #16981

Open
jaredlockhart wants to merge 2 commits into
mainfrom
16978
Open

fix(nimbus): Approve only our own Kinto review in the selenium teardown#16981
jaredlockhart wants to merge 2 commits into
mainfrom
16978

Conversation

@jaredlockhart

@jaredlockhart jaredlockhart commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

Because

  • The selenium teardown called approve() on the kinto_client factory, so it raised AttributeError and was swallowed by the bare except, leaving a pending review that a later test signs as if it were its own.
  • approve() checks only the collection status, so any caller signs whatever is pending, and the teardown runs for tests that xdist grouping deliberately keeps off that collection's worker.
  • approve() retries for 120s, so activating the teardown unbounded would burn that budget per test whenever no review is pending.

This commit

  • Calls the factory so the teardown approves via a KintoClient instance.
  • Gives KintoClient a slug and only signs when the pending review changes that slug's record, comparing the workspace and main buckets.
  • Threads the test's experiment slug through the kinto_client fixture so every call site is scoped to its own experiment.
  • Bounds the teardown to a short explicit retry budget.

Fixes #16978

@jaredlockhart
jaredlockhart marked this pull request as ready for review August 25, 2026 17:11
@jaredlockhart
jaredlockhart marked this pull request as draft August 25, 2026 19:52
@jaredlockhart

Copy link
Copy Markdown
Collaborator Author

Still trying to track down these intermittent Kinto approve CI failures, iterating in this PR with some extra logging.

@jaredlockhart jaredlockhart changed the title fix(nimbus): Approve the Kinto review left by the selenium teardown fix(nimbus): Approve only our own Kinto review in the selenium teardown Sep 3, 2026
Because

* The selenium teardown called approve() on the kinto_client factory, so it
  raised AttributeError and was swallowed by the bare except.
* end_experiment() still ran, leaving a pending review that a later test signs
  as if it were its own, cascading until a test finds nothing pending and
  raises "Unable to approve kinto review".
* approve() retries for 120s, so activating the teardown unbounded would burn
  that budget per test whenever no review is pending.

This commit

* Calls the factory so the teardown approves via a KintoClient instance.
* Adds an optional retries argument to approve(), leaving the default budget
  unchanged for in-test callers.
* Bounds the teardown to a short explicit budget.

Fixes #16978
@jaredlockhart
jaredlockhart marked this pull request as ready for review September 4, 2026 19:30
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.

Integration test selenium teardown never approves its Kinto review, leaving pending reviews for later tests to consume

1 participant