On 09.10.23 11:20, Alvaro Herrera wrote:
I tried this out. I agree it's a good change. BTW, this made me
realize that "unlike" is not a good name: maybe it should be called
"except".
right
I would add quotes to the words "like" and "unlike" there. Otherwise,
these sentences are hard to parse. Also, some commentary on what this
is about seems warranted: maybe "Check that this test properly defines
which dumps the output should match on." or similar.
Done.
I also moved the code a bit earlier, before the checks for supported
compression libraries etc., so it runs even if those cause a skip.
I didn't like using diag(), because automated runs will not alert to any
problems. Now maybe that's not critical, but I fear that people would
not notice problems if they are just noise in the output. Let's make
them test errors. fail() seems good enough: with the lines I quote
above and omitting the test corrections, I get this, which seems good
enough:
After researching this a bit more, I think "die" is the convention for
problems in the test definitions themselves. (Otherwise, you're writing
a test about the tests, which would be a bit weird.) The result is
approximately the same.
From ec4380986519f966303c14ea49223f0cf7729220 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Tue, 10 Oct 2023 09:54:43 +0200
Subject: [PATCH v2] Clean up some pg_dump tests
1) Remove useless entries from "unlike" lists. Runs that are not
listed in "like" don't need to be excluded in "unlike".
2) Ensure there is always a "like" list, even if it is empty. This
makes the test more self-documenting.
3) Use predefined lists such as %full_runs where appropriate, instead
of listing all runs separately.
Also add code that checks 1 and 2 automatically and dies with an error
for violations.
Discussion:
https://www.postgresql.org/message-id/flat/[email protected]
---
src/bin/pg_dump/t/002_pg_dump.pl | 78 +++++++++-----------------------
1 file changed, 21 insertions(+), 57 deletions(-)
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index 326c9a7639..eb3ec534b4 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -818,7 +818,7 @@
regexp => qr/^\QALTER COLLATION public.test0 OWNER TO \E.+;/m,
collation => 1,
like => { %full_runs, section_pre_data => 1, },
- unlike => { %dump_test_schema_runs, no_owner => 1, },
+ unlike => { no_owner => 1, },
},
'ALTER FOREIGN DATA WRAPPER dummy OWNER TO' => {
@@ -977,7 +977,7 @@
create_sql =>
'ALTER SCHEMA public OWNER TO "regress_quoted \"" role";',
regexp => qr/^(GRANT|REVOKE)/m,
- unlike => { defaults_public_owner => 1 },
+ like => {},
},
'ALTER SEQUENCE test_table_col1_seq' => {
@@ -1285,9 +1285,7 @@
{ %full_runs, %dump_test_schema_runs, section_pre_data => 1,
},
unlike => {
exclude_dump_test_schema => 1,
- only_dump_test_table => 1,
no_owner => 1,
- role => 1,
only_dump_measurement => 1,
},
},
@@ -1351,7 +1349,6 @@
binary_upgrade => 1,
no_large_objects => 1,
schema_only => 1,
- section_pre_data => 1,
},
},
@@ -3210,7 +3207,6 @@
binary_upgrade => 1,
exclude_dump_test_schema => 1,
schema_only => 1,
- only_dump_measurement => 1,
},
},
@@ -3457,7 +3453,6 @@
'Disabled trigger on partition is not created' => {
regexp => qr/CREATE TRIGGER test_trigger.*ON
dump_test_second_schema/,
like => {},
- unlike => { %full_runs, %dump_test_schema_runs },
},
# Triggers on partitions should not be dropped individually
@@ -3834,35 +3829,12 @@
\QCREATE INDEX measurement_city_id_logdate_idx ON ONLY
dump_test.measurement USING\E
/xm,
like => {
- binary_upgrade => 1,
- clean => 1,
- clean_if_exists => 1,
- compression => 1,
- createdb => 1,
- defaults => 1,
- exclude_test_table => 1,
- exclude_test_table_data => 1,
- no_toast_compression => 1,
- no_large_objects => 1,
- no_privs => 1,
- no_owner => 1,
- no_table_access_method => 1,
- only_dump_test_schema => 1,
- pg_dumpall_dbprivs => 1,
- pg_dumpall_exclude => 1,
- schema_only => 1,
+ %full_runs,
+ %dump_test_schema_runs,
section_post_data => 1,
- test_schema_plus_large_objects => 1,
- only_dump_measurement => 1,
- exclude_measurement_data => 1,
},
unlike => {
exclude_dump_test_schema => 1,
- only_dump_test_table => 1,
- pg_dumpall_globals => 1,
- pg_dumpall_globals_clean => 1,
- role => 1,
- section_pre_data => 1,
exclude_measurement => 1,
},
},
@@ -3913,7 +3885,6 @@
role => 1,
section_post_data => 1,
only_dump_measurement => 1,
- exclude_measurement_data => 1,
},
unlike => {
exclude_measurement => 1,
@@ -3927,35 +3898,12 @@
\QALTER INDEX dump_test.measurement_pkey ATTACH PARTITION
dump_test_second_schema.measurement_y2006m2_pkey\E
/xm,
like => {
- binary_upgrade => 1,
- clean => 1,
- clean_if_exists => 1,
- compression => 1,
- createdb => 1,
- defaults => 1,
- exclude_dump_test_schema => 1,
- exclude_test_table => 1,
- exclude_test_table_data => 1,
- no_toast_compression => 1,
- no_large_objects => 1,
- no_privs => 1,
- no_owner => 1,
- no_table_access_method => 1,
- pg_dumpall_dbprivs => 1,
- pg_dumpall_exclude => 1,
+ %full_runs,
role => 1,
- schema_only => 1,
section_post_data => 1,
only_dump_measurement => 1,
- exclude_measurement_data => 1,
},
unlike => {
- only_dump_test_schema => 1,
- only_dump_test_table => 1,
- pg_dumpall_globals => 1,
- pg_dumpall_globals_clean => 1,
- section_pre_data => 1,
- test_schema_plus_large_objects => 1,
exclude_measurement => 1,
},
},
@@ -4929,6 +4877,22 @@
$test_db = $tests{$test}->{database};
}
+ # Check for proper test definitions
+ #
+ # There should be a "like" list, even if it is empty. (This
+ # makes the test more self-documenting.)
+ if (!defined($tests{$test}->{like}))
+ {
+ die "missing \"like\" in test \"$test\"";
+ }
+ # Check for useless entries in "unlike" list. Runs that are
+ # not listed in "like" don't need to be excluded in "unlike".
+ if ($tests{$test}->{unlike}->{$test_key} &&
+ !defined($tests{$test}->{like}->{$test_key}))
+ {
+ die "useless \"unlike\" entry \"$test_key\" in test
\"$test\"";
+ }
+
# Skip any collation-related commands if there is no collation
support
if (!$collation_support && defined($tests{$test}->{collation}))
{
--
2.42.0
