From e06f5bbf5890d7aeff2f61f7c5e50e0258e2748c Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Mon, 15 Jun 2026 19:03:04 -0700 Subject: [PATCH] test(migrations): Deflake test_reverse_idempotency_all 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) Agent transcript: https://claudescope.sentry.dev/share/k51suCCzzW9KZZdJFGzOKnV-1ugFp1MOIK3FsEnHXSA --- tests/migrations/test_runner.py | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/tests/migrations/test_runner.py b/tests/migrations/test_runner.py index 56f21511697..2cb5711d0c5 100644 --- a/tests/migrations/test_runner.py +++ b/tests/migrations/test_runner.py @@ -351,13 +351,17 @@ def reverse_twice() -> None: ClickhouseClientSettings.MIGRATE ) - before_state = cluster_connection.execute( - "SELECT create_table_query FROM system.tables" - ).results + # `system.tables` has no guaranteed row order, and the + # reverse_twice() cycle in between can perturb the order rows + # are returned in (e.g. ON CLUSTER DDL re-registering tables). + # Order the snapshots so the comparison reflects the set of + # tables and their definitions, not the iteration order. + snapshot_query = ( + "SELECT create_table_query FROM system.tables ORDER BY create_table_query" + ) + before_state = cluster_connection.execute(snapshot_query).results reverse_twice() - after_state = cluster_connection.execute( - "SELECT create_table_query FROM system.tables" - ).results + after_state = cluster_connection.execute(snapshot_query).results assert before_state == after_state except UndefinedClickhouseCluster: # Some groups do not have a cluster defined (e.g. test_migration) @@ -489,7 +493,7 @@ def test_check_inactive_replica() -> None: with pytest.raises(InactiveClickhouseReplica) as exc: check_for_inactive_replicas([mock_cluster]) - assert exc.value.args[0] == ( + assert str(exc.value) == ( "Cluster test_cluster has inactive replicas for table bad_table_1 " "with 2 out of 3 replicas active.\n" "Cluster test_cluster has inactive replicas for table bad_table_2 "