Skip to content

test(migrations): Deflake test_reverse_idempotency_all#8038

Open
phacops wants to merge 1 commit into
masterfrom
pierre.massat/test/deflake-reverse-idempotency
Open

test(migrations): Deflake test_reverse_idempotency_all#8038
phacops wants to merge 1 commit into
masterfrom
pierre.massat/test/deflake-reverse-idempotency

Conversation

@phacops

@phacops phacops commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

test_reverse_idempotency_all snapshots system.tables before and after a reverse/fake-forward cycle and compares the two results for equality. The snapshot query had no ORDER BY, and system.tables has no guaranteed row order, so the comparison was order-sensitive.

In the distributed setup the reverse_twice() cycle between the two snapshots issues ON CLUSTER DDL (CREATE … IF NOT EXISTS / DROP … IF EXISTS) which can re-register tables and change the order rows come back in without changing the set of tables. That produced intermittent failures in the test_migrations_distributed CI job — e.g. on #7980, whose latest EAP migration drops and recreates three downsample materialized views on reverse, which is exactly the kind of churn that perturbs the row order. The reverse cycle itself is a no-op on the table set (fake-forward only updates status; the create/drop are guarded by IF [NOT] EXISTS), so the failures were spurious, not real state differences.

This orders both snapshots by create_table_query so the assertion reflects the set of tables and their definitions rather than the iteration order. A genuine difference in tables or definitions is still caught.

Also replaces exc.value.args[0] with str(exc.value) in test_check_inactive_replica — a pre-existing mypy error (BaseExcT_co_default has no attribute "args") in this file that pre-commit only surfaces once the file is otherwise modified. str(exc.value) is the equivalent message check and types cleanly.

Agent transcript: https://claudescope.sentry.dev/share/5BgPWGjZzfAo_gfbl1kqFzjsOJKf5sz8T4u3s0en9OY

test_reverse_idempotency_all snapshots `system.tables` before and after a
reverse/fake-forward cycle and compares the two results for equality. The
query had no ORDER BY, and `system.tables` has no guaranteed row order, so
the comparison was order-sensitive: the reverse_twice() cycle in between
(ON CLUSTER DDL re-registering tables in the distributed setup) could change
the order rows are returned in without changing the set of tables, producing
intermittent failures in the test_migrations_distributed CI job.

Order both snapshots by create_table_query so the assertion reflects the set
of tables and their definitions rather than the iteration order. A genuine
difference in tables or definitions is still caught.

Also replace `exc.value.args[0]` with `str(exc.value)` in
test_check_inactive_replica: a pre-existing mypy error in this file that
pre-commit only surfaces when the file is otherwise modified.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Agent transcript: https://claudescope.sentry.dev/share/k51suCCzzW9KZZdJFGzOKnV-1ugFp1MOIK3FsEnHXSA
@phacops phacops requested a review from a team as a code owner June 16, 2026 02:03
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.

1 participant