On Mon, Aug 19, 2019 at 10:23:19AM -0700, Melanie Plageman wrote: > Could you do the check that all steps have been used in dry_run mode > instead of when running the tests for real?
Sure, I was hesitating to do so. I have no issue in moving the check into run_testspec(). So done as attached. It is rather a pain to pass down custom options to isolationtester. For example, I have tested the updated version attached after hijacking -n into isolation_start_test(). Ugly hack, but for testing that's enough. Do you make use of this tool in a particular way in greenplum? Just wondering. (Could it make sense to have long options for isolationtester by the way?) -- Michael
diff --git a/src/test/isolation/isolationtester.c b/src/test/isolation/isolationtester.c
index f98bb1cf64..f8ec7ca7b4 100644
--- a/src/test/isolation/isolationtester.c
+++ b/src/test/isolation/isolationtester.c
@@ -251,10 +251,24 @@ 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. This check happens here so as dry-run is able
+ * to use it.
+ */
+ for (i = 0; i < testspec->nallsteps; i++)
+ {
+ if (!testspec->allsteps[i]->used)
+ fprintf(stderr, "unused step name: %s\n",
+ testspec->allsteps[i]->name);
+ }
}
/*
@@ -457,7 +471,11 @@ run_permutation(TestSpec *testspec, int nsteps, Step **steps)
{
printf("permutation");
for (i = 0; i < nsteps; i++)
+ {
+ /* Track the permutation as in-use */
+ steps[i]->used = true;
printf(" \"%s\"", steps[i]->name);
+ }
printf("\n");
return;
}
@@ -467,7 +485,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
