On Sat, May 28, 2022 at 04:14:01PM -0400, Tom Lane wrote: > Yeah, I'd noticed the obsoleted comments too, but not bothered to complain > since that was just WIP and not an officially proposed patch. I'll be > happy to review if you want to put up a full patch.
Well, here is a formal patch set, then. Please feel free to comment. FWIW, I am on the fence with dropping TESTDIR, as it could be used by out-of-core test code as well. If there are doubts about back-patching the first part, doing that only on HEAD would be fine to fix the problem of this thread. -- Michael
From 3e6b833deafdfafed949abae49f171c8def689dc Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Tue, 31 May 2022 15:44:49 +0900
Subject: [PATCH v3 1/2] Add TESTOUTDIR as directory for the output of the TAP
tests
TESTDIR is the directory of the build, while TESTOUTDIR would be
generally $TESTDIR/tmp_check/.
---
src/bin/psql/t/010_tab_completion.pl | 40 +++++++++++++-------------
src/test/perl/PostgreSQL/Test/Utils.pm | 4 +--
src/Makefile.global.in | 11 ++++---
src/tools/msvc/vcregress.pl | 2 ++
4 files changed, 31 insertions(+), 26 deletions(-)
diff --git a/src/bin/psql/t/010_tab_completion.pl b/src/bin/psql/t/010_tab_completion.pl
index 2eea515e87..1f41ce47e6 100644
--- a/src/bin/psql/t/010_tab_completion.pl
+++ b/src/bin/psql/t/010_tab_completion.pl
@@ -67,26 +67,26 @@ delete $ENV{TERM};
delete $ENV{LS_COLORS};
# In a VPATH build, we'll be started in the source directory, but we want
-# to run in the build directory so that we can use relative paths to
-# access the tmp_check subdirectory; otherwise the output from filename
-# completion tests is too variable.
-if ($ENV{TESTDIR})
+# to run in the test output directory so that we can use relative paths from
+# the tmp_check subdirectory; otherwise the output from filename completion
+# tests is too variable.
+if ($ENV{TESTOUTDIR})
{
- chdir $ENV{TESTDIR} or die "could not chdir to \"$ENV{TESTDIR}\": $!";
+ chdir "$ENV{TESTOUTDIR}" or die "could not chdir to \"$ENV{TESTOUTDIR}\": $!";
}
# Create some junk files for filename completion testing.
my $FH;
-open $FH, ">", "tmp_check/somefile"
- or die("could not create file \"tmp_check/somefile\": $!");
+open $FH, ">", "somefile"
+ or die("could not create file \"somefile\": $!");
print $FH "some stuff\n";
close $FH;
-open $FH, ">", "tmp_check/afile123"
- or die("could not create file \"tmp_check/afile123\": $!");
+open $FH, ">", "afile123"
+ or die("could not create file \"afile123\": $!");
print $FH "more stuff\n";
close $FH;
-open $FH, ">", "tmp_check/afile456"
- or die("could not create file \"tmp_check/afile456\": $!");
+open $FH, ">", "afile456"
+ or die("could not create file \"afile456\": $!");
print $FH "other stuff\n";
close $FH;
@@ -272,16 +272,16 @@ clear_query();
# check filename completion
check_completion(
- "\\lo_import tmp_check/some\t",
- qr|tmp_check/somefile |,
+ "\\lo_import some\t",
+ qr|somefile |,
"filename completion with one possibility");
clear_query();
# note: readline might print a bell before the completion
check_completion(
- "\\lo_import tmp_check/af\t",
- qr|tmp_check/af\a?ile|,
+ "\\lo_import af\t",
+ qr|af\a?ile|,
"filename completion with multiple possibilities");
# broken versions of libedit require clear_line not clear_query here
@@ -291,15 +291,15 @@ clear_line();
# note: broken versions of libedit want to backslash the closing quote;
# not much we can do about that
check_completion(
- "COPY foo FROM tmp_check/some\t",
- qr|'tmp_check/somefile\\?' |,
+ "COPY foo FROM some\t",
+ qr|'somefile\\?' |,
"quoted filename completion with one possibility");
clear_line();
check_completion(
- "COPY foo FROM tmp_check/af\t",
- qr|'tmp_check/afile|,
+ "COPY foo FROM af\t",
+ qr|'afile|,
"quoted filename completion with multiple possibilities");
# some versions of readline/libedit require two tabs here, some only need one
@@ -307,7 +307,7 @@ check_completion(
# the quotes might appear, too
check_completion(
"\t\t",
- qr|afile123'? +'?(tmp_check/)?afile456|,
+ qr|afile123'? +'?afile456|,
"offer multiple file choices");
clear_line();
diff --git a/src/test/perl/PostgreSQL/Test/Utils.pm b/src/test/perl/PostgreSQL/Test/Utils.pm
index 1ca2cc5917..6cd3ff2caf 100644
--- a/src/test/perl/PostgreSQL/Test/Utils.pm
+++ b/src/test/perl/PostgreSQL/Test/Utils.pm
@@ -190,9 +190,9 @@ INIT
$SIG{PIPE} = 'IGNORE';
# Determine output directories, and create them. The base path is the
- # TESTDIR environment variable, which is normally set by the invoking
+ # TESTOUTDUR environment variable, which is normally set by the invoking
# Makefile.
- $tmp_check = $ENV{TESTDIR} ? "$ENV{TESTDIR}/tmp_check" : "tmp_check";
+ $tmp_check = $ENV{TESTOUTDIR} ? $ENV{TESTOUTDIR} : "tmp_check";
$log_path = "$tmp_check/log";
mkdir $tmp_check;
diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index 051718e4fe..4a4e1bd4c9 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -452,7 +452,8 @@ echo "+++ tap install-check in $(subdir) +++" && \
rm -rf '$(CURDIR)'/tmp_check && \
$(MKDIR_P) '$(CURDIR)'/tmp_check && \
cd $(srcdir) && \
- TESTDIR='$(CURDIR)' PATH="$(bindir):$(CURDIR):$$PATH" \
+ TESTOUTDIR='$(CURDIR)/tmp_check' TESTDIR='$(CURDIR)' \
+ PATH="$(bindir):$(CURDIR):$$PATH" \
PGPORT='6$(DEF_PGPORT)' top_builddir='$(CURDIR)/$(top_builddir)' \
PG_REGRESS='$(CURDIR)/$(top_builddir)/src/test/regress/pg_regress' \
$(PROVE) $(PG_PROVE_FLAGS) $(PROVE_FLAGS) $(if $(PROVE_TESTS),$(PROVE_TESTS),t/*.pl)
@@ -463,8 +464,9 @@ echo "+++ tap install-check in $(subdir) +++" && \
rm -rf '$(CURDIR)'/tmp_check && \
$(MKDIR_P) '$(CURDIR)'/tmp_check && \
cd $(srcdir) && \
- TESTDIR='$(CURDIR)' PATH="$(bindir):$(CURDIR):$$PATH" \
- PGPORT='6$(DEF_PGPORT)' top_builddir='$(top_builddir)' \
+ TESTOUTDIR='$(CURDIR)/tmp_check' TESTDIR='$(CURDIR)' \
+ PATH="$(bindir):$(CURDIR):$$PATH" PGPORT='6$(DEF_PGPORT)' \
+ top_builddir='$(top_builddir)' \
PG_REGRESS='$(top_builddir)/src/test/regress/pg_regress' \
$(PROVE) $(PG_PROVE_FLAGS) $(PROVE_FLAGS) $(if $(PROVE_TESTS),$(PROVE_TESTS),t/*.pl)
endef
@@ -475,7 +477,8 @@ echo "+++ tap check in $(subdir) +++" && \
rm -rf '$(CURDIR)'/tmp_check && \
$(MKDIR_P) '$(CURDIR)'/tmp_check && \
cd $(srcdir) && \
- TESTDIR='$(CURDIR)' $(with_temp_install) PGPORT='6$(DEF_PGPORT)' \
+ TESTOUTDIR='$(CURDIR)/tmp_check' TESTDIR='$(CURDIR)' \
+ $(with_temp_install) PGPORT='6$(DEF_PGPORT)' \
PG_REGRESS='$(CURDIR)/$(top_builddir)/src/test/regress/pg_regress' \
$(PROVE) $(PG_PROVE_FLAGS) $(PROVE_FLAGS) $(if $(PROVE_TESTS),$(PROVE_TESTS),t/*.pl)
endef
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index c3729f6be5..6f07a31464 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -296,6 +296,8 @@ sub tap_check
# add the module build dir as the second element in the PATH
$ENV{PATH} =~ s!;!;$topdir/$Config/$module;!;
+ $ENV{TESTOUTDIR} = "$dir/tmp_check";
+
rmtree('tmp_check');
system(@args);
my $status = $? >> 8;
--
2.36.1
From 0de7e32dbb79fda17384b480452f9b26b22e0ccb Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Tue, 31 May 2022 16:01:46 +0900
Subject: [PATCH v3 2/2] Run pg_upgrade test in test output directory, as of
TESTOUTDIR
In passing, add names to tests that didn't have any yet.
---
src/bin/pg_upgrade/t/002_pg_upgrade.pl | 24 +++++++++++++++++++-----
1 file changed, 19 insertions(+), 5 deletions(-)
diff --git a/src/bin/pg_upgrade/t/002_pg_upgrade.pl b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
index 75ac768a96..cb685d5d73 100644
--- a/src/bin/pg_upgrade/t/002_pg_upgrade.pl
+++ b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
@@ -2,7 +2,7 @@
use strict;
use warnings;
-use Cwd qw(abs_path getcwd);
+use Cwd qw(abs_path);
use File::Basename qw(dirname);
use File::Compare;
@@ -23,7 +23,9 @@ sub generate_db
}
$dbname .= $suffix;
- $node->command_ok([ 'createdb', $dbname ]);
+ $node->command_ok(
+ [ 'createdb', $dbname ],
+ "created database with ASCII characters from $from_char to $to_char");
}
# The test of pg_upgrade requires two clusters, an old one and a new one
@@ -71,7 +73,8 @@ if (defined($ENV{olddump}))
# Load the dump using the "postgres" database as "regression" does
# not exist yet, and we are done here.
- $oldnode->command_ok([ 'psql', '-X', '-f', $olddumpfile, 'postgres' ]);
+ $oldnode->command_ok([ 'psql', '-X', '-f', $olddumpfile, 'postgres' ],
+ 'loaded old dump file');
}
else
{
@@ -136,7 +139,8 @@ if (defined($ENV{oldinstall}))
'psql', '-X',
'-f', "$srcdir/src/bin/pg_upgrade/upgrade_adapt.sql",
'regression'
- ]);
+ ],
+ 'ran adapt script');
}
# Initialize a new node for the upgrade.
@@ -202,6 +206,15 @@ if (defined($ENV{oldinstall}))
}
}
+# In a VPATH build, we'll be started in the source directory, but we want
+# to run pg_upgrade in the test output directory so that any files generated
+# finish in it, like delete_old_cluster.sh.
+if ($ENV{TESTOUTDIR})
+{
+ chdir $ENV{TESTOUTDIR}
+ or die "could not chdir to \"$ENV{TESTOUTDIR}\": $!";
+}
+
# Upgrade the instance.
$oldnode->stop;
command_ok(
@@ -233,7 +246,8 @@ $newnode->command_ok(
'pg_dumpall', '--no-sync',
'-d', $newnode->connstr('postgres'),
'-f', "$tempdir/dump2.sql"
- ]);
+ ],
+ 'dump before running pg_upgrade');
# Compare the two dumps, there should be no differences.
my $compare_res = compare("$tempdir/dump1.sql", "$tempdir/dump2.sql");
--
2.36.1
signature.asc
Description: PGP signature
