diff --git a/test/harness_selftest.sh b/test/harness_selftest.sh index 4881557..981d1cd 100755 --- a/test/harness_selftest.sh +++ b/test/harness_selftest.sh @@ -186,12 +186,108 @@ not_a_suite() { } # the SUITES=( ... ) array, flattened to one name per line +# Ask the runner, rather than parsing its source. stderr is dropped because a +# runner carrying the stray-name mistake reports "command not found" on the way +# past it, which is the diagnosis and not this function's output. +# +# Asked ONCE, for the real runner, and cached. The checks below call this inside +# two loops over every test file, so the first version forked a fresh bash 250-odd +# times. Under a six-way matrix that is slow and, worse, fragile: a transient +# failure to fork returns an empty list, and an empty list reads as "that suite is +# unregistered". It did exactly that in the #473 matrix, failing on PG16 and PG17 +# with four names each, different names each time, while PG15/18/19 passed. An +# intermittent red naming innocent suites is the worst kind, so the premise below +# makes an empty answer say what it is. +_SUITE_LIST="$(bash "$RUNNER" --list-suites 2>/dev/null)" +check "premise: the runner answered --list-suites, so the two checks below mean something" \ + "$([ -n "$_SUITE_LIST" ] && echo yes || echo "no (empty)")" "yes" + listed_suites() { - awk '/^SUITES=\(/,/\)/' "$RUNNER" | tr ' \t' '\n\n' | - sed -e 's/^SUITES=(//' -e 's/)$//' -e 's/\\$//' | - grep -E '^[a-z0-9_]+$' + if [ $# -gt 0 ]; then + bash "$1" --list-suites 2>/dev/null + else + printf '%s\n' "$_SUITE_LIST" + fi } +# ---- the list must be read the way the RUNNER reads it ---------------------- +# +# The two checks below rest on listed_suites, so what listed_suites believes is +# load-bearing. It used to believe its own parser: an awk range plus sed plus +# grep, which is a reimplementation of bash's array parsing, and the two disagree +# on exactly the mistake this project keeps making. +# +# Appending a name AFTER the closing paren is valid shell. `bash -n` passes. To +# bash the name is a stray COMMAND and not a member, so the suite never runs. To +# the awk parser it was a member, so "every suite is registered" passed and the +# suite silently did not run. The tally cannot catch it either, because +# "suites that ran: N of M" takes M from ${#SUITES[@]} and is self-consistent +# with the suite missing. +# +# Measured before this was fixed: bash reported "stray_suite: command not found" +# while listed_suites reported it as registered. +# +# So the fixture below is the real runner with that exact mistake applied, and +# the assertion is that the extraction agrees with bash rather than with awk. +_fx="$(mktemp /tmp/pgc-runner-fixture.XXXXXX.sh)" +awk ' + /^SUITES=\(/ { inarr = 1 } + inarr && /\)/ && !seen { print $0 " stray_not_a_suite"; seen = 1; inarr = 0; next } + { print } +' "$RUNNER" > "$_fx" + +check_num "premise: the fixture really does carry the stray name" \ + "$(grep -c 'stray_not_a_suite' "$_fx")" "1" +check_num "a name after the array's closing paren is not read as a registered suite" \ + "$(listed_suites "$_fx" | grep -cx stray_not_a_suite)" "0" + +# And the mistake is worse than a stray command, which is worth pinning because +# the first version of this test assumed otherwise and asserted the opposite. +# +# SUITES=(alpha beta gamma) stray_name +# -> stray_name: command not found +# -> ${#SUITES[@]} is 0 +# +# `NAME=value cmd` scopes the assignment to that one command, and an array +# literal is no exception. So the name after the paren does not join the array, +# it DESTROYS it: every suite disappears and the matrix would run none of them. +# The runner's "NO SUITES RAN" guard is the backstop for that, and this is what +# stops the registration check above from calling the wreck healthy. +check_num "and the mistake empties the whole array rather than appending to it" \ + "$(listed_suites "$_fx" | grep -c .)" "0" + +# The control has to be a runner that is NOT sabotaged, because for the fixture +# above an empty answer is the correct one. Reading the real runner is what shows +# the extraction can return names at all. +check_num "positive control: the real runner's list is read, and contains isolation" \ + "$(listed_suites | grep -cx isolation)" "1" +check "positive control: and it is a whole list, not one lucky line" \ + "$([ "$(listed_suites | grep -c .)" -gt 50 ] && echo yes || echo no)" "yes" +rm -f "$_fx" + +# ---- the list stays sorted, which is what actually stops the conflicts ------ +# +# One name per line was not enough on its own. Measured, on this repository, by +# branching twice and merging: +# +# one line, both additions on the same line CONFLICT +# one per line, both appended at the end CONFLICT +# one per line + sorted, names far apart clean +# one per line + sorted, names that sort adjacently CONFLICT +# +# Everyone appends at the end, which is the shape all four of #469's conflicts +# had, so one-per-line alone would have left them all conflicting. Sorted gives a +# new suite an insertion point decided by its NAME, so two unrelated additions +# land in different places and merge. It is a large reduction and not a cure: +# two names that sort next to each other still collide. +# +# This check is what keeps the property true. Without it the order decays the +# first time somebody appends by hand, and the reduction quietly goes away. +_sorted_expected="$(listed_suites | sort)" +_sorted_actual="$(listed_suites)" +check "the suite list is sorted, so two new suites land in different places" \ + "$([ "$_sorted_actual" = "$_sorted_expected" ] && echo sorted || echo "not sorted")" "sorted" + unregistered="" for f in "$TESTDIR"/*.sh; do name="$(basename "$f" .sh)" diff --git a/test/run_all_versions.sh b/test/run_all_versions.sh index 393c006..0b3e4f6 100755 --- a/test/run_all_versions.sh +++ b/test/run_all_versions.sh @@ -23,6 +23,154 @@ set -uo pipefail +# One name per line, and it must stay that way. +# +# This was a single backslash-continued line, so every pull request that adds a +# suite edited the same line and any two of them conflicted by construction. It +# happened four times in one day (#459 vs #462, #460 vs #462, #462 vs #468, and +# #444 behind them) and four more times the night #446, #468 and #444 landed. +# +# The resolution was the dangerous part, not the conflict. Appending the new name +# after the closing paren is valid shell that `bash -n` accepts, and it does not +# merely leave a stray command: `NAME=value cmd` scopes the assignment to that +# command, and an array literal is no exception, so `SUITES=(...) my_suite` leaves +# SUITES UNSET and the matrix runs nothing at all. harness_selftest pins that (#469). +# +# One name per line means two pull requests adding two suites touch two different +# lines and merge cleanly. Do not re-flow this into one line to save space. +SUITES=( + advisory_lock_class + alter_column_type + analyze_reltuples + analyze_stats + arrow_export + arrow_import + arrow_nested + arrow_nested_import + audit + bloom_lazy + bloom_setting + bloom_sizing + cancel_decode + column_projection + concurrency + concurrent_diff + corruption + decode_interrupts + differential + docs_style + drop_cleanup + encode_effort + encode_invariants + fk_referencing + fsst_margin + fuzz + fuzz_arrow + fuzz_parquet + generated_columns + hardening + harness_selftest + import_deferred + import_exclusion + index_only + isolation + logical_subscriber + native_agg + native_agg_addcolumn + native_agg_deletes + native_backend_crash + native_bloom + native_cancel + native_cluster + native_compact + native_ctas + native_dml + native_encoding + native_fastdecode + native_fetch_bigcap + native_fetch_cache + native_fetch_interrupt + native_fetch_position + native_fetch_projection + native_format + native_gap + native_groupagg + native_index + native_index_projection + native_ios + native_lazy_slot + native_ownership + native_parquet_codecs + native_parquet_fdw + native_parquet_flba + native_parquet_hardening + native_parquet_multifile + native_parquet_partition + native_parquet_projection + native_parquet_pushdown + native_parquet_schema + native_parquet_stack + native_parquet_streaming + native_parquet_units + native_projection + native_read_parquet + native_reclaim + native_reclaim_cycles + native_reclaim_frag + native_reclaim_reconcile + native_recluster + native_repack + native_rewrite + native_rewrite_conc + native_roundtrip + native_skip + native_sort_by + native_truncate + native_vacuum_race + native_vecskip + native_writer + native_zonemap + objstore_module + objstore_stash_recovery + parallel + parallel_copy + parallel_degree + parallel_export_parquet + parallel_vector_agg + parquet_export + parquet_import + parquet_nested + parquet_nested_import + pg19_vacuum_options + pg_dump_roundtrip + phase2 + phase3 + phase4 + phase5 + phase6 + planner_choice_quality + projections + pushdown_report + read_stream + recluster_extent + recovery + replication + rewrite_group_scan + row_triggers + server_file_privilege + smoke + sort_status + sorted_projection + temporal + ungrouped_vector_agg + unique_conc + wal_envelope + write_fsst_compressed + write_minmax_fastpath + zonemap_cost +) + + # --------------------------------------------------------------------------- # Run from a private copy of this script, and refuse to run twice at once. # @@ -52,6 +200,22 @@ PGC_RUN_LOCK="${PGC_RUN_LOCK:-/tmp/pgcolumnar-run_all_versions.lock}" # pattern misses it, and killing postmasters directly bypasses pg_ctl. This reads # the lock, signals the owner, and lets the owner's own trap stop the suites and # their clusters properly. +# --list-suites: print the matrix's suite list, one name per line, then exit. +# +# It exists so that nothing has to parse this array a second time. A gate that +# re-implements bash's array parsing in awk disagrees with bash on the very +# mistake this array invites: a name after the closing paren is a stray COMMAND +# to bash and a member to a text parser, so the gate passed while the suite +# silently never ran. Asking the runner means there is one parser, bash's, and +# no way for the two to drift. +# +# Handled BEFORE the run lock on purpose. harness_selftest calls this from +# inside a running matrix, and taking the lock there would refuse to answer. +if [ "${1:-}" = "--list-suites" ]; then + printf '%s\n' "${SUITES[@]}" + exit 0 +fi + if [ "${1:-}" = "--stop" ]; then if [ ! -e "$PGC_RUN_LOCK" ]; then echo "no matrix run in progress (no lock at $PGC_RUN_LOCK)" @@ -205,10 +369,6 @@ pgc_run_cleanup() { trap pgc_run_cleanup INT TERM SRCDIR="${PGC_RUN_SRCDIR:-$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)}" -SUITES=(harness_selftest docs_style smoke phase2 phase3 phase4 phase5 phase6 audit concurrency unique_conc \ - differential recovery replication native_backend_crash fuzz fuzz_parquet fuzz_arrow hardening concurrent_diff parallel sorted_projection \ - arrow_export parquet_export read_stream corruption \ - generated_columns temporal arrow_import index_only projections arrow_nested parquet_import parquet_nested arrow_nested_import parquet_nested_import native_writer native_roundtrip native_encoding native_fastdecode native_zonemap write_minmax_fastpath write_fsst_compressed fsst_margin encode_invariants encode_effort native_skip pushdown_report zonemap_cost native_agg native_agg_deletes native_agg_addcolumn native_groupagg ungrouped_vector_agg parallel_vector_agg native_bloom bloom_sizing bloom_setting bloom_lazy native_vecskip native_index native_index_projection native_fetch_position native_dml alter_column_type native_ios native_projection native_cluster pg19_vacuum_options native_repack native_compact native_recluster recluster_extent native_vacuum_race native_sort_by sort_status native_reclaim native_ownership drop_cleanup pg_dump_roundtrip native_reclaim_cycles native_reclaim_frag native_reclaim_reconcile native_gap native_format native_truncate native_rewrite native_rewrite_conc rewrite_group_scan native_parquet_schema native_read_parquet native_parquet_fdw native_parquet_pushdown native_parquet_hardening server_file_privilege native_parquet_stack native_parquet_units native_parquet_flba native_parquet_codecs native_parquet_projection native_parquet_multifile native_parquet_streaming native_parquet_partition native_cancel cancel_decode wal_envelope decode_interrupts import_exclusion import_deferred parallel_copy parallel_export_parquet fk_referencing row_triggers native_lazy_slot native_ctas native_fetch_cache native_fetch_bigcap native_fetch_interrupt analyze_stats analyze_reltuples native_fetch_projection column_projection advisory_lock_class logical_subscriber parallel_degree planner_choice_quality objstore_module objstore_stash_recovery isolation) # Default matrix: one assert-enabled pg_config per major, 15 through 19. DEFAULT_CONFIGS=(