Add a CI check to see if all tests are part of a schedule (#3959)

I recently forgot to add tests to a schedule in two of my PRs. One of
these was caught by review, but the other one was not. This adds a
script to causes CI to ensure that each test in the repo is included in
at least one schedule.

Three tests were found that were currently not part of a schedule. This PR
adds those three tests to a schedule as well and it also fixes some small
issues with these tests.

.circleci/config.yml

@@ -53,6 +53,12 @@ jobs:
       - run:
           name: 'Check for banned C API usage'
           command: ci/banned.h.sh
+      - run:
+          name: 'Check for tests missing in schedules'
+          command: ci/check_all_tests_are_run.sh
+      - run:
+          name: 'Check if all CI scripts are actually run'
+          command: ci/check_all_ci_scripts_are_run.sh
   check-sql-snapshots:
     docker:
       - image: 'citus/extbuilder:latest'

ci/README.md

@@ -114,6 +114,22 @@ means that `latest.sql` is not up to date with the SQL file of the highest
 version number in the directory. The output of the script shows you what is
 different.
 
+## `check_all_tests_are_run.sh`
+
+A test should always be included in a schedule file, otherwise it will not be
+run in CI. This is most commonly forgotten for newly added tests. In that case
+the dev ran it locally without running a full schedule with something like:
+```bash
+make -C src/test/regress/ check-minimal EXTRA_TESTS='multi_create_table_new_features'
+```
+
+## `check_all_ci_scripts_are_run.sh`
+
+This is the meta CI script. This checks that all existing CI scripts are
+actually run in CI. This is most commonly forgotten for newly added CI tests
+that the developer only ran locally. It also checks that all CI scripts have a
+section in this `README.md` file and that they include `ci/ci_helpers.sh`.
+
 ## `disallow_c_comments_in_migrations.sh`
 
 We do not use C-style comments in migration files as the stripped

ci/check_all_ci_scripts_are_run.sh

@@ -0,0 +1,29 @@
+#!/bin/bash
+set -euo pipefail
+
+# shellcheck disable=SC1091
+source ci/ci_helpers.sh
+
+
+# 1. Find all *.sh files in the ci directory
+# 2. Strip the directory
+# 3. Exclude some scripts that we should not run in CI directly
+ci_scripts=$(
+    find ci/ -iname "*.sh" |
+    sed -E 's#^ci/##g' |
+    grep -v -E '^(ci_helpers.sh|fix_style.sh)$'
+)
+for script in $ci_scripts; do
+    if ! grep "\\bci/$script\\b" .circleci/config.yml > /dev/null; then
+        echo "ERROR: CI script with name \"$script\" is not actually used in .circleci/config.yml"
+        exit 1
+    fi
+    if ! grep "^## \`$script\`\$" ci/README.md > /dev/null; then
+        echo "ERROR: CI script with name \"$script\" does not have a section in ci/README.md"
+        exit 1
+    fi
+    if ! grep "source ci/ci_helpers.sh" "ci/$script" > /dev/null; then
+        echo "ERROR: CI script with name \"$script\" does not include ci/ci_helpers.sh"
+        exit 1
+    fi
+done

ci/check_all_tests_are_run.sh

@@ -0,0 +1,25 @@
+#!/bin/bash
+set -euo pipefail
+
+# shellcheck disable=SC1091
+source ci/ci_helpers.sh
+
+
+cd src/test/regress
+
+# 1. Find all *.sql *.spec and *.source files in the sql, spec and input
+#    directories
+# 2. Strip the extension and the directory
+# 3. Ignore names that end with .include, those files are meant to be in an C
+#    preprocessor #include statement. They should not be in schedules.
+test_names=$(
+    find sql spec input -iname "*.sql" -o -iname "*.spec" -o -iname "*.source" |
+    sed -E 's#^\w+/([^/]+)\.[^.]+$#\1#g' |
+    grep -v '.include$'
+)
+for name in $test_names; do
+    if ! grep "\\b$name\\b" ./*_schedule > /dev/null; then
+        echo "ERROR: Test with name \"$name\" is not used in any of the schedule files"
+        exit 1
+    fi
+done

src/test/regress/Makefile

@@ -111,6 +111,11 @@ check-base-mx: all
 	$(pg_regress_multi_check) --load-extension=citus \
 	-- $(MULTI_REGRESS_OPTS) --schedule=$(citus_abs_srcdir)/mx_base_schedule $(EXTRA_TESTS)
 
+check-minimal-mx: all
+	$(pg_regress_multi_check) --load-extension=citus \
+	-- $(MULTI_REGRESS_OPTS) --schedule=$(citus_abs_srcdir)/mx_minimal_schedule $(EXTRA_TESTS)
+
+
 check-empty: all
 	$(pg_regress_multi_check) --load-extension=citus \
 	-- $(MULTI_REGRESS_OPTS) $(EXTRA_TESTS)

src/test/regress/expected/isolation_cancellation.out

@@ -12,7 +12,6 @@ step s1-reset:
     RESET ALL;
 
 step s1-drop:
-
     DROP TABLE cancel_table;
 
 
@@ -28,7 +27,6 @@ step s1-reset:
     RESET ALL;
 
 step s2-drop:
-
     DROP TABLE cancel_table;
 
 
@@ -50,7 +48,6 @@ step s1-reset:
     RESET ALL;
 
 step s1-drop:
-
     DROP TABLE cancel_table;
 
 
@@ -72,7 +69,6 @@ step s1-reset:
     RESET ALL;
 
 step s2-drop:
-
     DROP TABLE cancel_table;
 
 
@@ -97,7 +93,6 @@ step s1-reset:
     RESET ALL;
 
 step s1-drop:
-
     DROP TABLE cancel_table;
 
 
@@ -122,6 +117,5 @@ step s1-reset:
     RESET ALL;
 
 step s2-drop:
-
     DROP TABLE cancel_table;
 

src/test/regress/expected/multi_create_table_new_features.out

@@ -6,13 +6,13 @@
 CREATE TABLE table_identity_col (
     id integer GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY,
     payload text );
-SELECT master_create_distributed_table('table_identity_col', 'id', 'append');
+SELECT create_distributed_table('table_identity_col', 'id', 'append');
 ERROR:  cannot distribute relation: table_identity_col
 DETAIL:  Distributed relations must not use GENERATED ... AS IDENTITY.
 SELECT create_distributed_table('table_identity_col', 'id');
 ERROR:  cannot distribute relation: table_identity_col
 DETAIL:  Distributed relations must not use GENERATED ... AS IDENTITY.
-SELECT create_distributed_table('table_identity_col', 'text');
+SELECT create_distributed_table('table_identity_col', 'payload');
 ERROR:  cannot distribute relation: table_identity_col
 DETAIL:  Distributed relations must not use GENERATED ... AS IDENTITY.
 SELECT create_reference_table('table_identity_col');

src/test/regress/isolation_schedule

@@ -62,6 +62,7 @@ test: isolation_insert_select_conflict
 test: isolation_ref2ref_foreign_keys
 test: isolation_multiuser_locking
 test: shared_connection_waits
+test: isolation_cancellation
 
 # MX tests
 test: isolation_reference_on_mx

src/test/regress/multi_schedule

@@ -46,7 +46,7 @@ test: multi_behavioral_analytics_basics multi_behavioral_analytics_single_shard_
 test: multi_shard_update_delete recursive_dml_with_different_planners_executors
 test: insert_select_repartition window_functions dml_recursive multi_insert_select_window
 test: multi_insert_select_conflict create_table_triggers
-test: multi_row_insert insert_select_into_local_table
+test: multi_row_insert insert_select_into_local_table multi_create_table_new_features
 
 # following should not run in parallel because it relies on connection counts to workers
 test: insert_select_connection_leak

src/test/regress/mx_minimal_schedule

@@ -0,0 +1,9 @@
+# ----------
+# Only run few basic tests to set up a testing environment
+# ----------
+test: multi_cluster_management
+test: multi_test_helpers multi_test_helpers_superuser
+test: multi_test_catalog_views
+
+# the following test has to be run sequentially
+test: base_enable_mx

src/test/regress/sql/multi_create_table_new_features.sql

@@ -12,6 +12,6 @@ CREATE TABLE table_identity_col (
 SELECT create_distributed_table('table_identity_col', 'id', 'append');
 
 SELECT create_distributed_table('table_identity_col', 'id');
-SELECT create_distributed_table('table_identity_col', 'text');
+SELECT create_distributed_table('table_identity_col', 'payload');
 
 SELECT create_reference_table('table_identity_col');