From 1862409f720019249f22b059eaecc8fa11294c74 Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <dgustafsson@postgresql.org>
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)

