Repository navigation
Conversation
The integration tests under `cli/tests/` drive one real tenant, shared by every run in the repository, and several commands list or prune everything in it. So one run's fixture teardown breaks another run's command: deleting a source while its dataset still references it makes the platform's dataset listing return `404 No such source` for the whole tenant, failing an unrelated test. A release triggers this every time, because publishing it fires the Publish workflow seconds after its merge is pushed to master, and both workflows run the suite on two OSes — four legs on one tenant. It took out the v0.43.0 publish, which had to be re-run by hand. Give the leg that runs the suite a repository-wide concurrency group, shared by both workflows, so the suite never overlaps itself. The other legs get a group of their own so they are not queued behind it, and run unit and doc tests only: the suite exercises the CLI against the API over HTTP, which is the same work on either OS. `scripts/check` keeps running everything by default, so this does not change what it does for anyone running it locally.
This branch has not been deployed
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 integration tests under
cli/tests/drive one real tenant, shared by every run in the repository. Several commands under test list or prune everything in it, so one run's fixture teardown can break another run's command — and the failure lands on a test that has nothing to do with the code being changed.That is what took out the v0.43.0 publish (run 37785697074):
test_prune_dry_run_backs_up_without_deletingfailed withAnother run deleted a source while its dataset still referenced it, and the platform's dataset listing then 404s for the whole tenant.
re prunehas to list every dataset to check for shared sources, so it inherits that. The tag was already published, the binaries were not, and the release had to be re-run by hand.A release triggers this every time. Publishing fires the Publish workflow seconds after the merge is pushed to master, and both workflows run the suite on ubuntu and macos — four legs on one tenant. The v0.43.0 windows overlapped exactly:
v0.42.0 got lucky.
What changed
integration-test-tenant, shared by both workflows, withcancel-in-progress: false. The suite can no longer overlap itself.github.run_id) so they are never queued behind it.SKIP_INTEGRATION_TESTS=1. The suite exercises the CLI against the API over HTTP, which is identical work on either OS; the per-OS build and unit tests still run everywhere, so portability coverage is unchanged.scripts/checkruns everything by default, so nothing changes for anyone running it locally.One group rather than one per OS, because a cross-OS pair races just as happily as a same-OS one. The group is on the job and not the workflow because GitHub keeps only a single pending run per group and cancels any earlier one — with every matrix leg in the group, legs would be cancelled rather than queued.
Test plan
SKIP_INTEGRATION_TESTS=1 ./scripts/checkwithREINFER_CLI_TEST_{PROJECT,TOKEN,ENDPOINT}unset: passes, 54 + 41 unit tests, no tenant needed.--lib --binsselects only the two unit-test binaries, where--all-targetspicks up three including the live suite.${{ }}.SKIP_INTEGRATION_TESTSis written${{ matrix.os != 'ubuntu-24.04' && '1' || '0' }}rather than the other way round on purpose: GitHub casts the string'0'to a falsy number, socond && '0' || '1'yields'1'on both branches and would have skipped the suite everywhere.Not fixed here
Two runs still cannot safely share the tenant — they are only kept from doing so. Proper isolation means a tenant per run, which needs infrastructure rather than a workflow change. Separately, the platform returning 404 for an entire dataset listing because one dataset references a deleted source looks worth a ticket of its own.