Hi all, I have been looking at the isolation tests, and we have in some specs steps which are defined but not used in any permutations. In order to detect them, I have been using the attached trick to track which permutations are used. This allows to find immediately any over-engineered spec by generating diffs about steps defined by not used in any permutations. On HEAD, we have six specs entering in this category.
Would that be useful? -- Michael
diff --git a/src/test/isolation/isolationtester.c b/src/test/isolation/isolationtester.c index f98bb1cf64..402bb1f546 100644 --- a/src/test/isolation/isolationtester.c +++ b/src/test/isolation/isolationtester.c @@ -239,6 +239,17 @@ main(int argc, char **argv) */ run_testspec(testspec); + /* + * Verify that all steps have been used, complaining about anything + * defined but not used. + */ + for (i = 0; i < testspec->nallsteps; i++) + { + if (!testspec->allsteps[i]->used) + fprintf(stderr, "unused step name: %s\n", + testspec->allsteps[i]->name); + } + return 0; } @@ -467,7 +478,11 @@ run_permutation(TestSpec *testspec, int nsteps, Step **steps) printf("\nstarting permutation:"); for (i = 0; i < nsteps; i++) + { + /* Track the permutation as in-use */ + steps[i]->used = true; printf(" %s", steps[i]->name); + } printf("\n"); /* Perform setup */ diff --git a/src/test/isolation/isolationtester.h b/src/test/isolation/isolationtester.h index 7f91e6433f..d9d2a14ecf 100644 --- a/src/test/isolation/isolationtester.h +++ b/src/test/isolation/isolationtester.h @@ -29,6 +29,7 @@ struct Session struct Step { int session; + bool used; char *name; char *sql; char *errormsg; diff --git a/src/test/isolation/specparse.y b/src/test/isolation/specparse.y index fb8a4d706c..2dfe3533ff 100644 --- a/src/test/isolation/specparse.y +++ b/src/test/isolation/specparse.y @@ -145,6 +145,7 @@ step: $$ = pg_malloc(sizeof(Step)); $$->name = $2; $$->sql = $3; + $$->used = false; $$->errormsg = NULL; } ; diff --git a/src/test/isolation/specs/freeze-the-dead.spec b/src/test/isolation/specs/freeze-the-dead.spec index 8c3649902a..915bf15b92 100644 --- a/src/test/isolation/specs/freeze-the-dead.spec +++ b/src/test/isolation/specs/freeze-the-dead.spec @@ -19,7 +19,6 @@ session "s1" step "s1_begin" { BEGIN; } step "s1_update" { UPDATE tab_freeze SET x = x + 1 WHERE id = 3; } step "s1_commit" { COMMIT; } -step "s1_vacuum" { VACUUM FREEZE tab_freeze; } step "s1_selectone" { BEGIN; SET LOCAL enable_seqscan = false; @@ -28,7 +27,6 @@ step "s1_selectone" { COMMIT; } step "s1_selectall" { SELECT * FROM tab_freeze ORDER BY name, id; } -step "s1_reindex" { REINDEX TABLE tab_freeze; } session "s2" step "s2_begin" { BEGIN; } @@ -40,7 +38,6 @@ session "s3" step "s3_begin" { BEGIN; } step "s3_key_share" { SELECT id FROM tab_freeze WHERE id = 3 FOR KEY SHARE; } step "s3_commit" { COMMIT; } -step "s3_vacuum" { VACUUM FREEZE tab_freeze; } # This permutation verifies that a previous bug # https://postgr.es/m/e5711e62-8fdf-4dca-a888-c200bf6b5...@amazon.com diff --git a/src/test/isolation/specs/insert-conflict-do-nothing.spec b/src/test/isolation/specs/insert-conflict-do-nothing.spec index 9b92c35cec..71acc380c7 100644 --- a/src/test/isolation/specs/insert-conflict-do-nothing.spec +++ b/src/test/isolation/specs/insert-conflict-do-nothing.spec @@ -33,7 +33,6 @@ setup step "donothing2" { INSERT INTO ints(key, val) VALUES(1, 'donothing2') ON CONFLICT DO NOTHING; } step "select2" { SELECT * FROM ints; } step "c2" { COMMIT; } -step "a2" { ABORT; } # Regular case where one session block-waits on another to determine if it # should proceed with an insert or do nothing. diff --git a/src/test/isolation/specs/insert-conflict-do-update-2.spec b/src/test/isolation/specs/insert-conflict-do-update-2.spec index f5b4f601b5..12f6be8000 100644 --- a/src/test/isolation/specs/insert-conflict-do-update-2.spec +++ b/src/test/isolation/specs/insert-conflict-do-update-2.spec @@ -32,7 +32,6 @@ setup step "insert2" { INSERT INTO upsert(key, payload) VALUES('FOOFOO', 'insert2') ON CONFLICT (lower(key)) DO UPDATE set key = EXCLUDED.key, payload = upsert.payload || ' updated by insert2'; } step "select2" { SELECT * FROM upsert; } step "c2" { COMMIT; } -step "a2" { ABORT; } # One session (session 2) block-waits on another (session 1) to determine if it # should proceed with an insert or update. The user can still usefully UPDATE diff --git a/src/test/isolation/specs/insert-conflict-do-update.spec b/src/test/isolation/specs/insert-conflict-do-update.spec index 5d335a3444..7c8cb47100 100644 --- a/src/test/isolation/specs/insert-conflict-do-update.spec +++ b/src/test/isolation/specs/insert-conflict-do-update.spec @@ -30,7 +30,6 @@ setup step "insert2" { INSERT INTO upsert(key, val) VALUES(1, 'insert2') ON CONFLICT (key) DO UPDATE set val = upsert.val || ' updated by insert2'; } step "select2" { SELECT * FROM upsert; } step "c2" { COMMIT; } -step "a2" { ABORT; } # One session (session 2) block-waits on another (session 1) to determine if it # should proceed with an insert or update. Notably, this entails updating a diff --git a/src/test/isolation/specs/sequence-ddl.spec b/src/test/isolation/specs/sequence-ddl.spec index a04fd1cc7e..f2a3ff628b 100644 --- a/src/test/isolation/specs/sequence-ddl.spec +++ b/src/test/isolation/specs/sequence-ddl.spec @@ -15,7 +15,6 @@ setup { BEGIN; } step "s1alter" { ALTER SEQUENCE seq1 MAXVALUE 10; } step "s1alter2" { ALTER SEQUENCE seq1 MAXVALUE 20; } step "s1restart" { ALTER SEQUENCE seq1 RESTART WITH 5; } -step "s1setval" { SELECT setval('seq1', 5); } step "s1commit" { COMMIT; } session "s2" diff --git a/src/test/isolation/specs/tuplelock-upgrade-no-deadlock.spec b/src/test/isolation/specs/tuplelock-upgrade-no-deadlock.spec index 73f71b17a7..106c2465c0 100644 --- a/src/test/isolation/specs/tuplelock-upgrade-no-deadlock.spec +++ b/src/test/isolation/specs/tuplelock-upgrade-no-deadlock.spec @@ -19,7 +19,6 @@ teardown session "s0" step "s0_begin" { begin; } step "s0_keyshare" { select id from tlu_job where id = 1 for key share;} -step "s0_share" { select id from tlu_job where id = 1 for share;} step "s0_rollback" { rollback; } session "s1" @@ -28,7 +27,6 @@ step "s1_keyshare" { select id from tlu_job where id = 1 for key share;} step "s1_share" { select id from tlu_job where id = 1 for share; } step "s1_fornokeyupd" { select id from tlu_job where id = 1 for no key update; } step "s1_update" { update tlu_job set name = 'b' where id = 1; } -step "s1_delete" { delete from tlu_job where id = 1; } step "s1_savept_e" { savepoint s1_e; } step "s1_savept_f" { savepoint s1_f; } step "s1_rollback_e" { rollback to s1_e; } @@ -44,7 +42,6 @@ step "s2_for_update" { select id from tlu_job where id = 1 for update; } step "s2_update" { update tlu_job set name = 'b' where id = 1; } step "s2_delete" { delete from tlu_job where id = 1; } step "s2_rollback" { rollback; } -step "s2_commit" { commit; } session "s3" setup { begin; }
signature.asc
Description: PGP signature