From 74f9a88c6f7ddfe26019dbd50f98c2789029ad9f Mon Sep 17 00:00:00 2001 From: Ashutosh Bapat Date: Fri, 20 Dec 2024 15:22:14 +0530 Subject: [PATCH 2/2] Address comments by Daniel Gustafsson To be merged with the earlier commit. --- doc/src/sgml/regress.sgml | 3 ++- src/bin/pg_upgrade/t/002_pg_upgrade.pl | 14 ++++++-------- src/test/perl/PostgreSQL/Test/AdjustDump.pm | 2 +- src/test/perl/PostgreSQL/Test/Utils.pm | 7 +++++-- 4 files changed, 14 insertions(+), 12 deletions(-) diff --git a/doc/src/sgml/regress.sgml b/doc/src/sgml/regress.sgml index 4be5d2d7d52..60da8eb95e5 100644 --- a/doc/src/sgml/regress.sgml +++ b/doc/src/sgml/regress.sgml @@ -343,7 +343,8 @@ make check-world PG_TEST_EXTRA='kerberos ldap ssl load_balance libpq_encryption' When enabled, src/bin/pg_upgrade/t/002_pg_upgrade.pl tests dump and restore of regression database left behind by the - regression run. Not enabled by default because it is time consuming. + regression run. Not enabled by default because it is time and resource + consuming. diff --git a/src/bin/pg_upgrade/t/002_pg_upgrade.pl b/src/bin/pg_upgrade/t/002_pg_upgrade.pl index 42b68527146..a817ed0d00b 100644 --- a/src/bin/pg_upgrade/t/002_pg_upgrade.pl +++ b/src/bin/pg_upgrade/t/002_pg_upgrade.pl @@ -38,7 +38,7 @@ sub generate_db # Filter the contents of a dump before its use in a content comparison for # upgrade testing. This returns the path to the filtered dump. -sub filter_dump_for_upgrade +sub filter_dump { my ($is_old, $old_version, $dump_file) = @_; my $dump_contents = slurp_file($dump_file); @@ -525,10 +525,8 @@ push(@dump_command, '--extra-float-digits', '0') $newnode->command_ok(\@dump_command, 'dump after running pg_upgrade'); # Filter the contents of the dumps. -my $dump1_filtered = - filter_dump_for_upgrade(1, $oldnode->pg_version, $dump1_file); -my $dump2_filtered = - filter_dump_for_upgrade(0, $oldnode->pg_version, $dump2_file); +my $dump1_filtered = filter_dump(1, $oldnode->pg_version, $dump1_file); +my $dump2_filtered = filter_dump(0, $oldnode->pg_version, $dump2_file); # Compare the two dumps, there should be no differences. compare_dumps($dump1_filtered, $dump2_filtered, @@ -545,6 +543,7 @@ sub test_regression_dump_restore { my ($src_node, %node_params) = @_; my $dst_node = PostgreSQL::Test::Cluster->new('dst_node'); + my %dump_formats = ('plain' => 'p', 'tar' => 't', 'directory' => 'd', 'custom' => 'c'); # Dump the original database for comparison later. my $src_dump = get_dump_for_comparison($src_node->connstr('regression'), @@ -554,10 +553,9 @@ sub test_regression_dump_restore $dst_node->init(%node_params); $dst_node->start; - for my $format ('plain', 'tar', 'directory', 'custom') + while (my ($format, $format_spec) = each %dump_formats) { my $dump_file = "$tempdir/regression_dump.$format"; - my $format_spec = substr($format, 0, 1); my $restored_db = 'regression_' . $format; # Even though we compare only schema from the original and the restored @@ -628,7 +626,7 @@ sub get_dump_for_comparison 'dump for comparison succeeded'); open(my $dh, '>', $dump_adjusted) - || die "opening $dump_adjusted "; + || die "could not open $dump_adjusted for writing the adjusted dump: $!"; print $dh adjust_regress_dumpfile(slurp_file($dumpfile), $adjust_child_columns); close($dh); diff --git a/src/test/perl/PostgreSQL/Test/AdjustDump.pm b/src/test/perl/PostgreSQL/Test/AdjustDump.pm index 0b0abb0cefc..5b9990e4719 100644 --- a/src/test/perl/PostgreSQL/Test/AdjustDump.pm +++ b/src/test/perl/PostgreSQL/Test/AdjustDump.pm @@ -18,7 +18,7 @@ PostgreSQL::Test::AdjustDump - helper module for dump and restore tests =head1 DESCRIPTION C encapsulates various hacks needed to -compare the results of dump and retore tests +compare the results of dump and restore tests =cut diff --git a/src/test/perl/PostgreSQL/Test/Utils.pm b/src/test/perl/PostgreSQL/Test/Utils.pm index 6efe5faf77d..bf56eb4b23c 100644 --- a/src/test/perl/PostgreSQL/Test/Utils.pm +++ b/src/test/perl/PostgreSQL/Test/Utils.pm @@ -1120,8 +1120,11 @@ sub compare_dumps } else { - note('first dump file: ' . $dump1); - note('second dump file: ' . $dump2); + # Fail if the comparison succeeds because the files are the same. This + # will detect simple programming errors. It won't detect more complex + # errors like passing different links pointing to the same underlying + # file. + ok($dump1 ne $dump2, "dump files being compared are distinct") } return; -- 2.34.1