From 1862409f720019249f22b059eaecc8fa11294c74 Mon Sep 17 00:00:00 2001 From: Daniel Gustafsson Date: Fri, 1 Dec 2023 12:10:17 +0100 Subject: [PATCH v4 2/2] pg_regress: Detect global objects left over after test The project policy is that regress tests should not leave any global objects behind, but there was no check enforcing this. pg_regress will now extract a list of roles, tablespaces and subscriptions before and after running the test(s), and then compares the lists to ensure no new objects are present. Discussion: https://postgr.es/m/CA+TgmoZ+K2OU0Ojs8_4SgqPWJJq7e0XiLd-uJGZ11R_WKF1URQ@mail.gmail.com --- src/test/regress/pg_regress.c | 89 +++++++++++++++++++++++++++++++++++ 1 file changed, 89 insertions(+) diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c index a9b8246cb7..c30a853c8c 100644 --- a/src/test/regress/pg_regress.c +++ b/src/test/regress/pg_regress.c @@ -132,6 +132,8 @@ static char sockself[MAXPGPATH]; static char socklock[MAXPGPATH]; static StringInfo failed_tests = NULL; static bool in_note = false; +static _stringlist *globals_before = NULL; +static _stringlist *globals_after = NULL; static _resultmap *resultmap = NULL; @@ -244,6 +246,60 @@ split_to_stringlist(const char *s, const char *delim, _stringlist **listhead) free(sc); } +/* + * Query the cluster and save the first column of the results into a + * stringlist. + */ +static void +query_to_stringlist(const char *dbname, const char *query, _stringlist **listhead) +{ + PGconn *conn; + PGresult *res; + StringInfo buf = makeStringInfo(); + + appendStringInfo(buf, "dbname=%s", dbname); + + conn = PQconnectdb(buf->data); + if (PQstatus(conn) != CONNECTION_OK) + bail("unable to connect to cluster: %s", PQerrorMessage(conn)); + + res = PQexec(conn, query); + if (PQresultStatus(res) != PGRES_TUPLES_OK) + bail("unable to query database: %s", PQerrorMessage(conn)); + + for (int i = 0; i < PQntuples(res); i++) + add_stringlist_item(listhead, pstrdup(PQgetvalue(res, i, 0))); + + pfree(buf->data); + PQclear(res); + PQfinish(conn); +} + +/* + * Compare two stringlists for string equality + */ +static bool +diff_stringlist(_stringlist *a, _stringlist *b) +{ + _stringlist *sla = a; + _stringlist *slb = b; + + for (; sla && slb; sla = sla->next, slb = slb->next) + { + if (strcmp(sla->str, slb->str) != 0) + { + diag("left over global object detected: %s", slb->str); + return false; + } + } + + /* If we reached the end of both lists at the same time, they are equal */ + if (sla == NULL && slb == NULL) + return true; + + return false; +} + /* * Bailing out is for unrecoverable errors which prevents further testing to * occur and after which the test run should be aborted. By passing noatexit @@ -2608,6 +2664,18 @@ regression_main(int argc, char *argv[], create_role(sl->str, dblist); } + /* + * Store the global objects before the test starts such that we can check + * for any objects left behind after the tests finish. + */ + query_to_stringlist("postgres", + "(SELECT rolname AS obj FROM pg_catalog.pg_roles ORDER BY 1) " + "UNION ALL " + "(SELECT spcname AS obj FROM pg_catalog.pg_tablespace ORDER BY 1) " + "UNION ALL " + "(SELECT subname AS obj FROM pg_catalog.pg_subscription ORDER BY 1)", + &globals_before); + /* * Ready to run the tests */ @@ -2621,6 +2689,27 @@ regression_main(int argc, char *argv[], run_single_test(sl->str, startfunc, postfunc); } + /* + * Store the global objects again after the tests have finished and + * compare with the list from before the tests. + */ + query_to_stringlist("postgres", + "(SELECT rolname AS obj FROM pg_catalog.pg_roles ORDER BY 1) " + "UNION ALL " + "(SELECT spcname AS obj FROM pg_catalog.pg_tablespace ORDER BY 1) " + "UNION ALL " + "(SELECT subname AS obj FROM pg_catalog.pg_subscription ORDER BY 1)", + &globals_after); + + if (!diff_stringlist(globals_before, globals_after)) + { + test_status_failed("global_objects", 0.0, false); + } + else + { + test_status_ok("global_objects", 0.0, false); + } + /* * Shut down temp installation's postmaster */ -- 2.32.1 (Apple Git-133)