Detect unused steps in isolation specs and do some cleanup

This is useful for developers to find out if an isolation spec is
over-engineered or if it needs more work by warning at the end of a
test run if a step is not used, generating a failure with extra diffs.

While on it, clean up all the specs which include steps not used in any
permutations to simplify them.

Author: Michael Paquier
Reviewed-by: Asim Praveen, Melanie Plageman
Discussion: https://postgr.es/m/20190819080820.GG18166@paquier.xyz

diff --git a/src/test/isolation/isolationtester.c b/src/test/isolation/isolationtester.c
index 66ebe3b27e9153f9feb28317aeb551cc5c9ffd76..556b46d93f401563714f78254aa79db0abb5b106 100644 (file)
--- a/src/test/isolation/isolationtester.c
+++ b/src/test/isolation/isolationtester.c
@@ -81,7 +81,7 @@ main(int argc, char **argv)
                                puts("isolationtester (PostgreSQL) " PG_VERSION);
                                exit(0);
                        default:
-                               fprintf(stderr, "Usage: isolationtester [-n] [CONNINFO]\n");
+                               fprintf(stderr, "Usage: isolationtester [CONNINFO]\n");
                                return EXIT_FAILURE;
                }
        }
@@ -235,10 +235,23 @@ static int *piles;
 static void
 run_testspec(TestSpec *testspec)
 {
+       int                     i;
+
        if (testspec->permutations)
                run_named_permutations(testspec);
        else
                run_all_permutations(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);
+       }
 }
 
 /*
@@ -438,7 +451,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 7f91e6433ffa7128d5d31d5a1f1352fad8ce6246..d9d2a14ecf4db4af60808de9dd031fb984f352d3 100644 (file)
--- 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 fb8a4d706c0ec7ed0e3103517e75beacb03bdc7f..2dfe3533ff724b9fa91f6c2f8200bc76d4dc996c 100644 (file)
--- 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 8c3649902af388f845ebaeb59f2555f87e287daf..915bf15b9251497dc765341f5b6a30eaeae2c1b9 100644 (file)
--- 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-C200BF6B5742@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 9b92c35cec6654c5c2b9625129834adcc8db0d4d..71acc380c7a941d3bb9641024f5566229b024337 100644 (file)
--- 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 f5b4f601b58c88838fe8421928389d47f2eac1a6..12f6be8000f990cac5f2b8fac77fff860962a003 100644 (file)
--- 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 5d335a34449574056ee7da9d362d05aec7419390..7c8cb4710025318fda5c32db8376d890d267d2f3 100644 (file)
--- 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 a04fd1cc7e8cb1d9fbcc26f1d0d57d7c7ea8ad7e..f2a3ff628bfabf78236257df43fafebe98339153 100644 (file)
--- 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 73f71b17a7afb4bcdc298e2a2634022547281949..106c2465c0aebabe26212be55f8dc707c677abc5 100644 (file)
--- 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; }