convert libpq uri-regress tests to tap test
Hi,
When verifying that the meson port actually runs all perl based tests I came
across src/interfaces/libpq/test/regress.pl. Instead of running tests yet
another way, it seems better to convert it to a tap test.
I hope others agree?
Where would we want that test to live? Right now we have the slightly odd
convention that some tap tests live in src/test/{misc,modules,...}. But
e.g. frontend binary ones are below src/bin/.
For now I've left it in src/interfaces/libpq/test, with the test in
t/001_uri.pl. But we should at least get rid of the test/...
Perhaps we should just rename src/test/modules/libpq_pipeline to
src/test/modules/libpq and move uri-regress in there as well?
The tap test needs a bit more polish, mostly posted this to get some feedback
before wasting more time :)
Greetings,
Andres Freund
Attachments:
v1-0001-Convert-src-interfaces-libpq-test-to-a-tap-test.patchtext/x-diff; charset=us-asciiDownload
From 77b3666c9a7f17f98120cfc9c2a8e99f7d4ab491 Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Wed, 23 Feb 2022 12:22:56 -0800
Subject: [PATCH v1] Convert src/interfaces/libpq/test to a tap test.
---
src/interfaces/libpq/Makefile | 2 +-
src/interfaces/libpq/test/Makefile | 10 +-
src/interfaces/libpq/test/README | 7 -
src/interfaces/libpq/test/expected.out | 171 ----------------
src/interfaces/libpq/test/regress.in | 57 ------
src/interfaces/libpq/test/regress.pl | 65 ------
src/interfaces/libpq/test/t/001_uri.pl | 266 +++++++++++++++++++++++++
7 files changed, 274 insertions(+), 304 deletions(-)
delete mode 100644 src/interfaces/libpq/test/README
delete mode 100644 src/interfaces/libpq/test/expected.out
delete mode 100644 src/interfaces/libpq/test/regress.in
delete mode 100644 src/interfaces/libpq/test/regress.pl
create mode 100644 src/interfaces/libpq/test/t/001_uri.pl
diff --git a/src/interfaces/libpq/Makefile b/src/interfaces/libpq/Makefile
index 844c95d47de..8920f7e6365 100644
--- a/src/interfaces/libpq/Makefile
+++ b/src/interfaces/libpq/Makefile
@@ -137,7 +137,7 @@ install: all installdirs install-lib
$(INSTALL_DATA) $(srcdir)/pqexpbuffer.h '$(DESTDIR)$(includedir_internal)'
$(INSTALL_DATA) $(srcdir)/pg_service.conf.sample '$(DESTDIR)$(datadir)/pg_service.conf.sample'
-installcheck:
+installcheck check:
$(MAKE) -C test $@
installdirs: installdirs-lib
diff --git a/src/interfaces/libpq/test/Makefile b/src/interfaces/libpq/test/Makefile
index 4832fab9d23..5dbfe5d09ae 100644
--- a/src/interfaces/libpq/test/Makefile
+++ b/src/interfaces/libpq/test/Makefile
@@ -1,3 +1,5 @@
+# src/interfaces/libpq/test/Makefile
+
subdir = src/interfaces/libpq/test
top_builddir = ../../../..
include $(top_builddir)/src/Makefile.global
@@ -13,10 +15,12 @@ PROGS = uri-regress
all: $(PROGS)
+check: all
+ $(prove_check)
+
installcheck: all
- SRCDIR='$(top_srcdir)' SUBDIR='$(subdir)' \
- $(PERL) $(top_srcdir)/$(subdir)/regress.pl
+ $(prove_installcheck)
clean distclean maintainer-clean:
rm -f $(PROGS) *.o
- rm -f regress.out regress.diff
+ rm -rf tmp_check
diff --git a/src/interfaces/libpq/test/README b/src/interfaces/libpq/test/README
deleted file mode 100644
index a05eb6bb3bc..00000000000
--- a/src/interfaces/libpq/test/README
+++ /dev/null
@@ -1,7 +0,0 @@
-This is a testsuite for testing libpq URI connection string syntax.
-
-To run the suite, use 'make installcheck' command. It works by
-running 'regress.pl' from this directory with appropriate environment
-set up, which in turn feeds up lines from 'regress.in' to
-'uri-regress' test program and compares the output against the correct
-one in 'expected.out' file.
diff --git a/src/interfaces/libpq/test/expected.out b/src/interfaces/libpq/test/expected.out
deleted file mode 100644
index d375e82b4ac..00000000000
--- a/src/interfaces/libpq/test/expected.out
+++ /dev/null
@@ -1,171 +0,0 @@
-trying postgresql://uri-user:secret@host:12345/db
-user='uri-user' password='secret' dbname='db' host='host' port='12345' (inet)
-
-trying postgresql://uri-user@host:12345/db
-user='uri-user' dbname='db' host='host' port='12345' (inet)
-
-trying postgresql://uri-user@host/db
-user='uri-user' dbname='db' host='host' (inet)
-
-trying postgresql://host:12345/db
-dbname='db' host='host' port='12345' (inet)
-
-trying postgresql://host/db
-dbname='db' host='host' (inet)
-
-trying postgresql://uri-user@host:12345/
-user='uri-user' host='host' port='12345' (inet)
-
-trying postgresql://uri-user@host/
-user='uri-user' host='host' (inet)
-
-trying postgresql://uri-user@
-user='uri-user' (local)
-
-trying postgresql://host:12345/
-host='host' port='12345' (inet)
-
-trying postgresql://host:12345
-host='host' port='12345' (inet)
-
-trying postgresql://host/db
-dbname='db' host='host' (inet)
-
-trying postgresql://host/
-host='host' (inet)
-
-trying postgresql://host
-host='host' (inet)
-
-trying postgresql://
-(local)
-
-trying postgresql://?hostaddr=127.0.0.1
-hostaddr='127.0.0.1' (inet)
-
-trying postgresql://example.com?hostaddr=63.1.2.4
-host='example.com' hostaddr='63.1.2.4' (inet)
-
-trying postgresql://%68ost/
-host='host' (inet)
-
-trying postgresql://host/db?user=uri-user
-user='uri-user' dbname='db' host='host' (inet)
-
-trying postgresql://host/db?user=uri-user&port=12345
-user='uri-user' dbname='db' host='host' port='12345' (inet)
-
-trying postgresql://host/db?u%73er=someotheruser&port=12345
-user='someotheruser' dbname='db' host='host' port='12345' (inet)
-
-trying postgresql://host/db?u%7aer=someotheruser&port=12345
-uri-regress: invalid URI query parameter: "uzer"
-
-trying postgresql://host:12345?user=uri-user
-user='uri-user' host='host' port='12345' (inet)
-
-trying postgresql://host?user=uri-user
-user='uri-user' host='host' (inet)
-
-trying postgresql://host?
-host='host' (inet)
-
-trying postgresql://[::1]:12345/db
-dbname='db' host='::1' port='12345' (inet)
-
-trying postgresql://[::1]/db
-dbname='db' host='::1' (inet)
-
-trying postgresql://[2001:db8::1234]/
-host='2001:db8::1234' (inet)
-
-trying postgresql://[200z:db8::1234]/
-host='200z:db8::1234' (inet)
-
-trying postgresql://[::1]
-host='::1' (inet)
-
-trying postgres://
-(local)
-
-trying postgres:///
-(local)
-
-trying postgres:///db
-dbname='db' (local)
-
-trying postgres://uri-user@/db
-user='uri-user' dbname='db' (local)
-
-trying postgres://?host=/path/to/socket/dir
-host='/path/to/socket/dir' (local)
-
-trying postgresql://host?uzer=
-uri-regress: invalid URI query parameter: "uzer"
-
-trying postgre://
-uri-regress: missing "=" after "postgre://" in connection info string
-
-trying postgres://[::1
-uri-regress: end of string reached when looking for matching "]" in IPv6 host address in URI: "postgres://[::1"
-
-trying postgres://[]
-uri-regress: IPv6 host address may not be empty in URI: "postgres://[]"
-
-trying postgres://[::1]z
-uri-regress: unexpected character "z" at position 17 in URI (expected ":" or "/"): "postgres://[::1]z"
-
-trying postgresql://host?zzz
-uri-regress: missing key/value separator "=" in URI query parameter: "zzz"
-
-trying postgresql://host?value1&value2
-uri-regress: missing key/value separator "=" in URI query parameter: "value1"
-
-trying postgresql://host?key=key=value
-uri-regress: extra key/value separator "=" in URI query parameter: "key"
-
-trying postgres://host?dbname=%XXfoo
-uri-regress: invalid percent-encoded token: "%XXfoo"
-
-trying postgresql://a%00b
-uri-regress: forbidden value %00 in percent-encoded value: "a%00b"
-
-trying postgresql://%zz
-uri-regress: invalid percent-encoded token: "%zz"
-
-trying postgresql://%1
-uri-regress: invalid percent-encoded token: "%1"
-
-trying postgresql://%
-uri-regress: invalid percent-encoded token: "%"
-
-trying postgres://@host
-host='host' (inet)
-
-trying postgres://host:/
-host='host' (inet)
-
-trying postgres://:12345/
-port='12345' (local)
-
-trying postgres://otheruser@?host=/no/such/directory
-user='otheruser' host='/no/such/directory' (local)
-
-trying postgres://otheruser@/?host=/no/such/directory
-user='otheruser' host='/no/such/directory' (local)
-
-trying postgres://otheruser@:12345?host=/no/such/socket/path
-user='otheruser' host='/no/such/socket/path' port='12345' (local)
-
-trying postgres://otheruser@:12345/db?host=/path/to/socket
-user='otheruser' dbname='db' host='/path/to/socket' port='12345' (local)
-
-trying postgres://:12345/db?host=/path/to/socket
-dbname='db' host='/path/to/socket' port='12345' (local)
-
-trying postgres://:12345?host=/path/to/socket
-host='/path/to/socket' port='12345' (local)
-
-trying postgres://%2Fvar%2Flib%2Fpostgresql/dbname
-dbname='dbname' host='/var/lib/postgresql' (local)
-
diff --git a/src/interfaces/libpq/test/regress.in b/src/interfaces/libpq/test/regress.in
deleted file mode 100644
index de034f39141..00000000000
--- a/src/interfaces/libpq/test/regress.in
+++ /dev/null
@@ -1,57 +0,0 @@
-postgresql://uri-user:secret@host:12345/db
-postgresql://uri-user@host:12345/db
-postgresql://uri-user@host/db
-postgresql://host:12345/db
-postgresql://host/db
-postgresql://uri-user@host:12345/
-postgresql://uri-user@host/
-postgresql://uri-user@
-postgresql://host:12345/
-postgresql://host:12345
-postgresql://host/db
-postgresql://host/
-postgresql://host
-postgresql://
-postgresql://?hostaddr=127.0.0.1
-postgresql://example.com?hostaddr=63.1.2.4
-postgresql://%68ost/
-postgresql://host/db?user=uri-user
-postgresql://host/db?user=uri-user&port=12345
-postgresql://host/db?u%73er=someotheruser&port=12345
-postgresql://host/db?u%7aer=someotheruser&port=12345
-postgresql://host:12345?user=uri-user
-postgresql://host?user=uri-user
-postgresql://host?
-postgresql://[::1]:12345/db
-postgresql://[::1]/db
-postgresql://[2001:db8::1234]/
-postgresql://[200z:db8::1234]/
-postgresql://[::1]
-postgres://
-postgres:///
-postgres:///db
-postgres://uri-user@/db
-postgres://?host=/path/to/socket/dir
-postgresql://host?uzer=
-postgre://
-postgres://[::1
-postgres://[]
-postgres://[::1]z
-postgresql://host?zzz
-postgresql://host?value1&value2
-postgresql://host?key=key=value
-postgres://host?dbname=%XXfoo
-postgresql://a%00b
-postgresql://%zz
-postgresql://%1
-postgresql://%
-postgres://@host
-postgres://host:/
-postgres://:12345/
-postgres://otheruser@?host=/no/such/directory
-postgres://otheruser@/?host=/no/such/directory
-postgres://otheruser@:12345?host=/no/such/socket/path
-postgres://otheruser@:12345/db?host=/path/to/socket
-postgres://:12345/db?host=/path/to/socket
-postgres://:12345?host=/path/to/socket
-postgres://%2Fvar%2Flib%2Fpostgresql/dbname
diff --git a/src/interfaces/libpq/test/regress.pl b/src/interfaces/libpq/test/regress.pl
deleted file mode 100644
index 70691dabe68..00000000000
--- a/src/interfaces/libpq/test/regress.pl
+++ /dev/null
@@ -1,65 +0,0 @@
-#!/usr/bin/perl
-
-# Copyright (c) 2021-2022, PostgreSQL Global Development Group
-
-use strict;
-use warnings;
-
-# use of SRCDIR/SUBDIR is required for supporting VPath builds
-my $srcdir = $ENV{'SRCDIR'} or die 'SRCDIR environment variable is not set';
-my $subdir = $ENV{'SUBDIR'} or die 'SUBDIR environment variable is not set';
-
-my $regress_in = "$srcdir/$subdir/regress.in";
-my $expected_out = "$srcdir/$subdir/expected.out";
-
-# the output file should land in the build_dir of VPath, or just in
-# the current dir, if VPath isn't used
-my $regress_out = "regress.out";
-
-# open input file first, so possible error isn't sent to redirected STDERR
-open(my $regress_in_fh, "<", $regress_in)
- or die "can't open $regress_in for reading: $!";
-
-# save STDOUT/ERR and redirect both to regress.out
-open(my $oldout_fh, ">&", \*STDOUT) or die "can't dup STDOUT: $!";
-open(my $olderr_fh, ">&", \*STDERR) or die "can't dup STDERR: $!";
-
-open(STDOUT, ">", $regress_out)
- or die "can't open $regress_out for writing: $!";
-open(STDERR, ">&", \*STDOUT) or die "can't dup STDOUT: $!";
-
-# read lines from regress.in and run uri-regress on them
-while (<$regress_in_fh>)
-{
- chomp;
- print "trying $_\n";
- system("./uri-regress \"$_\"");
- print "\n";
-}
-
-# restore STDOUT/ERR so we can print the outcome to the user
-open(STDERR, ">&", $olderr_fh)
- or die; # can't complain as STDERR is still duped
-open(STDOUT, ">&", $oldout_fh) or die "can't restore STDOUT: $!";
-
-# just in case
-close $regress_in_fh;
-
-my $diff_status = system(
- "diff -c \"$srcdir/$subdir/expected.out\" regress.out >regress.diff");
-
-print "=" x 70, "\n";
-if ($diff_status == 0)
-{
- print "All tests passed\n";
- exit 0;
-}
-else
-{
- print <<EOF;
-FAILED: the test result differs from the expected output
-
-Review the difference in "$subdir/regress.diff"
-EOF
- exit 1;
-}
diff --git a/src/interfaces/libpq/test/t/001_uri.pl b/src/interfaces/libpq/test/t/001_uri.pl
new file mode 100644
index 00000000000..a75cc06444d
--- /dev/null
+++ b/src/interfaces/libpq/test/t/001_uri.pl
@@ -0,0 +1,266 @@
+# Copyright (c) 2021-2022, PostgreSQL Global Development Group
+use strict;
+use warnings;
+
+use PostgreSQL::Test::Utils;
+use Test::More;
+use IPC::Run;
+
+my @tests = (
+ [q{postgresql://uri-user:secret@host:12345/db},
+ q{user='uri-user' password='secret' dbname='db' host='host' port='12345' (inet)},
+ q{},
+ ],
+ [q{postgresql://uri-user@host:12345/db},
+ q{user='uri-user' dbname='db' host='host' port='12345' (inet)},
+ q{},
+ ],
+ [q{postgresql://uri-user@host/db},
+ q{user='uri-user' dbname='db' host='host' (inet)},
+ q{},
+ ],
+ [q{postgresql://host:12345/db},
+ q{dbname='db' host='host' port='12345' (inet)},
+ q{},
+ ],
+ [q{postgresql://host/db},
+ q{dbname='db' host='host' (inet)},
+ q{},
+ ],
+ [q{postgresql://uri-user@host:12345/},
+ q{user='uri-user' host='host' port='12345' (inet)},
+ q{},
+ ],
+ [q{postgresql://uri-user@host/},
+ q{user='uri-user' host='host' (inet)},
+ q{},
+ ],
+ [q{postgresql://uri-user@},
+ q{user='uri-user' (local)},
+ q{},
+ ],
+ [q{postgresql://host:12345/},
+ q{host='host' port='12345' (inet)},
+ q{},
+ ],
+ [q{postgresql://host:12345},
+ q{host='host' port='12345' (inet)},
+ q{},
+ ],
+ [q{postgresql://host/db},
+ q{dbname='db' host='host' (inet)},
+ q{},
+ ],
+ [q{postgresql://host/},
+ q{host='host' (inet)},
+ q{},
+ ],
+ [q{postgresql://host},
+ q{host='host' (inet)},
+ q{},
+ ],
+ [q{postgresql://},
+ q{(local)},
+ q{},
+ ],
+ [q{postgresql://?hostaddr=127.0.0.1},
+ q{hostaddr='127.0.0.1' (inet)},
+ q{},
+ ],
+ [q{postgresql://example.com?hostaddr=63.1.2.4},
+ q{host='example.com' hostaddr='63.1.2.4' (inet)},
+ q{},
+ ],
+ [q{postgresql://%68ost/},
+ q{host='host' (inet)},
+ q{},
+ ],
+ [q{postgresql://host/db?user=uri-user},
+ q{user='uri-user' dbname='db' host='host' (inet)},
+ q{},
+ ],
+ [q{postgresql://host/db?user=uri-user&port=12345},
+ q{user='uri-user' dbname='db' host='host' port='12345' (inet)},
+ q{},
+ ],
+ [q{postgresql://host/db?u%73er=someotheruser&port=12345},
+ q{user='someotheruser' dbname='db' host='host' port='12345' (inet)},
+ q{},
+ ],
+ [q{postgresql://host/db?u%7aer=someotheruser&port=12345},
+ q{},
+ q{uri-regress: invalid URI query parameter: "uzer"},
+ ],
+ [q{postgresql://host:12345?user=uri-user},
+ q{user='uri-user' host='host' port='12345' (inet)},
+ q{},
+ ],
+ [q{postgresql://host?user=uri-user},
+ q{user='uri-user' host='host' (inet)},
+ q{},
+ ],
+ [q{postgresql://host?},
+ q{host='host' (inet)},
+ q{},
+ ],
+ [q{postgresql://[::1]:12345/db},
+ q{dbname='db' host='::1' port='12345' (inet)},
+ q{},
+ ],
+ [q{postgresql://[::1]/db},
+ q{dbname='db' host='::1' (inet)},
+ q{},
+ ],
+ [q{postgresql://[2001:db8::1234]/},
+ q{host='2001:db8::1234' (inet)},
+ q{},
+ ],
+ [q{postgresql://[200z:db8::1234]/},
+ q{host='200z:db8::1234' (inet)},
+ q{},
+ ],
+ [q{postgresql://[::1]},
+ q{host='::1' (inet)},
+ q{},
+ ],
+ [q{postgres://},
+ q{(local)},
+ q{},
+ ],
+ [q{postgres:///},
+ q{(local)},
+ q{},
+ ],
+ [q{postgres:///db},
+ q{dbname='db' (local)},
+ q{},
+ ],
+ [q{postgres://uri-user@/db},
+ q{user='uri-user' dbname='db' (local)},
+ q{},
+ ],
+ [q{postgres://?host=/path/to/socket/dir},
+ q{host='/path/to/socket/dir' (local)},
+ q{},
+ ],
+ [q{postgresql://host?uzer=},
+ q{},
+ q{uri-regress: invalid URI query parameter: "uzer"},
+ ],
+ [q{postgre://},
+ q{},
+ q{uri-regress: missing "=" after "postgre://" in connection info string},
+ ],
+ [q{postgres://[::1},
+ q{},
+ q{uri-regress: end of string reached when looking for matching "]" in IPv6 host address in URI: "postgres://[::1"},
+ ],
+ [q{postgres://[]},
+ q{},
+ q{uri-regress: IPv6 host address may not be empty in URI: "postgres://[]"},
+ ],
+ [q{postgres://[::1]z},
+ q{},
+ q{uri-regress: unexpected character "z" at position 17 in URI (expected ":" or "/"): "postgres://[::1]z"},
+ ],
+ [q{postgresql://host?zzz},
+ q{},
+ q{uri-regress: missing key/value separator "=" in URI query parameter: "zzz"},
+ ],
+ [q{postgresql://host?value1&value2},
+ q{},
+ q{uri-regress: missing key/value separator "=" in URI query parameter: "value1"},
+ ],
+ [q{postgresql://host?key=key=value},
+ q{},
+ q{uri-regress: extra key/value separator "=" in URI query parameter: "key"},
+ ],
+ [q{postgres://host?dbname=%XXfoo},
+ q{},
+ q{uri-regress: invalid percent-encoded token: "%XXfoo"},
+ ],
+ [q{postgresql://a%00b},
+ q{},
+ q{uri-regress: forbidden value %00 in percent-encoded value: "a%00b"},
+ ],
+ [q{postgresql://%zz},
+ q{},
+ q{uri-regress: invalid percent-encoded token: "%zz"},
+ ],
+ [q{postgresql://%1},
+ q{},
+ q{uri-regress: invalid percent-encoded token: "%1"},
+ ],
+ [q{postgresql://%},
+ q{},
+ q{uri-regress: invalid percent-encoded token: "%"},
+ ],
+ [q{postgres://@host},
+ q{host='host' (inet)},
+ q{},
+ ],
+ [q{postgres://host:/},
+ q{host='host' (inet)},
+ q{},
+ ],
+ [q{postgres://:12345/},
+ q{port='12345' (local)},
+ q{},
+ ],
+ [q{postgres://otheruser@?host=/no/such/directory},
+ q{user='otheruser' host='/no/such/directory' (local)},
+ q{},
+ ],
+ [q{postgres://otheruser@/?host=/no/such/directory},
+ q{user='otheruser' host='/no/such/directory' (local)},
+ q{},
+ ],
+ [q{postgres://otheruser@:12345?host=/no/such/socket/path},
+ q{user='otheruser' host='/no/such/socket/path' port='12345' (local)},
+ q{},
+ ],
+ [q{postgres://otheruser@:12345/db?host=/path/to/socket},
+ q{user='otheruser' dbname='db' host='/path/to/socket' port='12345' (local)},
+ q{},
+ ],
+ [q{postgres://:12345/db?host=/path/to/socket},
+ q{dbname='db' host='/path/to/socket' port='12345' (local)},
+ q{},
+ ],
+ [q{postgres://:12345?host=/path/to/socket},
+ q{host='/path/to/socket' port='12345' (local)},
+ q{},
+ ],
+ [q{postgres://%2Fvar%2Flib%2Fpostgresql/dbname},
+ q{dbname='dbname' host='/var/lib/postgresql' (local)},
+ q{},
+ ]
+ );
+
+sub test_uri
+{
+ local $Test::Builder::Level = $Test::Builder::Level + 1;
+ my ($uri, $expect_stdout, $expect_stderr) = @$_;
+
+ my $stdout;
+ my $stderr;
+ my $result;
+
+ $result = IPC::Run::run ['uri-regress', $uri], '>', \$stdout, '2>', \$stderr;
+ chomp($stdout);
+ chomp($stderr);
+ is_deeply([$result, $stdout, $stderr],
+ [$expect_stderr eq '', $expect_stdout, $expect_stderr],
+ $uri);
+}
+
+foreach (@tests)
+{
+ my $in =$_;
+ my $uri = @$in[0];
+
+ test_uri $in;
+ #subtest $uri => \&test_uri, $in;
+}
+
+done_testing();
--
2.34.0
On 2/23/22 15:30, Andres Freund wrote:
Hi,
When verifying that the meson port actually runs all perl based tests I came
across src/interfaces/libpq/test/regress.pl. Instead of running tests yet
another way, it seems better to convert it to a tap test.I hope others agree?
yes
Where would we want that test to live? Right now we have the slightly odd
convention that some tap tests live in src/test/{misc,modules,...}. But
e.g. frontend binary ones are below src/bin/.For now I've left it in src/interfaces/libpq/test, with the test in
t/001_uri.pl. But we should at least get rid of the test/...Perhaps we should just rename src/test/modules/libpq_pipeline to
src/test/modules/libpq and move uri-regress in there as well?
WFM
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
On 23 Feb 2022, at 21:30, Andres Freund <andres@anarazel.de> wrote:
When verifying that the meson port actually runs all perl based tests I came
across src/interfaces/libpq/test/regress.pl. Instead of running tests yet
another way, it seems better to convert it to a tap test.I hope others agree?
Many +1's on that.
The tap test needs a bit more polish, mostly posted this to get some feedback
before wasting more time :)
Skimming the new test it seems structurally fine to me.
--
Daniel Gustafsson https://vmware.com/
Andrew Dunstan <andrew@dunslane.net> writes:
On 2/23/22 15:30, Andres Freund wrote:
Perhaps we should just rename src/test/modules/libpq_pipeline to
src/test/modules/libpq and move uri-regress in there as well?
WFM
+1
regards, tom lane
Hi,
On 2022-02-23 15:57:08 -0500, Tom Lane wrote:
Andrew Dunstan <andrew@dunslane.net> writes:
On 2/23/22 15:30, Andres Freund wrote:
Perhaps we should just rename src/test/modules/libpq_pipeline to
src/test/modules/libpq and move uri-regress in there as well?WFM
+1
Cool.
One question on making that happen: Right now the makefiles building C stuff
in src/test/modules all have the contrib-style stanza to build via pgxs. But
afaics pgxs doesn't support building multiple programs. Which
src/test/modules/libpq would need to.
Aaics there's currently no way to have Mkvcbuild.pm build multiple programs in
one dir via its makefile parsing stuff? Andrew, any suggestions for not
needing to spell this out in both the makefile and Mkvcbuild.pm?
Separately: I don't really understand why we do the whole if USE_PGXS dance in
src/test/modules?
- Andres
On 2022-Feb-23, Andres Freund wrote:
When verifying that the meson port actually runs all perl based tests I came
across src/interfaces/libpq/test/regress.pl. Instead of running tests yet
another way, it seems better to convert it to a tap test.I hope others agree?
WFM.
Perhaps we should just rename src/test/modules/libpq_pipeline to
src/test/modules/libpq and move uri-regress in there as well?
Well, building multiple binaries would require some trickery that might
be excessive for this little tool. But +1 to that on principle.
--
Álvaro Herrera Valdivia, Chile — https://www.EnterpriseDB.com/
"La virtud es el justo medio entre dos defectos" (Aristóteles)
On 2022-Feb-23, Andres Freund wrote:
Separately: I don't really understand why we do the whole if USE_PGXS dance in
src/test/modules?
Yeah, it seems a bit silly. I'm not opposed to making these makefiles
unconditionally do the PGXS thing -- or the non-PGXS thing, if that
makes it easier to build multiple binaries.
--
Álvaro Herrera Valdivia, Chile — https://www.EnterpriseDB.com/
"La grandeza es una experiencia transitoria. Nunca es consistente.
Depende en gran parte de la imaginación humana creadora de mitos"
(Irulan)
On 2/23/22 16:16, Andres Freund wrote:
Hi,
On 2022-02-23 15:57:08 -0500, Tom Lane wrote:
Andrew Dunstan <andrew@dunslane.net> writes:
On 2/23/22 15:30, Andres Freund wrote:
Perhaps we should just rename src/test/modules/libpq_pipeline to
src/test/modules/libpq and move uri-regress in there as well?WFM
+1
Cool.
One question on making that happen: Right now the makefiles building C stuff
in src/test/modules all have the contrib-style stanza to build via pgxs. But
afaics pgxs doesn't support building multiple programs. Which
src/test/modules/libpq would need to.
That's my understanding also.
Aaics there's currently no way to have Mkvcbuild.pm build multiple programs in
one dir via its makefile parsing stuff? Andrew, any suggestions for not
needing to spell this out in both the makefile and Mkvcbuild.pm?
Well, it should be a SMOC. If you can solve the first problem I'm sure I
can come up with a solution for Mkvcbuild.pm. But until we see the shape
of the pgxs changes, planning for Mkcvbuild.pm changes seems remature.
Separately: I don't really understand why we do the whole if USE_PGXS dance in
src/test/modules?
ENOCLUE
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
On 2022-Feb-23, Andres Freund wrote:
Separately: I don't really understand why we do the whole if USE_PGXS dance in
src/test/modules?
Yeah, it seems a bit silly. I'm not opposed to making these makefiles
unconditionally do the PGXS thing -- or the non-PGXS thing, if that
makes it easier to build multiple binaries.
Agreed, we could easily lose that. I think we only do it in contrib
to serve as an example of how to use PGXS ... although considering
we're not *testing* that build mode, one wonders how much that proves.
In any case, src/test/modules doesn't have to do it.
regards, tom lane
On 23.02.22 21:30, Andres Freund wrote:
hen verifying that the meson port actually runs all perl based tests I came
across src/interfaces/libpq/test/regress.pl. Instead of running tests yet
another way, it seems better to convert it to a tap test.I hope others agree?
Where would we want that test to live? Right now we have the slightly odd
convention that some tap tests live in src/test/{misc,modules,...}. But
e.g. frontend binary ones are below src/bin/.
libpq TAP tests should be in src/interfaces/libpq/t/.
I think there were issues that the build farm wouldn't pick up a test
located there, but that should be fixed rather than worked around.
Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
On 23.02.22 21:30, Andres Freund wrote:
Where would we want that test to live? Right now we have the slightly odd
convention that some tap tests live in src/test/{misc,modules,...}. But
e.g. frontend binary ones are below src/bin/.
libpq TAP tests should be in src/interfaces/libpq/t/.
I think there were issues that the build farm wouldn't pick up a test
located there, but that should be fixed rather than worked around.
That's failing to account for the fact that a libpq test can't
really be a pure-perl TAP test; you need some C code to drive the
library. I don't agree with intermixing such code with libpq
itself, independently of any buildsystem issues (which there
might well be). So I think the design of putting such tests under
src/modules is fine.
regards, tom lane
On 23.02.22 23:58, Tom Lane wrote:
Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
On 23.02.22 21:30, Andres Freund wrote:
Where would we want that test to live? Right now we have the slightly odd
convention that some tap tests live in src/test/{misc,modules,...}. But
e.g. frontend binary ones are below src/bin/.libpq TAP tests should be in src/interfaces/libpq/t/.
I think there were issues that the build farm wouldn't pick up a test
located there, but that should be fixed rather than worked around.That's failing to account for the fact that a libpq test can't
really be a pure-perl TAP test; you need some C code to drive the
library. I don't agree with intermixing such code with libpq
itself, independently of any buildsystem issues (which there
might well be).
Such things could be put under src/interfaces/libpq/test, or some other
subdirectory. We already have src/interfaces/ecpg/test.
So I think the design of putting such tests under
src/modules is fine.
I don't get what the rationale for that would be. libpq tests are not
"modules" of any kind.
If I'm working on libpq, I want to do make && make check inside the
libpq source directory.
Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
On 23.02.22 23:58, Tom Lane wrote:
Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
libpq TAP tests should be in src/interfaces/libpq/t/.
That's failing to account for the fact that a libpq test can't
really be a pure-perl TAP test; you need some C code to drive the
library.
Such things could be put under src/interfaces/libpq/test, or some other
subdirectory. We already have src/interfaces/ecpg/test.
OK, but then the TAP scripts are under src/interfaces/libpq/test/t,
which isn't what you said. I have no great objection to moving
src/test/modules/libpq_pipeline/ to src/interfaces/libpq/test/,
though, as long as the buildfarm will cope.
regards, tom lane
On 2/23/22 20:52, Tom Lane wrote:
Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
On 23.02.22 23:58, Tom Lane wrote:
Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
libpq TAP tests should be in src/interfaces/libpq/t/.
That's failing to account for the fact that a libpq test can't
really be a pure-perl TAP test; you need some C code to drive the
library.Such things could be put under src/interfaces/libpq/test, or some other
subdirectory. We already have src/interfaces/ecpg/test.OK, but then the TAP scripts are under src/interfaces/libpq/test/t,
which isn't what you said. I have no great objection to moving
src/test/modules/libpq_pipeline/ to src/interfaces/libpq/test/,
though, as long as the buildfarm will cope.
It won't without some adjustment.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
On 24.02.22 02:52, Tom Lane wrote:
Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
On 23.02.22 23:58, Tom Lane wrote:
Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
libpq TAP tests should be in src/interfaces/libpq/t/.
That's failing to account for the fact that a libpq test can't
really be a pure-perl TAP test; you need some C code to drive the
library.Such things could be put under src/interfaces/libpq/test, or some other
subdirectory. We already have src/interfaces/ecpg/test.OK, but then the TAP scripts are under src/interfaces/libpq/test/t,
which isn't what you said. I have no great objection to moving
src/test/modules/libpq_pipeline/ to src/interfaces/libpq/test/,
though, as long as the buildfarm will cope.
I think the TAP scripts should be in src/interfaces/libpq/t/, as usual.
The supporting code snippets could live in some other directory under
src/interfaces/libpq/, which might be called "test" or something else,
not that important.
I think we should pick a layout that is proper and future-proof and then
adjust the buildfarm client as necessary. The issue of writing
libpq-specific tests has come up a few times recently; I think it would
be worth finding a proper solution to this that would facilitate that
work in the future.
Hi,
On 2022-02-24 13:31:40 +0100, Peter Eisentraut wrote:
I think the TAP scripts should be in src/interfaces/libpq/t/, as usual. The
supporting code snippets could live in some other directory under
src/interfaces/libpq/, which might be called "test" or something else, not
that important.
Why not in t/? We can't easily build the test programs in libpq/ itself, but
libpq/t should be fairly doable.
Greetings,
Andres Freund
Andres Freund <andres@anarazel.de> writes:
On 2022-02-24 13:31:40 +0100, Peter Eisentraut wrote:
I think the TAP scripts should be in src/interfaces/libpq/t/, as usual. The
supporting code snippets could live in some other directory under
src/interfaces/libpq/, which might be called "test" or something else, not
that important.
Why not in t/? We can't easily build the test programs in libpq/ itself, but
libpq/t should be fairly doable.
I think that having t/ directories contain only Perl test scripts
is a good convention that we should stick to. Peter's proposal
of a separate test/ subdirectory for C test scaffolding is
probably fine.
regards, tom lane
Hi,
On 2022-02-24 10:17:28 -0500, Tom Lane wrote:
Andres Freund <andres@anarazel.de> writes:
On 2022-02-24 13:31:40 +0100, Peter Eisentraut wrote:
I think the TAP scripts should be in src/interfaces/libpq/t/, as usual. The
supporting code snippets could live in some other directory under
src/interfaces/libpq/, which might be called "test" or something else, not
that important.Why not in t/? We can't easily build the test programs in libpq/ itself, but
libpq/t should be fairly doable.I think that having t/ directories contain only Perl test scripts
is a good convention that we should stick to. Peter's proposal
of a separate test/ subdirectory for C test scaffolding is
probably fine.
That exists today and continues to exist in the patch upthread, so it's easy
;). I just need to move the libpq/test/t to libpq/t and adjust the binary
path.
One annoying bit is that our current tap invocation infrastructure for msvc
won't know how to deal with that. We put the build directory containing t/
onto PATH, but that won't work for test/. But we also don't want to install
test binaries. Not sure what the solution for that is.
Even on !windows, we only know how to find "test executables" in tap tests via
PATH. We're in the source dir, so we can't just do test/executable.
I probably just need another coffee, but right now I don't even see how to add
anything to PATH given $(prove_check)'s definition - it ends up with multiple
shells. We can fix that by using && in the definition, which might be a good
thing anyway?
Attached three patches:
0001: Convert src/interfaces/libpq/test to a tap test
0002: Add tap test infrastructure to src/interfaces/libpq
0003: Move libpq_pipeline test into src/interfaces/libpq.
I did 0001 and 0002 in that order because prove errors out with a stacktrace
if no tap tests exist... It might make more sense to commit them together, but
for review it's easier to keep them separate I think.
Andrew, do you have an idea about the feasibility of supporting any of this
with the msvc build?
I'm mildly inclined to only do 0001 and 0002 for now. We'd not loose msvc
coverage, because it already doesn't build the test. Once we've ironed that
stuff out, we could do 0003?
Greetings,
Andres Freund
Attachments:
v2-0001-Convert-src-interfaces-libpq-test-to-a-tap-test.patchtext/x-diff; charset=us-asciiDownload
From 71fa1532a1540e8bbf8bdee3ec0b64e863f212f2 Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Wed, 23 Feb 2022 12:22:56 -0800
Subject: [PATCH v2 1/3] Convert src/interfaces/libpq/test to a tap test.
The invocation of the tap test will be added in the next commit.
---
src/interfaces/libpq/t/001_uri.pl | 265 +++++++++++++++++++++++++
src/interfaces/libpq/test/.gitignore | 2 -
src/interfaces/libpq/test/Makefile | 7 +-
src/interfaces/libpq/test/README | 7 -
src/interfaces/libpq/test/expected.out | 171 ----------------
src/interfaces/libpq/test/regress.in | 57 ------
src/interfaces/libpq/test/regress.pl | 65 ------
7 files changed, 267 insertions(+), 307 deletions(-)
create mode 100644 src/interfaces/libpq/t/001_uri.pl
delete mode 100644 src/interfaces/libpq/test/README
delete mode 100644 src/interfaces/libpq/test/expected.out
delete mode 100644 src/interfaces/libpq/test/regress.in
delete mode 100644 src/interfaces/libpq/test/regress.pl
diff --git a/src/interfaces/libpq/t/001_uri.pl b/src/interfaces/libpq/t/001_uri.pl
new file mode 100644
index 00000000000..0225666d9f6
--- /dev/null
+++ b/src/interfaces/libpq/t/001_uri.pl
@@ -0,0 +1,265 @@
+# Copyright (c) 2021-2022, PostgreSQL Global Development Group
+use strict;
+use warnings;
+
+use PostgreSQL::Test::Utils;
+use Test::More;
+use IPC::Run;
+
+my @tests = (
+ [q{postgresql://uri-user:secret@host:12345/db},
+ q{user='uri-user' password='secret' dbname='db' host='host' port='12345' (inet)},
+ q{},
+ ],
+ [q{postgresql://uri-user@host:12345/db},
+ q{user='uri-user' dbname='db' host='host' port='12345' (inet)},
+ q{},
+ ],
+ [q{postgresql://uri-user@host/db},
+ q{user='uri-user' dbname='db' host='host' (inet)},
+ q{},
+ ],
+ [q{postgresql://host:12345/db},
+ q{dbname='db' host='host' port='12345' (inet)},
+ q{},
+ ],
+ [q{postgresql://host/db},
+ q{dbname='db' host='host' (inet)},
+ q{},
+ ],
+ [q{postgresql://uri-user@host:12345/},
+ q{user='uri-user' host='host' port='12345' (inet)},
+ q{},
+ ],
+ [q{postgresql://uri-user@host/},
+ q{user='uri-user' host='host' (inet)},
+ q{},
+ ],
+ [q{postgresql://uri-user@},
+ q{user='uri-user' (local)},
+ q{},
+ ],
+ [q{postgresql://host:12345/},
+ q{host='host' port='12345' (inet)},
+ q{},
+ ],
+ [q{postgresql://host:12345},
+ q{host='host' port='12345' (inet)},
+ q{},
+ ],
+ [q{postgresql://host/db},
+ q{dbname='db' host='host' (inet)},
+ q{},
+ ],
+ [q{postgresql://host/},
+ q{host='host' (inet)},
+ q{},
+ ],
+ [q{postgresql://host},
+ q{host='host' (inet)},
+ q{},
+ ],
+ [q{postgresql://},
+ q{(local)},
+ q{},
+ ],
+ [q{postgresql://?hostaddr=127.0.0.1},
+ q{hostaddr='127.0.0.1' (inet)},
+ q{},
+ ],
+ [q{postgresql://example.com?hostaddr=63.1.2.4},
+ q{host='example.com' hostaddr='63.1.2.4' (inet)},
+ q{},
+ ],
+ [q{postgresql://%68ost/},
+ q{host='host' (inet)},
+ q{},
+ ],
+ [q{postgresql://host/db?user=uri-user},
+ q{user='uri-user' dbname='db' host='host' (inet)},
+ q{},
+ ],
+ [q{postgresql://host/db?user=uri-user&port=12345},
+ q{user='uri-user' dbname='db' host='host' port='12345' (inet)},
+ q{},
+ ],
+ [q{postgresql://host/db?u%73er=someotheruser&port=12345},
+ q{user='someotheruser' dbname='db' host='host' port='12345' (inet)},
+ q{},
+ ],
+ [q{postgresql://host/db?u%7aer=someotheruser&port=12345},
+ q{},
+ q{uri-regress: invalid URI query parameter: "uzer"},
+ ],
+ [q{postgresql://host:12345?user=uri-user},
+ q{user='uri-user' host='host' port='12345' (inet)},
+ q{},
+ ],
+ [q{postgresql://host?user=uri-user},
+ q{user='uri-user' host='host' (inet)},
+ q{},
+ ],
+ [q{postgresql://host?},
+ q{host='host' (inet)},
+ q{},
+ ],
+ [q{postgresql://[::1]:12345/db},
+ q{dbname='db' host='::1' port='12345' (inet)},
+ q{},
+ ],
+ [q{postgresql://[::1]/db},
+ q{dbname='db' host='::1' (inet)},
+ q{},
+ ],
+ [q{postgresql://[2001:db8::1234]/},
+ q{host='2001:db8::1234' (inet)},
+ q{},
+ ],
+ [q{postgresql://[200z:db8::1234]/},
+ q{host='200z:db8::1234' (inet)},
+ q{},
+ ],
+ [q{postgresql://[::1]},
+ q{host='::1' (inet)},
+ q{},
+ ],
+ [q{postgres://},
+ q{(local)},
+ q{},
+ ],
+ [q{postgres:///},
+ q{(local)},
+ q{},
+ ],
+ [q{postgres:///db},
+ q{dbname='db' (local)},
+ q{},
+ ],
+ [q{postgres://uri-user@/db},
+ q{user='uri-user' dbname='db' (local)},
+ q{},
+ ],
+ [q{postgres://?host=/path/to/socket/dir},
+ q{host='/path/to/socket/dir' (local)},
+ q{},
+ ],
+ [q{postgresql://host?uzer=},
+ q{},
+ q{uri-regress: invalid URI query parameter: "uzer"},
+ ],
+ [q{postgre://},
+ q{},
+ q{uri-regress: missing "=" after "postgre://" in connection info string},
+ ],
+ [q{postgres://[::1},
+ q{},
+ q{uri-regress: end of string reached when looking for matching "]" in IPv6 host address in URI: "postgres://[::1"},
+ ],
+ [q{postgres://[]},
+ q{},
+ q{uri-regress: IPv6 host address may not be empty in URI: "postgres://[]"},
+ ],
+ [q{postgres://[::1]z},
+ q{},
+ q{uri-regress: unexpected character "z" at position 17 in URI (expected ":" or "/"): "postgres://[::1]z"},
+ ],
+ [q{postgresql://host?zzz},
+ q{},
+ q{uri-regress: missing key/value separator "=" in URI query parameter: "zzz"},
+ ],
+ [q{postgresql://host?value1&value2},
+ q{},
+ q{uri-regress: missing key/value separator "=" in URI query parameter: "value1"},
+ ],
+ [q{postgresql://host?key=key=value},
+ q{},
+ q{uri-regress: extra key/value separator "=" in URI query parameter: "key"},
+ ],
+ [q{postgres://host?dbname=%XXfoo},
+ q{},
+ q{uri-regress: invalid percent-encoded token: "%XXfoo"},
+ ],
+ [q{postgresql://a%00b},
+ q{},
+ q{uri-regress: forbidden value %00 in percent-encoded value: "a%00b"},
+ ],
+ [q{postgresql://%zz},
+ q{},
+ q{uri-regress: invalid percent-encoded token: "%zz"},
+ ],
+ [q{postgresql://%1},
+ q{},
+ q{uri-regress: invalid percent-encoded token: "%1"},
+ ],
+ [q{postgresql://%},
+ q{},
+ q{uri-regress: invalid percent-encoded token: "%"},
+ ],
+ [q{postgres://@host},
+ q{host='host' (inet)},
+ q{},
+ ],
+ [q{postgres://host:/},
+ q{host='host' (inet)},
+ q{},
+ ],
+ [q{postgres://:12345/},
+ q{port='12345' (local)},
+ q{},
+ ],
+ [q{postgres://otheruser@?host=/no/such/directory},
+ q{user='otheruser' host='/no/such/directory' (local)},
+ q{},
+ ],
+ [q{postgres://otheruser@/?host=/no/such/directory},
+ q{user='otheruser' host='/no/such/directory' (local)},
+ q{},
+ ],
+ [q{postgres://otheruser@:12345?host=/no/such/socket/path},
+ q{user='otheruser' host='/no/such/socket/path' port='12345' (local)},
+ q{},
+ ],
+ [q{postgres://otheruser@:12345/db?host=/path/to/socket},
+ q{user='otheruser' dbname='db' host='/path/to/socket' port='12345' (local)},
+ q{},
+ ],
+ [q{postgres://:12345/db?host=/path/to/socket},
+ q{dbname='db' host='/path/to/socket' port='12345' (local)},
+ q{},
+ ],
+ [q{postgres://:12345?host=/path/to/socket},
+ q{host='/path/to/socket' port='12345' (local)},
+ q{},
+ ],
+ [q{postgres://%2Fvar%2Flib%2Fpostgresql/dbname},
+ q{dbname='dbname' host='/var/lib/postgresql' (local)},
+ q{},
+ ]
+ );
+
+sub test_uri
+{
+ local $Test::Builder::Level = $Test::Builder::Level + 1;
+ my ($uri, $expect_stdout, $expect_stderr) = @$_;
+
+ my $stdout;
+ my $stderr;
+ my $result;
+
+ $result = IPC::Run::run ['uri-regress', $uri], '>', \$stdout, '2>', \$stderr;
+ chomp($stdout);
+ chomp($stderr);
+ is_deeply([$result, $stdout, $stderr],
+ [$expect_stderr eq '', $expect_stdout, $expect_stderr],
+ $uri);
+}
+
+foreach (@tests)
+{
+ my $in =$_;
+ my $uri = @$in[0];
+
+ test_uri $in;
+}
+
+done_testing();
diff --git a/src/interfaces/libpq/test/.gitignore b/src/interfaces/libpq/test/.gitignore
index 5387b3b6d94..5e803d8816a 100644
--- a/src/interfaces/libpq/test/.gitignore
+++ b/src/interfaces/libpq/test/.gitignore
@@ -1,3 +1 @@
/uri-regress
-/regress.diff
-/regress.out
diff --git a/src/interfaces/libpq/test/Makefile b/src/interfaces/libpq/test/Makefile
index 4832fab9d23..54212159065 100644
--- a/src/interfaces/libpq/test/Makefile
+++ b/src/interfaces/libpq/test/Makefile
@@ -1,3 +1,5 @@
+# src/interfaces/libpq/test/Makefile
+
subdir = src/interfaces/libpq/test
top_builddir = ../../../..
include $(top_builddir)/src/Makefile.global
@@ -13,10 +15,5 @@ PROGS = uri-regress
all: $(PROGS)
-installcheck: all
- SRCDIR='$(top_srcdir)' SUBDIR='$(subdir)' \
- $(PERL) $(top_srcdir)/$(subdir)/regress.pl
-
clean distclean maintainer-clean:
rm -f $(PROGS) *.o
- rm -f regress.out regress.diff
diff --git a/src/interfaces/libpq/test/README b/src/interfaces/libpq/test/README
deleted file mode 100644
index a05eb6bb3bc..00000000000
--- a/src/interfaces/libpq/test/README
+++ /dev/null
@@ -1,7 +0,0 @@
-This is a testsuite for testing libpq URI connection string syntax.
-
-To run the suite, use 'make installcheck' command. It works by
-running 'regress.pl' from this directory with appropriate environment
-set up, which in turn feeds up lines from 'regress.in' to
-'uri-regress' test program and compares the output against the correct
-one in 'expected.out' file.
diff --git a/src/interfaces/libpq/test/expected.out b/src/interfaces/libpq/test/expected.out
deleted file mode 100644
index d375e82b4ac..00000000000
--- a/src/interfaces/libpq/test/expected.out
+++ /dev/null
@@ -1,171 +0,0 @@
-trying postgresql://uri-user:secret@host:12345/db
-user='uri-user' password='secret' dbname='db' host='host' port='12345' (inet)
-
-trying postgresql://uri-user@host:12345/db
-user='uri-user' dbname='db' host='host' port='12345' (inet)
-
-trying postgresql://uri-user@host/db
-user='uri-user' dbname='db' host='host' (inet)
-
-trying postgresql://host:12345/db
-dbname='db' host='host' port='12345' (inet)
-
-trying postgresql://host/db
-dbname='db' host='host' (inet)
-
-trying postgresql://uri-user@host:12345/
-user='uri-user' host='host' port='12345' (inet)
-
-trying postgresql://uri-user@host/
-user='uri-user' host='host' (inet)
-
-trying postgresql://uri-user@
-user='uri-user' (local)
-
-trying postgresql://host:12345/
-host='host' port='12345' (inet)
-
-trying postgresql://host:12345
-host='host' port='12345' (inet)
-
-trying postgresql://host/db
-dbname='db' host='host' (inet)
-
-trying postgresql://host/
-host='host' (inet)
-
-trying postgresql://host
-host='host' (inet)
-
-trying postgresql://
-(local)
-
-trying postgresql://?hostaddr=127.0.0.1
-hostaddr='127.0.0.1' (inet)
-
-trying postgresql://example.com?hostaddr=63.1.2.4
-host='example.com' hostaddr='63.1.2.4' (inet)
-
-trying postgresql://%68ost/
-host='host' (inet)
-
-trying postgresql://host/db?user=uri-user
-user='uri-user' dbname='db' host='host' (inet)
-
-trying postgresql://host/db?user=uri-user&port=12345
-user='uri-user' dbname='db' host='host' port='12345' (inet)
-
-trying postgresql://host/db?u%73er=someotheruser&port=12345
-user='someotheruser' dbname='db' host='host' port='12345' (inet)
-
-trying postgresql://host/db?u%7aer=someotheruser&port=12345
-uri-regress: invalid URI query parameter: "uzer"
-
-trying postgresql://host:12345?user=uri-user
-user='uri-user' host='host' port='12345' (inet)
-
-trying postgresql://host?user=uri-user
-user='uri-user' host='host' (inet)
-
-trying postgresql://host?
-host='host' (inet)
-
-trying postgresql://[::1]:12345/db
-dbname='db' host='::1' port='12345' (inet)
-
-trying postgresql://[::1]/db
-dbname='db' host='::1' (inet)
-
-trying postgresql://[2001:db8::1234]/
-host='2001:db8::1234' (inet)
-
-trying postgresql://[200z:db8::1234]/
-host='200z:db8::1234' (inet)
-
-trying postgresql://[::1]
-host='::1' (inet)
-
-trying postgres://
-(local)
-
-trying postgres:///
-(local)
-
-trying postgres:///db
-dbname='db' (local)
-
-trying postgres://uri-user@/db
-user='uri-user' dbname='db' (local)
-
-trying postgres://?host=/path/to/socket/dir
-host='/path/to/socket/dir' (local)
-
-trying postgresql://host?uzer=
-uri-regress: invalid URI query parameter: "uzer"
-
-trying postgre://
-uri-regress: missing "=" after "postgre://" in connection info string
-
-trying postgres://[::1
-uri-regress: end of string reached when looking for matching "]" in IPv6 host address in URI: "postgres://[::1"
-
-trying postgres://[]
-uri-regress: IPv6 host address may not be empty in URI: "postgres://[]"
-
-trying postgres://[::1]z
-uri-regress: unexpected character "z" at position 17 in URI (expected ":" or "/"): "postgres://[::1]z"
-
-trying postgresql://host?zzz
-uri-regress: missing key/value separator "=" in URI query parameter: "zzz"
-
-trying postgresql://host?value1&value2
-uri-regress: missing key/value separator "=" in URI query parameter: "value1"
-
-trying postgresql://host?key=key=value
-uri-regress: extra key/value separator "=" in URI query parameter: "key"
-
-trying postgres://host?dbname=%XXfoo
-uri-regress: invalid percent-encoded token: "%XXfoo"
-
-trying postgresql://a%00b
-uri-regress: forbidden value %00 in percent-encoded value: "a%00b"
-
-trying postgresql://%zz
-uri-regress: invalid percent-encoded token: "%zz"
-
-trying postgresql://%1
-uri-regress: invalid percent-encoded token: "%1"
-
-trying postgresql://%
-uri-regress: invalid percent-encoded token: "%"
-
-trying postgres://@host
-host='host' (inet)
-
-trying postgres://host:/
-host='host' (inet)
-
-trying postgres://:12345/
-port='12345' (local)
-
-trying postgres://otheruser@?host=/no/such/directory
-user='otheruser' host='/no/such/directory' (local)
-
-trying postgres://otheruser@/?host=/no/such/directory
-user='otheruser' host='/no/such/directory' (local)
-
-trying postgres://otheruser@:12345?host=/no/such/socket/path
-user='otheruser' host='/no/such/socket/path' port='12345' (local)
-
-trying postgres://otheruser@:12345/db?host=/path/to/socket
-user='otheruser' dbname='db' host='/path/to/socket' port='12345' (local)
-
-trying postgres://:12345/db?host=/path/to/socket
-dbname='db' host='/path/to/socket' port='12345' (local)
-
-trying postgres://:12345?host=/path/to/socket
-host='/path/to/socket' port='12345' (local)
-
-trying postgres://%2Fvar%2Flib%2Fpostgresql/dbname
-dbname='dbname' host='/var/lib/postgresql' (local)
-
diff --git a/src/interfaces/libpq/test/regress.in b/src/interfaces/libpq/test/regress.in
deleted file mode 100644
index de034f39141..00000000000
--- a/src/interfaces/libpq/test/regress.in
+++ /dev/null
@@ -1,57 +0,0 @@
-postgresql://uri-user:secret@host:12345/db
-postgresql://uri-user@host:12345/db
-postgresql://uri-user@host/db
-postgresql://host:12345/db
-postgresql://host/db
-postgresql://uri-user@host:12345/
-postgresql://uri-user@host/
-postgresql://uri-user@
-postgresql://host:12345/
-postgresql://host:12345
-postgresql://host/db
-postgresql://host/
-postgresql://host
-postgresql://
-postgresql://?hostaddr=127.0.0.1
-postgresql://example.com?hostaddr=63.1.2.4
-postgresql://%68ost/
-postgresql://host/db?user=uri-user
-postgresql://host/db?user=uri-user&port=12345
-postgresql://host/db?u%73er=someotheruser&port=12345
-postgresql://host/db?u%7aer=someotheruser&port=12345
-postgresql://host:12345?user=uri-user
-postgresql://host?user=uri-user
-postgresql://host?
-postgresql://[::1]:12345/db
-postgresql://[::1]/db
-postgresql://[2001:db8::1234]/
-postgresql://[200z:db8::1234]/
-postgresql://[::1]
-postgres://
-postgres:///
-postgres:///db
-postgres://uri-user@/db
-postgres://?host=/path/to/socket/dir
-postgresql://host?uzer=
-postgre://
-postgres://[::1
-postgres://[]
-postgres://[::1]z
-postgresql://host?zzz
-postgresql://host?value1&value2
-postgresql://host?key=key=value
-postgres://host?dbname=%XXfoo
-postgresql://a%00b
-postgresql://%zz
-postgresql://%1
-postgresql://%
-postgres://@host
-postgres://host:/
-postgres://:12345/
-postgres://otheruser@?host=/no/such/directory
-postgres://otheruser@/?host=/no/such/directory
-postgres://otheruser@:12345?host=/no/such/socket/path
-postgres://otheruser@:12345/db?host=/path/to/socket
-postgres://:12345/db?host=/path/to/socket
-postgres://:12345?host=/path/to/socket
-postgres://%2Fvar%2Flib%2Fpostgresql/dbname
diff --git a/src/interfaces/libpq/test/regress.pl b/src/interfaces/libpq/test/regress.pl
deleted file mode 100644
index 70691dabe68..00000000000
--- a/src/interfaces/libpq/test/regress.pl
+++ /dev/null
@@ -1,65 +0,0 @@
-#!/usr/bin/perl
-
-# Copyright (c) 2021-2022, PostgreSQL Global Development Group
-
-use strict;
-use warnings;
-
-# use of SRCDIR/SUBDIR is required for supporting VPath builds
-my $srcdir = $ENV{'SRCDIR'} or die 'SRCDIR environment variable is not set';
-my $subdir = $ENV{'SUBDIR'} or die 'SUBDIR environment variable is not set';
-
-my $regress_in = "$srcdir/$subdir/regress.in";
-my $expected_out = "$srcdir/$subdir/expected.out";
-
-# the output file should land in the build_dir of VPath, or just in
-# the current dir, if VPath isn't used
-my $regress_out = "regress.out";
-
-# open input file first, so possible error isn't sent to redirected STDERR
-open(my $regress_in_fh, "<", $regress_in)
- or die "can't open $regress_in for reading: $!";
-
-# save STDOUT/ERR and redirect both to regress.out
-open(my $oldout_fh, ">&", \*STDOUT) or die "can't dup STDOUT: $!";
-open(my $olderr_fh, ">&", \*STDERR) or die "can't dup STDERR: $!";
-
-open(STDOUT, ">", $regress_out)
- or die "can't open $regress_out for writing: $!";
-open(STDERR, ">&", \*STDOUT) or die "can't dup STDOUT: $!";
-
-# read lines from regress.in and run uri-regress on them
-while (<$regress_in_fh>)
-{
- chomp;
- print "trying $_\n";
- system("./uri-regress \"$_\"");
- print "\n";
-}
-
-# restore STDOUT/ERR so we can print the outcome to the user
-open(STDERR, ">&", $olderr_fh)
- or die; # can't complain as STDERR is still duped
-open(STDOUT, ">&", $oldout_fh) or die "can't restore STDOUT: $!";
-
-# just in case
-close $regress_in_fh;
-
-my $diff_status = system(
- "diff -c \"$srcdir/$subdir/expected.out\" regress.out >regress.diff");
-
-print "=" x 70, "\n";
-if ($diff_status == 0)
-{
- print "All tests passed\n";
- exit 0;
-}
-else
-{
- print <<EOF;
-FAILED: the test result differs from the expected output
-
-Review the difference in "$subdir/regress.diff"
-EOF
- exit 1;
-}
--
2.34.0
v2-0002-Add-tap-test-infrastructure-to-src-interfaces-lib.patchtext/x-diff; charset=us-asciiDownload
From d66a339e051cee97847b6e5c1e1b5b9d846562c3 Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Thu, 24 Feb 2022 08:20:06 -0800
Subject: [PATCH v2 2/3] Add tap test infrastructure to src/interfaces/libpq.
---
src/interfaces/libpq/.gitignore | 1 +
src/interfaces/libpq/Makefile | 13 ++++++++++---
src/Makefile.global.in | 12 ++++++------
3 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/src/interfaces/libpq/.gitignore b/src/interfaces/libpq/.gitignore
index 7478dc344ac..829d683ed27 100644
--- a/src/interfaces/libpq/.gitignore
+++ b/src/interfaces/libpq/.gitignore
@@ -1,2 +1,3 @@
/exports.list
/libpq-refs-stamp
+/tmp_check/
diff --git a/src/interfaces/libpq/Makefile b/src/interfaces/libpq/Makefile
index 844c95d47de..1061547980f 100644
--- a/src/interfaces/libpq/Makefile
+++ b/src/interfaces/libpq/Makefile
@@ -137,8 +137,14 @@ install: all installdirs install-lib
$(INSTALL_DATA) $(srcdir)/pqexpbuffer.h '$(DESTDIR)$(includedir_internal)'
$(INSTALL_DATA) $(srcdir)/pg_service.conf.sample '$(DESTDIR)$(datadir)/pg_service.conf.sample'
-installcheck:
- $(MAKE) -C test $@
+test-build:
+ $(MAKE) -C test all
+
+check: test-build all
+ PATH="$(CURDIR)/test:$$PATH" && $(prove_check)
+
+installcheck: test-build all
+ PATH="$(CURDIR)/test:$$PATH" && $(prove_installcheck)
installdirs: installdirs-lib
$(MKDIR_P) '$(DESTDIR)$(includedir)' '$(DESTDIR)$(includedir_internal)' '$(DESTDIR)$(datadir)'
@@ -151,8 +157,9 @@ uninstall: uninstall-lib
rm -f '$(DESTDIR)$(includedir_internal)/pqexpbuffer.h'
rm -f '$(DESTDIR)$(datadir)/pg_service.conf.sample'
-clean distclean: clean-lib
+clean distclean: clean-lib test-clean
$(MAKE) -C test $@
+ rm -rf tmp_check
rm -f $(OBJS) pthread.h libpq-refs-stamp
# Might be left over from a Win32 client-only build
rm -f pg_config_paths.h
diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index c980444233f..bbdc1c4bda6 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -448,8 +448,8 @@ ifeq ($(enable_tap_tests),yes)
ifndef PGXS
define prove_installcheck
-rm -rf '$(CURDIR)'/tmp_check
-$(MKDIR_P) '$(CURDIR)'/tmp_check
+rm -rf '$(CURDIR)'/tmp_check && \
+$(MKDIR_P) '$(CURDIR)'/tmp_check && \
cd $(srcdir) && \
TESTDIR='$(CURDIR)' PATH="$(bindir):$(CURDIR):$$PATH" \
PGPORT='6$(DEF_PGPORT)' top_builddir='$(CURDIR)/$(top_builddir)' \
@@ -458,8 +458,8 @@ cd $(srcdir) && \
endef
else # PGXS case
define prove_installcheck
-rm -rf '$(CURDIR)'/tmp_check
-$(MKDIR_P) '$(CURDIR)'/tmp_check
+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)' \
@@ -469,8 +469,8 @@ endef
endif # PGXS
define prove_check
-rm -rf '$(CURDIR)'/tmp_check
-$(MKDIR_P) '$(CURDIR)'/tmp_check
+rm -rf '$(CURDIR)'/tmp_check && \
+$(MKDIR_P) '$(CURDIR)'/tmp_check && \
cd $(srcdir) && \
TESTDIR='$(CURDIR)' $(with_temp_install) PGPORT='6$(DEF_PGPORT)' \
PG_REGRESS='$(CURDIR)/$(top_builddir)/src/test/regress/pg_regress' \
--
2.34.0
v2-0003-Move-libpq_pipeline-test-into-src-interfaces-libp.patchtext/x-diff; charset=us-asciiDownload
From 6f06e37bdfd9947339d377b5184194ad7c78b59f Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Thu, 24 Feb 2022 08:27:41 -0800
Subject: [PATCH v2 3/3] Move libpq_pipeline test into src/interfaces/libpq.
---
.../libpq/t/002_pipeline.pl} | 2 +-
src/interfaces/libpq/test/.gitignore | 1 +
src/interfaces/libpq/test/Makefile | 2 +-
.../libpq/test}/libpq_pipeline.c | 0
.../test}/traces/disallowed_in_pipeline.trace | 0
.../libpq/test}/traces/multi_pipelines.trace | 0
.../libpq/test}/traces/nosync.trace | 0
.../libpq/test}/traces/pipeline_abort.trace | 0
.../libpq/test}/traces/prepared.trace | 0
.../libpq/test}/traces/simple_pipeline.trace | 0
.../libpq/test}/traces/singlerow.trace | 0
.../libpq/test}/traces/transaction.trace | 0
src/test/modules/libpq_pipeline/.gitignore | 5 ----
src/test/modules/libpq_pipeline/Makefile | 25 -------------------
src/test/modules/libpq_pipeline/README | 1 -
15 files changed, 3 insertions(+), 33 deletions(-)
rename src/{test/modules/libpq_pipeline/t/001_libpq_pipeline.pl => interfaces/libpq/t/002_pipeline.pl} (96%)
rename src/{test/modules/libpq_pipeline => interfaces/libpq/test}/libpq_pipeline.c (100%)
rename src/{test/modules/libpq_pipeline => interfaces/libpq/test}/traces/disallowed_in_pipeline.trace (100%)
rename src/{test/modules/libpq_pipeline => interfaces/libpq/test}/traces/multi_pipelines.trace (100%)
rename src/{test/modules/libpq_pipeline => interfaces/libpq/test}/traces/nosync.trace (100%)
rename src/{test/modules/libpq_pipeline => interfaces/libpq/test}/traces/pipeline_abort.trace (100%)
rename src/{test/modules/libpq_pipeline => interfaces/libpq/test}/traces/prepared.trace (100%)
rename src/{test/modules/libpq_pipeline => interfaces/libpq/test}/traces/simple_pipeline.trace (100%)
rename src/{test/modules/libpq_pipeline => interfaces/libpq/test}/traces/singlerow.trace (100%)
rename src/{test/modules/libpq_pipeline => interfaces/libpq/test}/traces/transaction.trace (100%)
delete mode 100644 src/test/modules/libpq_pipeline/.gitignore
delete mode 100644 src/test/modules/libpq_pipeline/Makefile
delete mode 100644 src/test/modules/libpq_pipeline/README
diff --git a/src/test/modules/libpq_pipeline/t/001_libpq_pipeline.pl b/src/interfaces/libpq/t/002_pipeline.pl
similarity index 96%
rename from src/test/modules/libpq_pipeline/t/001_libpq_pipeline.pl
rename to src/interfaces/libpq/t/002_pipeline.pl
index 0c164dcaba5..2e288d8ba7c 100644
--- a/src/test/modules/libpq_pipeline/t/001_libpq_pipeline.pl
+++ b/src/interfaces/libpq/t/002_pipeline.pl
@@ -49,7 +49,7 @@ for my $testname (@tests)
my $expected;
my $result;
- $expected = slurp_file_eval("traces/$testname.trace");
+ $expected = slurp_file_eval("test/traces/$testname.trace");
next unless $expected ne "";
$result = slurp_file_eval($traceout);
next unless $result ne "";
diff --git a/src/interfaces/libpq/test/.gitignore b/src/interfaces/libpq/test/.gitignore
index 5e803d8816a..e24d7f64dc3 100644
--- a/src/interfaces/libpq/test/.gitignore
+++ b/src/interfaces/libpq/test/.gitignore
@@ -1 +1,2 @@
/uri-regress
+/libpq_pipeline
diff --git a/src/interfaces/libpq/test/Makefile b/src/interfaces/libpq/test/Makefile
index 54212159065..9f99309653f 100644
--- a/src/interfaces/libpq/test/Makefile
+++ b/src/interfaces/libpq/test/Makefile
@@ -11,7 +11,7 @@ endif
override CPPFLAGS := -I$(libpq_srcdir) $(CPPFLAGS)
LDFLAGS_INTERNAL += $(libpq_pgport)
-PROGS = uri-regress
+PROGS = uri-regress libpq_pipeline
all: $(PROGS)
diff --git a/src/test/modules/libpq_pipeline/libpq_pipeline.c b/src/interfaces/libpq/test/libpq_pipeline.c
similarity index 100%
rename from src/test/modules/libpq_pipeline/libpq_pipeline.c
rename to src/interfaces/libpq/test/libpq_pipeline.c
diff --git a/src/test/modules/libpq_pipeline/traces/disallowed_in_pipeline.trace b/src/interfaces/libpq/test/traces/disallowed_in_pipeline.trace
similarity index 100%
rename from src/test/modules/libpq_pipeline/traces/disallowed_in_pipeline.trace
rename to src/interfaces/libpq/test/traces/disallowed_in_pipeline.trace
diff --git a/src/test/modules/libpq_pipeline/traces/multi_pipelines.trace b/src/interfaces/libpq/test/traces/multi_pipelines.trace
similarity index 100%
rename from src/test/modules/libpq_pipeline/traces/multi_pipelines.trace
rename to src/interfaces/libpq/test/traces/multi_pipelines.trace
diff --git a/src/test/modules/libpq_pipeline/traces/nosync.trace b/src/interfaces/libpq/test/traces/nosync.trace
similarity index 100%
rename from src/test/modules/libpq_pipeline/traces/nosync.trace
rename to src/interfaces/libpq/test/traces/nosync.trace
diff --git a/src/test/modules/libpq_pipeline/traces/pipeline_abort.trace b/src/interfaces/libpq/test/traces/pipeline_abort.trace
similarity index 100%
rename from src/test/modules/libpq_pipeline/traces/pipeline_abort.trace
rename to src/interfaces/libpq/test/traces/pipeline_abort.trace
diff --git a/src/test/modules/libpq_pipeline/traces/prepared.trace b/src/interfaces/libpq/test/traces/prepared.trace
similarity index 100%
rename from src/test/modules/libpq_pipeline/traces/prepared.trace
rename to src/interfaces/libpq/test/traces/prepared.trace
diff --git a/src/test/modules/libpq_pipeline/traces/simple_pipeline.trace b/src/interfaces/libpq/test/traces/simple_pipeline.trace
similarity index 100%
rename from src/test/modules/libpq_pipeline/traces/simple_pipeline.trace
rename to src/interfaces/libpq/test/traces/simple_pipeline.trace
diff --git a/src/test/modules/libpq_pipeline/traces/singlerow.trace b/src/interfaces/libpq/test/traces/singlerow.trace
similarity index 100%
rename from src/test/modules/libpq_pipeline/traces/singlerow.trace
rename to src/interfaces/libpq/test/traces/singlerow.trace
diff --git a/src/test/modules/libpq_pipeline/traces/transaction.trace b/src/interfaces/libpq/test/traces/transaction.trace
similarity index 100%
rename from src/test/modules/libpq_pipeline/traces/transaction.trace
rename to src/interfaces/libpq/test/traces/transaction.trace
diff --git a/src/test/modules/libpq_pipeline/.gitignore b/src/test/modules/libpq_pipeline/.gitignore
deleted file mode 100644
index 3a11e786b83..00000000000
--- a/src/test/modules/libpq_pipeline/.gitignore
+++ /dev/null
@@ -1,5 +0,0 @@
-# Generated subdirectories
-/log/
-/results/
-/tmp_check/
-/libpq_pipeline
diff --git a/src/test/modules/libpq_pipeline/Makefile b/src/test/modules/libpq_pipeline/Makefile
deleted file mode 100644
index 65acc3e997e..00000000000
--- a/src/test/modules/libpq_pipeline/Makefile
+++ /dev/null
@@ -1,25 +0,0 @@
-# src/test/modules/libpq_pipeline/Makefile
-
-PGFILEDESC = "libpq_pipeline - test program for pipeline execution"
-PGAPPICON = win32
-
-PROGRAM = libpq_pipeline
-OBJS = $(WIN32RES) libpq_pipeline.o
-
-NO_INSTALL = 1
-
-PG_CPPFLAGS = -I$(libpq_srcdir)
-PG_LIBS_INTERNAL += $(libpq_pgport)
-
-TAP_TESTS = 1
-
-ifdef USE_PGXS
-PG_CONFIG = pg_config
-PGXS := $(shell $(PG_CONFIG) --pgxs)
-include $(PGXS)
-else
-subdir = src/test/modules/libpq_pipeline
-top_builddir = ../../../..
-include $(top_builddir)/src/Makefile.global
-include $(top_srcdir)/contrib/contrib-global.mk
-endif
diff --git a/src/test/modules/libpq_pipeline/README b/src/test/modules/libpq_pipeline/README
deleted file mode 100644
index d8174dd579a..00000000000
--- a/src/test/modules/libpq_pipeline/README
+++ /dev/null
@@ -1 +0,0 @@
-Test programs and libraries for libpq
--
2.34.0
On Thu, 2022-02-24 at 08:46 -0800, Andres Freund wrote:
One annoying bit is that our current tap invocation infrastructure for msvc
won't know how to deal with that. We put the build directory containing t/
onto PATH, but that won't work for test/. But we also don't want to install
test binaries. Not sure what the solution for that is.
Would it help if the C executable, not Perl, was the thing actually
producing the TAP output? The binaries built from test/ could be placed
into t/. Or does that just open up a new set of problems?
--Jacob
Hi,
On 2022-02-24 17:03:33 +0000, Jacob Champion wrote:
On Thu, 2022-02-24 at 08:46 -0800, Andres Freund wrote:
One annoying bit is that our current tap invocation infrastructure for msvc
won't know how to deal with that. We put the build directory containing t/
onto PATH, but that won't work for test/. But we also don't want to install
test binaries. Not sure what the solution for that is.Would it help if the C executable, not Perl, was the thing actually
producing the TAP output? The binaries built from test/ could be placed
into t/. Or does that just open up a new set of problems?
I don't think it would help that much. And for the libpq pipeline test it
doesn't really make sense anyway, because we intentionally use it with
different traces and such.
I've thought about a few C tap tests that I'd like, but I think that's a
separate discussion.
Greetings,
Andres Freund
On 24.02.22 16:17, Tom Lane wrote:
I think that having t/ directories contain only Perl test scripts
is a good convention that we should stick to. Peter's proposal
of a separate test/ subdirectory for C test scaffolding is
probably fine.
I wonder if there are any conventions in the Perl community about where
to put test support files relative to the "t" directory.
Hi,
On 2022-02-24 08:46:23 -0800, Andres Freund wrote:
I'm mildly inclined to only do 0001 and 0002 for now. We'd not loose msvc
coverage, because it already doesn't build the test. Once we've ironed that
stuff out, we could do 0003?
From what I can see in the buildfarm client, we'd not loose (nor gain) any
buildfarm coverage either. It doesn't run the test today.
Greetings,
Andres Freund
Hi,
On 2022-02-25 09:56:47 -0800, Andres Freund wrote:
On 2022-02-24 08:46:23 -0800, Andres Freund wrote:
I'm mildly inclined to only do 0001 and 0002 for now. We'd not loose msvc
coverage, because it already doesn't build the test. Once we've ironed that
stuff out, we could do 0003?From what I can see in the buildfarm client, we'd not loose (nor gain) any
buildfarm coverage either. It doesn't run the test today.
Attached are rebased patches. I polished 0001, the regress.pl -> 001_uri.pl
conversion some more (although some of perltidy's changes aren't clearly an
improvement).
I'd like to commit 0001 and 0002 soon, unless somebody sees a reason not to?
Greetings,
Andres Freund
Attachments:
v3-0001-Convert-src-interfaces-libpq-test-to-a-tap-test.patchtext/x-diff; charset=us-asciiDownload
From 87017e4b0a78c486eca24aab96f32e1168c82b93 Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Wed, 23 Feb 2022 12:22:56 -0800
Subject: [PATCH v3 1/3] Convert src/interfaces/libpq/test to a tap test.
The old form of the test needed a bunch of custom infrastructure. These days
tap tests provide the necessary infrastructure to do better.
We discussed whether to move this test to src/test/modules, alongside
libpq_pipeline, but concluded that the opposite direction would be
better. libpq_pipeline will be moved at a later date, once the buildfarm and
msvc build infrastructure is ready for it.
The invocation of the tap test will be added in the next commit. It involves
just enough buildsystem changes to be worth commiting separately. Can't happen
the other way round because prove errors out when invoked without tests.
Discussion: https://postgr.es/m/20220223203031.ezrd73ohvjgfksow@alap3.anarazel.de
---
src/interfaces/libpq/t/001_uri.pl | 244 +++++++++++++++++++++++++
src/interfaces/libpq/test/.gitignore | 2 -
src/interfaces/libpq/test/Makefile | 7 +-
src/interfaces/libpq/test/README | 7 -
src/interfaces/libpq/test/expected.out | 171 -----------------
src/interfaces/libpq/test/regress.in | 57 ------
src/interfaces/libpq/test/regress.pl | 65 -------
7 files changed, 246 insertions(+), 307 deletions(-)
create mode 100644 src/interfaces/libpq/t/001_uri.pl
delete mode 100644 src/interfaces/libpq/test/README
delete mode 100644 src/interfaces/libpq/test/expected.out
delete mode 100644 src/interfaces/libpq/test/regress.in
delete mode 100644 src/interfaces/libpq/test/regress.pl
diff --git a/src/interfaces/libpq/t/001_uri.pl b/src/interfaces/libpq/t/001_uri.pl
new file mode 100644
index 00000000000..90f370f8fd6
--- /dev/null
+++ b/src/interfaces/libpq/t/001_uri.pl
@@ -0,0 +1,244 @@
+# Copyright (c) 2021-2022, PostgreSQL Global Development Group
+use strict;
+use warnings;
+
+use PostgreSQL::Test::Utils;
+use Test::More;
+use IPC::Run;
+
+
+# List of URIs tests. For each test the first element is the input string, the
+# second the expected stdout and the third the expected stderr.
+my @tests = (
+ [
+ q{postgresql://uri-user:secret@host:12345/db},
+ q{user='uri-user' password='secret' dbname='db' host='host' port='12345' (inet)},
+ q{},
+ ],
+ [
+ q{postgresql://uri-user@host:12345/db},
+ q{user='uri-user' dbname='db' host='host' port='12345' (inet)}, q{},
+ ],
+ [
+ q{postgresql://uri-user@host/db},
+ q{user='uri-user' dbname='db' host='host' (inet)}, q{},
+ ],
+ [
+ q{postgresql://host:12345/db},
+ q{dbname='db' host='host' port='12345' (inet)}, q{},
+ ],
+ [ q{postgresql://host/db}, q{dbname='db' host='host' (inet)}, q{}, ],
+ [
+ q{postgresql://uri-user@host:12345/},
+ q{user='uri-user' host='host' port='12345' (inet)},
+ q{},
+ ],
+ [
+ q{postgresql://uri-user@host/},
+ q{user='uri-user' host='host' (inet)},
+ q{},
+ ],
+ [ q{postgresql://uri-user@}, q{user='uri-user' (local)}, q{}, ],
+ [ q{postgresql://host:12345/}, q{host='host' port='12345' (inet)}, q{}, ],
+ [ q{postgresql://host:12345}, q{host='host' port='12345' (inet)}, q{}, ],
+ [ q{postgresql://host/db}, q{dbname='db' host='host' (inet)}, q{}, ],
+ [ q{postgresql://host/}, q{host='host' (inet)}, q{}, ],
+ [ q{postgresql://host}, q{host='host' (inet)}, q{}, ],
+ [ q{postgresql://}, q{(local)}, q{}, ],
+ [
+ q{postgresql://?hostaddr=127.0.0.1}, q{hostaddr='127.0.0.1' (inet)},
+ q{},
+ ],
+ [
+ q{postgresql://example.com?hostaddr=63.1.2.4},
+ q{host='example.com' hostaddr='63.1.2.4' (inet)},
+ q{},
+ ],
+ [ q{postgresql://%68ost/}, q{host='host' (inet)}, q{}, ],
+ [
+ q{postgresql://host/db?user=uri-user},
+ q{user='uri-user' dbname='db' host='host' (inet)},
+ q{},
+ ],
+ [
+ q{postgresql://host/db?user=uri-user&port=12345},
+ q{user='uri-user' dbname='db' host='host' port='12345' (inet)},
+ q{},
+ ],
+ [
+ q{postgresql://host/db?u%73er=someotheruser&port=12345},
+ q{user='someotheruser' dbname='db' host='host' port='12345' (inet)},
+ q{},
+ ],
+ [
+ q{postgresql://host/db?u%7aer=someotheruser&port=12345}, q{},
+ q{uri-regress: invalid URI query parameter: "uzer"},
+ ],
+ [
+ q{postgresql://host:12345?user=uri-user},
+ q{user='uri-user' host='host' port='12345' (inet)},
+ q{},
+ ],
+ [
+ q{postgresql://host?user=uri-user},
+ q{user='uri-user' host='host' (inet)},
+ q{},
+ ],
+ [ q{postgresql://host?}, q{host='host' (inet)}, q{}, ],
+ [
+ q{postgresql://[::1]:12345/db},
+ q{dbname='db' host='::1' port='12345' (inet)},
+ q{},
+ ],
+ [ q{postgresql://[::1]/db}, q{dbname='db' host='::1' (inet)}, q{}, ],
+ [
+ q{postgresql://[2001:db8::1234]/}, q{host='2001:db8::1234' (inet)},
+ q{},
+ ],
+ [
+ q{postgresql://[200z:db8::1234]/}, q{host='200z:db8::1234' (inet)},
+ q{},
+ ],
+ [ q{postgresql://[::1]}, q{host='::1' (inet)}, q{}, ],
+ [ q{postgres://}, q{(local)}, q{}, ],
+ [ q{postgres:///}, q{(local)}, q{}, ],
+ [ q{postgres:///db}, q{dbname='db' (local)}, q{}, ],
+ [
+ q{postgres://uri-user@/db}, q{user='uri-user' dbname='db' (local)},
+ q{},
+ ],
+ [
+ q{postgres://?host=/path/to/socket/dir},
+ q{host='/path/to/socket/dir' (local)},
+ q{},
+ ],
+ [
+ q{postgresql://host?uzer=}, q{},
+ q{uri-regress: invalid URI query parameter: "uzer"},
+ ],
+ [
+ q{postgre://},
+ q{},
+ q{uri-regress: missing "=" after "postgre://" in connection info string},
+ ],
+ [
+ q{postgres://[::1},
+ q{},
+ q{uri-regress: end of string reached when looking for matching "]" in IPv6 host address in URI: "postgres://[::1"},
+ ],
+ [
+ q{postgres://[]},
+ q{},
+ q{uri-regress: IPv6 host address may not be empty in URI: "postgres://[]"},
+ ],
+ [
+ q{postgres://[::1]z},
+ q{},
+ q{uri-regress: unexpected character "z" at position 17 in URI (expected ":" or "/"): "postgres://[::1]z"},
+ ],
+ [
+ q{postgresql://host?zzz},
+ q{},
+ q{uri-regress: missing key/value separator "=" in URI query parameter: "zzz"},
+ ],
+ [
+ q{postgresql://host?value1&value2},
+ q{},
+ q{uri-regress: missing key/value separator "=" in URI query parameter: "value1"},
+ ],
+ [
+ q{postgresql://host?key=key=value},
+ q{},
+ q{uri-regress: extra key/value separator "=" in URI query parameter: "key"},
+ ],
+ [
+ q{postgres://host?dbname=%XXfoo}, q{},
+ q{uri-regress: invalid percent-encoded token: "%XXfoo"},
+ ],
+ [
+ q{postgresql://a%00b},
+ q{},
+ q{uri-regress: forbidden value %00 in percent-encoded value: "a%00b"},
+ ],
+ [
+ q{postgresql://%zz}, q{},
+ q{uri-regress: invalid percent-encoded token: "%zz"},
+ ],
+ [
+ q{postgresql://%1}, q{},
+ q{uri-regress: invalid percent-encoded token: "%1"},
+ ],
+ [
+ q{postgresql://%}, q{},
+ q{uri-regress: invalid percent-encoded token: "%"},
+ ],
+ [ q{postgres://@host}, q{host='host' (inet)}, q{}, ],
+ [ q{postgres://host:/}, q{host='host' (inet)}, q{}, ],
+ [ q{postgres://:12345/}, q{port='12345' (local)}, q{}, ],
+ [
+ q{postgres://otheruser@?host=/no/such/directory},
+ q{user='otheruser' host='/no/such/directory' (local)},
+ q{},
+ ],
+ [
+ q{postgres://otheruser@/?host=/no/such/directory},
+ q{user='otheruser' host='/no/such/directory' (local)},
+ q{},
+ ],
+ [
+ q{postgres://otheruser@:12345?host=/no/such/socket/path},
+ q{user='otheruser' host='/no/such/socket/path' port='12345' (local)},
+ q{},
+ ],
+ [
+ q{postgres://otheruser@:12345/db?host=/path/to/socket},
+ q{user='otheruser' dbname='db' host='/path/to/socket' port='12345' (local)},
+ q{},
+ ],
+ [
+ q{postgres://:12345/db?host=/path/to/socket},
+ q{dbname='db' host='/path/to/socket' port='12345' (local)},
+ q{},
+ ],
+ [
+ q{postgres://:12345?host=/path/to/socket},
+ q{host='/path/to/socket' port='12345' (local)},
+ q{},
+ ],
+ [
+ q{postgres://%2Fvar%2Flib%2Fpostgresql/dbname},
+ q{dbname='dbname' host='/var/lib/postgresql' (local)},
+ q{},
+ ]);
+
+# test to run for each of the above test definitions
+sub test_uri
+{
+ local $Test::Builder::Level = $Test::Builder::Level + 1;
+
+ my $uri;
+ my %expect;
+ my %result;
+
+ ($uri, $expect{stdout}, $expect{stderr}) = @$_;
+
+ $expect{'exit'} = $expect{stderr} eq '';
+
+ my $cmd = [ 'uri-regress', $uri ];
+ $result{exit} = IPC::Run::run $cmd, '>', \$result{stdout}, '2>',
+ \$result{stderr};
+
+ chomp($result{stdout});
+ chomp($result{stderr});
+
+ # use is_deeply so there's one test result for each test above, without
+ # loosing the information whether stdout/stderr mismatched.
+ is_deeply(\%result, \%expect, $uri);
+}
+
+foreach (@tests)
+{
+ test_uri($_);
+}
+
+done_testing();
diff --git a/src/interfaces/libpq/test/.gitignore b/src/interfaces/libpq/test/.gitignore
index 5387b3b6d94..5e803d8816a 100644
--- a/src/interfaces/libpq/test/.gitignore
+++ b/src/interfaces/libpq/test/.gitignore
@@ -1,3 +1 @@
/uri-regress
-/regress.diff
-/regress.out
diff --git a/src/interfaces/libpq/test/Makefile b/src/interfaces/libpq/test/Makefile
index 4832fab9d23..54212159065 100644
--- a/src/interfaces/libpq/test/Makefile
+++ b/src/interfaces/libpq/test/Makefile
@@ -1,3 +1,5 @@
+# src/interfaces/libpq/test/Makefile
+
subdir = src/interfaces/libpq/test
top_builddir = ../../../..
include $(top_builddir)/src/Makefile.global
@@ -13,10 +15,5 @@ PROGS = uri-regress
all: $(PROGS)
-installcheck: all
- SRCDIR='$(top_srcdir)' SUBDIR='$(subdir)' \
- $(PERL) $(top_srcdir)/$(subdir)/regress.pl
-
clean distclean maintainer-clean:
rm -f $(PROGS) *.o
- rm -f regress.out regress.diff
diff --git a/src/interfaces/libpq/test/README b/src/interfaces/libpq/test/README
deleted file mode 100644
index a05eb6bb3bc..00000000000
--- a/src/interfaces/libpq/test/README
+++ /dev/null
@@ -1,7 +0,0 @@
-This is a testsuite for testing libpq URI connection string syntax.
-
-To run the suite, use 'make installcheck' command. It works by
-running 'regress.pl' from this directory with appropriate environment
-set up, which in turn feeds up lines from 'regress.in' to
-'uri-regress' test program and compares the output against the correct
-one in 'expected.out' file.
diff --git a/src/interfaces/libpq/test/expected.out b/src/interfaces/libpq/test/expected.out
deleted file mode 100644
index d375e82b4ac..00000000000
--- a/src/interfaces/libpq/test/expected.out
+++ /dev/null
@@ -1,171 +0,0 @@
-trying postgresql://uri-user:secret@host:12345/db
-user='uri-user' password='secret' dbname='db' host='host' port='12345' (inet)
-
-trying postgresql://uri-user@host:12345/db
-user='uri-user' dbname='db' host='host' port='12345' (inet)
-
-trying postgresql://uri-user@host/db
-user='uri-user' dbname='db' host='host' (inet)
-
-trying postgresql://host:12345/db
-dbname='db' host='host' port='12345' (inet)
-
-trying postgresql://host/db
-dbname='db' host='host' (inet)
-
-trying postgresql://uri-user@host:12345/
-user='uri-user' host='host' port='12345' (inet)
-
-trying postgresql://uri-user@host/
-user='uri-user' host='host' (inet)
-
-trying postgresql://uri-user@
-user='uri-user' (local)
-
-trying postgresql://host:12345/
-host='host' port='12345' (inet)
-
-trying postgresql://host:12345
-host='host' port='12345' (inet)
-
-trying postgresql://host/db
-dbname='db' host='host' (inet)
-
-trying postgresql://host/
-host='host' (inet)
-
-trying postgresql://host
-host='host' (inet)
-
-trying postgresql://
-(local)
-
-trying postgresql://?hostaddr=127.0.0.1
-hostaddr='127.0.0.1' (inet)
-
-trying postgresql://example.com?hostaddr=63.1.2.4
-host='example.com' hostaddr='63.1.2.4' (inet)
-
-trying postgresql://%68ost/
-host='host' (inet)
-
-trying postgresql://host/db?user=uri-user
-user='uri-user' dbname='db' host='host' (inet)
-
-trying postgresql://host/db?user=uri-user&port=12345
-user='uri-user' dbname='db' host='host' port='12345' (inet)
-
-trying postgresql://host/db?u%73er=someotheruser&port=12345
-user='someotheruser' dbname='db' host='host' port='12345' (inet)
-
-trying postgresql://host/db?u%7aer=someotheruser&port=12345
-uri-regress: invalid URI query parameter: "uzer"
-
-trying postgresql://host:12345?user=uri-user
-user='uri-user' host='host' port='12345' (inet)
-
-trying postgresql://host?user=uri-user
-user='uri-user' host='host' (inet)
-
-trying postgresql://host?
-host='host' (inet)
-
-trying postgresql://[::1]:12345/db
-dbname='db' host='::1' port='12345' (inet)
-
-trying postgresql://[::1]/db
-dbname='db' host='::1' (inet)
-
-trying postgresql://[2001:db8::1234]/
-host='2001:db8::1234' (inet)
-
-trying postgresql://[200z:db8::1234]/
-host='200z:db8::1234' (inet)
-
-trying postgresql://[::1]
-host='::1' (inet)
-
-trying postgres://
-(local)
-
-trying postgres:///
-(local)
-
-trying postgres:///db
-dbname='db' (local)
-
-trying postgres://uri-user@/db
-user='uri-user' dbname='db' (local)
-
-trying postgres://?host=/path/to/socket/dir
-host='/path/to/socket/dir' (local)
-
-trying postgresql://host?uzer=
-uri-regress: invalid URI query parameter: "uzer"
-
-trying postgre://
-uri-regress: missing "=" after "postgre://" in connection info string
-
-trying postgres://[::1
-uri-regress: end of string reached when looking for matching "]" in IPv6 host address in URI: "postgres://[::1"
-
-trying postgres://[]
-uri-regress: IPv6 host address may not be empty in URI: "postgres://[]"
-
-trying postgres://[::1]z
-uri-regress: unexpected character "z" at position 17 in URI (expected ":" or "/"): "postgres://[::1]z"
-
-trying postgresql://host?zzz
-uri-regress: missing key/value separator "=" in URI query parameter: "zzz"
-
-trying postgresql://host?value1&value2
-uri-regress: missing key/value separator "=" in URI query parameter: "value1"
-
-trying postgresql://host?key=key=value
-uri-regress: extra key/value separator "=" in URI query parameter: "key"
-
-trying postgres://host?dbname=%XXfoo
-uri-regress: invalid percent-encoded token: "%XXfoo"
-
-trying postgresql://a%00b
-uri-regress: forbidden value %00 in percent-encoded value: "a%00b"
-
-trying postgresql://%zz
-uri-regress: invalid percent-encoded token: "%zz"
-
-trying postgresql://%1
-uri-regress: invalid percent-encoded token: "%1"
-
-trying postgresql://%
-uri-regress: invalid percent-encoded token: "%"
-
-trying postgres://@host
-host='host' (inet)
-
-trying postgres://host:/
-host='host' (inet)
-
-trying postgres://:12345/
-port='12345' (local)
-
-trying postgres://otheruser@?host=/no/such/directory
-user='otheruser' host='/no/such/directory' (local)
-
-trying postgres://otheruser@/?host=/no/such/directory
-user='otheruser' host='/no/such/directory' (local)
-
-trying postgres://otheruser@:12345?host=/no/such/socket/path
-user='otheruser' host='/no/such/socket/path' port='12345' (local)
-
-trying postgres://otheruser@:12345/db?host=/path/to/socket
-user='otheruser' dbname='db' host='/path/to/socket' port='12345' (local)
-
-trying postgres://:12345/db?host=/path/to/socket
-dbname='db' host='/path/to/socket' port='12345' (local)
-
-trying postgres://:12345?host=/path/to/socket
-host='/path/to/socket' port='12345' (local)
-
-trying postgres://%2Fvar%2Flib%2Fpostgresql/dbname
-dbname='dbname' host='/var/lib/postgresql' (local)
-
diff --git a/src/interfaces/libpq/test/regress.in b/src/interfaces/libpq/test/regress.in
deleted file mode 100644
index de034f39141..00000000000
--- a/src/interfaces/libpq/test/regress.in
+++ /dev/null
@@ -1,57 +0,0 @@
-postgresql://uri-user:secret@host:12345/db
-postgresql://uri-user@host:12345/db
-postgresql://uri-user@host/db
-postgresql://host:12345/db
-postgresql://host/db
-postgresql://uri-user@host:12345/
-postgresql://uri-user@host/
-postgresql://uri-user@
-postgresql://host:12345/
-postgresql://host:12345
-postgresql://host/db
-postgresql://host/
-postgresql://host
-postgresql://
-postgresql://?hostaddr=127.0.0.1
-postgresql://example.com?hostaddr=63.1.2.4
-postgresql://%68ost/
-postgresql://host/db?user=uri-user
-postgresql://host/db?user=uri-user&port=12345
-postgresql://host/db?u%73er=someotheruser&port=12345
-postgresql://host/db?u%7aer=someotheruser&port=12345
-postgresql://host:12345?user=uri-user
-postgresql://host?user=uri-user
-postgresql://host?
-postgresql://[::1]:12345/db
-postgresql://[::1]/db
-postgresql://[2001:db8::1234]/
-postgresql://[200z:db8::1234]/
-postgresql://[::1]
-postgres://
-postgres:///
-postgres:///db
-postgres://uri-user@/db
-postgres://?host=/path/to/socket/dir
-postgresql://host?uzer=
-postgre://
-postgres://[::1
-postgres://[]
-postgres://[::1]z
-postgresql://host?zzz
-postgresql://host?value1&value2
-postgresql://host?key=key=value
-postgres://host?dbname=%XXfoo
-postgresql://a%00b
-postgresql://%zz
-postgresql://%1
-postgresql://%
-postgres://@host
-postgres://host:/
-postgres://:12345/
-postgres://otheruser@?host=/no/such/directory
-postgres://otheruser@/?host=/no/such/directory
-postgres://otheruser@:12345?host=/no/such/socket/path
-postgres://otheruser@:12345/db?host=/path/to/socket
-postgres://:12345/db?host=/path/to/socket
-postgres://:12345?host=/path/to/socket
-postgres://%2Fvar%2Flib%2Fpostgresql/dbname
diff --git a/src/interfaces/libpq/test/regress.pl b/src/interfaces/libpq/test/regress.pl
deleted file mode 100644
index 70691dabe68..00000000000
--- a/src/interfaces/libpq/test/regress.pl
+++ /dev/null
@@ -1,65 +0,0 @@
-#!/usr/bin/perl
-
-# Copyright (c) 2021-2022, PostgreSQL Global Development Group
-
-use strict;
-use warnings;
-
-# use of SRCDIR/SUBDIR is required for supporting VPath builds
-my $srcdir = $ENV{'SRCDIR'} or die 'SRCDIR environment variable is not set';
-my $subdir = $ENV{'SUBDIR'} or die 'SUBDIR environment variable is not set';
-
-my $regress_in = "$srcdir/$subdir/regress.in";
-my $expected_out = "$srcdir/$subdir/expected.out";
-
-# the output file should land in the build_dir of VPath, or just in
-# the current dir, if VPath isn't used
-my $regress_out = "regress.out";
-
-# open input file first, so possible error isn't sent to redirected STDERR
-open(my $regress_in_fh, "<", $regress_in)
- or die "can't open $regress_in for reading: $!";
-
-# save STDOUT/ERR and redirect both to regress.out
-open(my $oldout_fh, ">&", \*STDOUT) or die "can't dup STDOUT: $!";
-open(my $olderr_fh, ">&", \*STDERR) or die "can't dup STDERR: $!";
-
-open(STDOUT, ">", $regress_out)
- or die "can't open $regress_out for writing: $!";
-open(STDERR, ">&", \*STDOUT) or die "can't dup STDOUT: $!";
-
-# read lines from regress.in and run uri-regress on them
-while (<$regress_in_fh>)
-{
- chomp;
- print "trying $_\n";
- system("./uri-regress \"$_\"");
- print "\n";
-}
-
-# restore STDOUT/ERR so we can print the outcome to the user
-open(STDERR, ">&", $olderr_fh)
- or die; # can't complain as STDERR is still duped
-open(STDOUT, ">&", $oldout_fh) or die "can't restore STDOUT: $!";
-
-# just in case
-close $regress_in_fh;
-
-my $diff_status = system(
- "diff -c \"$srcdir/$subdir/expected.out\" regress.out >regress.diff");
-
-print "=" x 70, "\n";
-if ($diff_status == 0)
-{
- print "All tests passed\n";
- exit 0;
-}
-else
-{
- print <<EOF;
-FAILED: the test result differs from the expected output
-
-Review the difference in "$subdir/regress.diff"
-EOF
- exit 1;
-}
--
2.34.0
v3-0002-Run-tap-tests-in-src-interfaces-libpq.patchtext/x-diff; charset=us-asciiDownload
From a62ec39bbe9dd8179c03cee311dc3c8e3f5a072d Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Thu, 24 Feb 2022 08:20:06 -0800
Subject: [PATCH v3 2/3] Run tap tests in src/interfaces/libpq.
To be able to run binaries in the test/ directory, prove_[install]check need
to be executable in a single shell invocation, so that test/ can be added to
PATH.
Discussion: https://postgr.es/m/20220223203031.ezrd73ohvjgfksow@alap3.anarazel.de
---
src/interfaces/libpq/.gitignore | 1 +
src/interfaces/libpq/Makefile | 13 ++++++++++---
src/Makefile.global.in | 12 ++++++------
3 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/src/interfaces/libpq/.gitignore b/src/interfaces/libpq/.gitignore
index 7478dc344ac..829d683ed27 100644
--- a/src/interfaces/libpq/.gitignore
+++ b/src/interfaces/libpq/.gitignore
@@ -1,2 +1,3 @@
/exports.list
/libpq-refs-stamp
+/tmp_check/
diff --git a/src/interfaces/libpq/Makefile b/src/interfaces/libpq/Makefile
index 844c95d47de..1061547980f 100644
--- a/src/interfaces/libpq/Makefile
+++ b/src/interfaces/libpq/Makefile
@@ -137,8 +137,14 @@ install: all installdirs install-lib
$(INSTALL_DATA) $(srcdir)/pqexpbuffer.h '$(DESTDIR)$(includedir_internal)'
$(INSTALL_DATA) $(srcdir)/pg_service.conf.sample '$(DESTDIR)$(datadir)/pg_service.conf.sample'
-installcheck:
- $(MAKE) -C test $@
+test-build:
+ $(MAKE) -C test all
+
+check: test-build all
+ PATH="$(CURDIR)/test:$$PATH" && $(prove_check)
+
+installcheck: test-build all
+ PATH="$(CURDIR)/test:$$PATH" && $(prove_installcheck)
installdirs: installdirs-lib
$(MKDIR_P) '$(DESTDIR)$(includedir)' '$(DESTDIR)$(includedir_internal)' '$(DESTDIR)$(datadir)'
@@ -151,8 +157,9 @@ uninstall: uninstall-lib
rm -f '$(DESTDIR)$(includedir_internal)/pqexpbuffer.h'
rm -f '$(DESTDIR)$(datadir)/pg_service.conf.sample'
-clean distclean: clean-lib
+clean distclean: clean-lib test-clean
$(MAKE) -C test $@
+ rm -rf tmp_check
rm -f $(OBJS) pthread.h libpq-refs-stamp
# Might be left over from a Win32 client-only build
rm -f pg_config_paths.h
diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index c980444233f..bbdc1c4bda6 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -448,8 +448,8 @@ ifeq ($(enable_tap_tests),yes)
ifndef PGXS
define prove_installcheck
-rm -rf '$(CURDIR)'/tmp_check
-$(MKDIR_P) '$(CURDIR)'/tmp_check
+rm -rf '$(CURDIR)'/tmp_check && \
+$(MKDIR_P) '$(CURDIR)'/tmp_check && \
cd $(srcdir) && \
TESTDIR='$(CURDIR)' PATH="$(bindir):$(CURDIR):$$PATH" \
PGPORT='6$(DEF_PGPORT)' top_builddir='$(CURDIR)/$(top_builddir)' \
@@ -458,8 +458,8 @@ cd $(srcdir) && \
endef
else # PGXS case
define prove_installcheck
-rm -rf '$(CURDIR)'/tmp_check
-$(MKDIR_P) '$(CURDIR)'/tmp_check
+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)' \
@@ -469,8 +469,8 @@ endef
endif # PGXS
define prove_check
-rm -rf '$(CURDIR)'/tmp_check
-$(MKDIR_P) '$(CURDIR)'/tmp_check
+rm -rf '$(CURDIR)'/tmp_check && \
+$(MKDIR_P) '$(CURDIR)'/tmp_check && \
cd $(srcdir) && \
TESTDIR='$(CURDIR)' $(with_temp_install) PGPORT='6$(DEF_PGPORT)' \
PG_REGRESS='$(CURDIR)/$(top_builddir)/src/test/regress/pg_regress' \
--
2.34.0
v3-0003-Move-libpq_pipeline-test-into-src-interfaces-libp.patchtext/x-diff; charset=us-asciiDownload
From c603f34456d9feefd8d6ce52855d09744754fd2f Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Thu, 24 Feb 2022 08:27:41 -0800
Subject: [PATCH v3 3/3] Move libpq_pipeline test into src/interfaces/libpq.
---
.../libpq/t/002_pipeline.pl} | 2 +-
src/interfaces/libpq/test/.gitignore | 1 +
src/interfaces/libpq/test/Makefile | 2 +-
.../libpq/test}/libpq_pipeline.c | 0
.../test}/traces/disallowed_in_pipeline.trace | 0
.../libpq/test}/traces/multi_pipelines.trace | 0
.../libpq/test}/traces/nosync.trace | 0
.../libpq/test}/traces/pipeline_abort.trace | 0
.../libpq/test}/traces/prepared.trace | 0
.../libpq/test}/traces/simple_pipeline.trace | 0
.../libpq/test}/traces/singlerow.trace | 0
.../libpq/test}/traces/transaction.trace | 0
src/test/modules/libpq_pipeline/.gitignore | 5 ----
src/test/modules/libpq_pipeline/Makefile | 25 -------------------
src/test/modules/libpq_pipeline/README | 1 -
15 files changed, 3 insertions(+), 33 deletions(-)
rename src/{test/modules/libpq_pipeline/t/001_libpq_pipeline.pl => interfaces/libpq/t/002_pipeline.pl} (96%)
rename src/{test/modules/libpq_pipeline => interfaces/libpq/test}/libpq_pipeline.c (100%)
rename src/{test/modules/libpq_pipeline => interfaces/libpq/test}/traces/disallowed_in_pipeline.trace (100%)
rename src/{test/modules/libpq_pipeline => interfaces/libpq/test}/traces/multi_pipelines.trace (100%)
rename src/{test/modules/libpq_pipeline => interfaces/libpq/test}/traces/nosync.trace (100%)
rename src/{test/modules/libpq_pipeline => interfaces/libpq/test}/traces/pipeline_abort.trace (100%)
rename src/{test/modules/libpq_pipeline => interfaces/libpq/test}/traces/prepared.trace (100%)
rename src/{test/modules/libpq_pipeline => interfaces/libpq/test}/traces/simple_pipeline.trace (100%)
rename src/{test/modules/libpq_pipeline => interfaces/libpq/test}/traces/singlerow.trace (100%)
rename src/{test/modules/libpq_pipeline => interfaces/libpq/test}/traces/transaction.trace (100%)
delete mode 100644 src/test/modules/libpq_pipeline/.gitignore
delete mode 100644 src/test/modules/libpq_pipeline/Makefile
delete mode 100644 src/test/modules/libpq_pipeline/README
diff --git a/src/test/modules/libpq_pipeline/t/001_libpq_pipeline.pl b/src/interfaces/libpq/t/002_pipeline.pl
similarity index 96%
rename from src/test/modules/libpq_pipeline/t/001_libpq_pipeline.pl
rename to src/interfaces/libpq/t/002_pipeline.pl
index 0c164dcaba5..2e288d8ba7c 100644
--- a/src/test/modules/libpq_pipeline/t/001_libpq_pipeline.pl
+++ b/src/interfaces/libpq/t/002_pipeline.pl
@@ -49,7 +49,7 @@ for my $testname (@tests)
my $expected;
my $result;
- $expected = slurp_file_eval("traces/$testname.trace");
+ $expected = slurp_file_eval("test/traces/$testname.trace");
next unless $expected ne "";
$result = slurp_file_eval($traceout);
next unless $result ne "";
diff --git a/src/interfaces/libpq/test/.gitignore b/src/interfaces/libpq/test/.gitignore
index 5e803d8816a..e24d7f64dc3 100644
--- a/src/interfaces/libpq/test/.gitignore
+++ b/src/interfaces/libpq/test/.gitignore
@@ -1 +1,2 @@
/uri-regress
+/libpq_pipeline
diff --git a/src/interfaces/libpq/test/Makefile b/src/interfaces/libpq/test/Makefile
index 54212159065..9f99309653f 100644
--- a/src/interfaces/libpq/test/Makefile
+++ b/src/interfaces/libpq/test/Makefile
@@ -11,7 +11,7 @@ endif
override CPPFLAGS := -I$(libpq_srcdir) $(CPPFLAGS)
LDFLAGS_INTERNAL += $(libpq_pgport)
-PROGS = uri-regress
+PROGS = uri-regress libpq_pipeline
all: $(PROGS)
diff --git a/src/test/modules/libpq_pipeline/libpq_pipeline.c b/src/interfaces/libpq/test/libpq_pipeline.c
similarity index 100%
rename from src/test/modules/libpq_pipeline/libpq_pipeline.c
rename to src/interfaces/libpq/test/libpq_pipeline.c
diff --git a/src/test/modules/libpq_pipeline/traces/disallowed_in_pipeline.trace b/src/interfaces/libpq/test/traces/disallowed_in_pipeline.trace
similarity index 100%
rename from src/test/modules/libpq_pipeline/traces/disallowed_in_pipeline.trace
rename to src/interfaces/libpq/test/traces/disallowed_in_pipeline.trace
diff --git a/src/test/modules/libpq_pipeline/traces/multi_pipelines.trace b/src/interfaces/libpq/test/traces/multi_pipelines.trace
similarity index 100%
rename from src/test/modules/libpq_pipeline/traces/multi_pipelines.trace
rename to src/interfaces/libpq/test/traces/multi_pipelines.trace
diff --git a/src/test/modules/libpq_pipeline/traces/nosync.trace b/src/interfaces/libpq/test/traces/nosync.trace
similarity index 100%
rename from src/test/modules/libpq_pipeline/traces/nosync.trace
rename to src/interfaces/libpq/test/traces/nosync.trace
diff --git a/src/test/modules/libpq_pipeline/traces/pipeline_abort.trace b/src/interfaces/libpq/test/traces/pipeline_abort.trace
similarity index 100%
rename from src/test/modules/libpq_pipeline/traces/pipeline_abort.trace
rename to src/interfaces/libpq/test/traces/pipeline_abort.trace
diff --git a/src/test/modules/libpq_pipeline/traces/prepared.trace b/src/interfaces/libpq/test/traces/prepared.trace
similarity index 100%
rename from src/test/modules/libpq_pipeline/traces/prepared.trace
rename to src/interfaces/libpq/test/traces/prepared.trace
diff --git a/src/test/modules/libpq_pipeline/traces/simple_pipeline.trace b/src/interfaces/libpq/test/traces/simple_pipeline.trace
similarity index 100%
rename from src/test/modules/libpq_pipeline/traces/simple_pipeline.trace
rename to src/interfaces/libpq/test/traces/simple_pipeline.trace
diff --git a/src/test/modules/libpq_pipeline/traces/singlerow.trace b/src/interfaces/libpq/test/traces/singlerow.trace
similarity index 100%
rename from src/test/modules/libpq_pipeline/traces/singlerow.trace
rename to src/interfaces/libpq/test/traces/singlerow.trace
diff --git a/src/test/modules/libpq_pipeline/traces/transaction.trace b/src/interfaces/libpq/test/traces/transaction.trace
similarity index 100%
rename from src/test/modules/libpq_pipeline/traces/transaction.trace
rename to src/interfaces/libpq/test/traces/transaction.trace
diff --git a/src/test/modules/libpq_pipeline/.gitignore b/src/test/modules/libpq_pipeline/.gitignore
deleted file mode 100644
index 3a11e786b83..00000000000
--- a/src/test/modules/libpq_pipeline/.gitignore
+++ /dev/null
@@ -1,5 +0,0 @@
-# Generated subdirectories
-/log/
-/results/
-/tmp_check/
-/libpq_pipeline
diff --git a/src/test/modules/libpq_pipeline/Makefile b/src/test/modules/libpq_pipeline/Makefile
deleted file mode 100644
index 65acc3e997e..00000000000
--- a/src/test/modules/libpq_pipeline/Makefile
+++ /dev/null
@@ -1,25 +0,0 @@
-# src/test/modules/libpq_pipeline/Makefile
-
-PGFILEDESC = "libpq_pipeline - test program for pipeline execution"
-PGAPPICON = win32
-
-PROGRAM = libpq_pipeline
-OBJS = $(WIN32RES) libpq_pipeline.o
-
-NO_INSTALL = 1
-
-PG_CPPFLAGS = -I$(libpq_srcdir)
-PG_LIBS_INTERNAL += $(libpq_pgport)
-
-TAP_TESTS = 1
-
-ifdef USE_PGXS
-PG_CONFIG = pg_config
-PGXS := $(shell $(PG_CONFIG) --pgxs)
-include $(PGXS)
-else
-subdir = src/test/modules/libpq_pipeline
-top_builddir = ../../../..
-include $(top_builddir)/src/Makefile.global
-include $(top_srcdir)/contrib/contrib-global.mk
-endif
diff --git a/src/test/modules/libpq_pipeline/README b/src/test/modules/libpq_pipeline/README
deleted file mode 100644
index d8174dd579a..00000000000
--- a/src/test/modules/libpq_pipeline/README
+++ /dev/null
@@ -1 +0,0 @@
-Test programs and libraries for libpq
--
2.34.0
Hi,
On 2022-02-25 17:52:29 -0800, Andres Freund wrote:
I'd like to commit 0001 and 0002 soon, unless somebody sees a reason not to?
Pushed. Attached is the remainder, 0003, the move of libpq_pipeline to
src/interfaces/libpq that I'm not planning to push for now.
Regards,
Andres
Attachments:
v4-0001-Move-libpq_pipeline-test-into-src-interfaces-libp.patchtext/x-diff; charset=us-asciiDownload
From 61a89721c8aab3a1fd2300067c470807b4fe87bc Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Thu, 24 Feb 2022 08:27:41 -0800
Subject: [PATCH v4] Move libpq_pipeline test into src/interfaces/libpq.
---
.../libpq/t/002_pipeline.pl} | 2 +-
src/interfaces/libpq/test/.gitignore | 1 +
src/interfaces/libpq/test/Makefile | 2 +-
.../libpq/test}/libpq_pipeline.c | 0
.../test}/traces/disallowed_in_pipeline.trace | 0
.../libpq/test}/traces/multi_pipelines.trace | 0
.../libpq/test}/traces/nosync.trace | 0
.../libpq/test}/traces/pipeline_abort.trace | 0
.../libpq/test}/traces/prepared.trace | 0
.../libpq/test}/traces/simple_pipeline.trace | 0
.../libpq/test}/traces/singlerow.trace | 0
.../libpq/test}/traces/transaction.trace | 0
src/test/modules/Makefile | 1 -
src/test/modules/libpq_pipeline/.gitignore | 5 ----
src/test/modules/libpq_pipeline/Makefile | 25 -------------------
src/test/modules/libpq_pipeline/README | 1 -
16 files changed, 3 insertions(+), 34 deletions(-)
rename src/{test/modules/libpq_pipeline/t/001_libpq_pipeline.pl => interfaces/libpq/t/002_pipeline.pl} (96%)
rename src/{test/modules/libpq_pipeline => interfaces/libpq/test}/libpq_pipeline.c (100%)
rename src/{test/modules/libpq_pipeline => interfaces/libpq/test}/traces/disallowed_in_pipeline.trace (100%)
rename src/{test/modules/libpq_pipeline => interfaces/libpq/test}/traces/multi_pipelines.trace (100%)
rename src/{test/modules/libpq_pipeline => interfaces/libpq/test}/traces/nosync.trace (100%)
rename src/{test/modules/libpq_pipeline => interfaces/libpq/test}/traces/pipeline_abort.trace (100%)
rename src/{test/modules/libpq_pipeline => interfaces/libpq/test}/traces/prepared.trace (100%)
rename src/{test/modules/libpq_pipeline => interfaces/libpq/test}/traces/simple_pipeline.trace (100%)
rename src/{test/modules/libpq_pipeline => interfaces/libpq/test}/traces/singlerow.trace (100%)
rename src/{test/modules/libpq_pipeline => interfaces/libpq/test}/traces/transaction.trace (100%)
delete mode 100644 src/test/modules/libpq_pipeline/.gitignore
delete mode 100644 src/test/modules/libpq_pipeline/Makefile
delete mode 100644 src/test/modules/libpq_pipeline/README
diff --git a/src/test/modules/libpq_pipeline/t/001_libpq_pipeline.pl b/src/interfaces/libpq/t/002_pipeline.pl
similarity index 96%
rename from src/test/modules/libpq_pipeline/t/001_libpq_pipeline.pl
rename to src/interfaces/libpq/t/002_pipeline.pl
index 0c164dcaba5..2e288d8ba7c 100644
--- a/src/test/modules/libpq_pipeline/t/001_libpq_pipeline.pl
+++ b/src/interfaces/libpq/t/002_pipeline.pl
@@ -49,7 +49,7 @@ for my $testname (@tests)
my $expected;
my $result;
- $expected = slurp_file_eval("traces/$testname.trace");
+ $expected = slurp_file_eval("test/traces/$testname.trace");
next unless $expected ne "";
$result = slurp_file_eval($traceout);
next unless $result ne "";
diff --git a/src/interfaces/libpq/test/.gitignore b/src/interfaces/libpq/test/.gitignore
index 5e803d8816a..e24d7f64dc3 100644
--- a/src/interfaces/libpq/test/.gitignore
+++ b/src/interfaces/libpq/test/.gitignore
@@ -1 +1,2 @@
/uri-regress
+/libpq_pipeline
diff --git a/src/interfaces/libpq/test/Makefile b/src/interfaces/libpq/test/Makefile
index 54212159065..9f99309653f 100644
--- a/src/interfaces/libpq/test/Makefile
+++ b/src/interfaces/libpq/test/Makefile
@@ -11,7 +11,7 @@ endif
override CPPFLAGS := -I$(libpq_srcdir) $(CPPFLAGS)
LDFLAGS_INTERNAL += $(libpq_pgport)
-PROGS = uri-regress
+PROGS = uri-regress libpq_pipeline
all: $(PROGS)
diff --git a/src/test/modules/libpq_pipeline/libpq_pipeline.c b/src/interfaces/libpq/test/libpq_pipeline.c
similarity index 100%
rename from src/test/modules/libpq_pipeline/libpq_pipeline.c
rename to src/interfaces/libpq/test/libpq_pipeline.c
diff --git a/src/test/modules/libpq_pipeline/traces/disallowed_in_pipeline.trace b/src/interfaces/libpq/test/traces/disallowed_in_pipeline.trace
similarity index 100%
rename from src/test/modules/libpq_pipeline/traces/disallowed_in_pipeline.trace
rename to src/interfaces/libpq/test/traces/disallowed_in_pipeline.trace
diff --git a/src/test/modules/libpq_pipeline/traces/multi_pipelines.trace b/src/interfaces/libpq/test/traces/multi_pipelines.trace
similarity index 100%
rename from src/test/modules/libpq_pipeline/traces/multi_pipelines.trace
rename to src/interfaces/libpq/test/traces/multi_pipelines.trace
diff --git a/src/test/modules/libpq_pipeline/traces/nosync.trace b/src/interfaces/libpq/test/traces/nosync.trace
similarity index 100%
rename from src/test/modules/libpq_pipeline/traces/nosync.trace
rename to src/interfaces/libpq/test/traces/nosync.trace
diff --git a/src/test/modules/libpq_pipeline/traces/pipeline_abort.trace b/src/interfaces/libpq/test/traces/pipeline_abort.trace
similarity index 100%
rename from src/test/modules/libpq_pipeline/traces/pipeline_abort.trace
rename to src/interfaces/libpq/test/traces/pipeline_abort.trace
diff --git a/src/test/modules/libpq_pipeline/traces/prepared.trace b/src/interfaces/libpq/test/traces/prepared.trace
similarity index 100%
rename from src/test/modules/libpq_pipeline/traces/prepared.trace
rename to src/interfaces/libpq/test/traces/prepared.trace
diff --git a/src/test/modules/libpq_pipeline/traces/simple_pipeline.trace b/src/interfaces/libpq/test/traces/simple_pipeline.trace
similarity index 100%
rename from src/test/modules/libpq_pipeline/traces/simple_pipeline.trace
rename to src/interfaces/libpq/test/traces/simple_pipeline.trace
diff --git a/src/test/modules/libpq_pipeline/traces/singlerow.trace b/src/interfaces/libpq/test/traces/singlerow.trace
similarity index 100%
rename from src/test/modules/libpq_pipeline/traces/singlerow.trace
rename to src/interfaces/libpq/test/traces/singlerow.trace
diff --git a/src/test/modules/libpq_pipeline/traces/transaction.trace b/src/interfaces/libpq/test/traces/transaction.trace
similarity index 100%
rename from src/test/modules/libpq_pipeline/traces/transaction.trace
rename to src/interfaces/libpq/test/traces/transaction.trace
diff --git a/src/test/modules/Makefile b/src/test/modules/Makefile
index dffc79b2d9a..17f31f45d4e 100644
--- a/src/test/modules/Makefile
+++ b/src/test/modules/Makefile
@@ -10,7 +10,6 @@ SUBDIRS = \
delay_execution \
dummy_index_am \
dummy_seclabel \
- libpq_pipeline \
plsample \
snapshot_too_old \
spgist_name_ops \
diff --git a/src/test/modules/libpq_pipeline/.gitignore b/src/test/modules/libpq_pipeline/.gitignore
deleted file mode 100644
index 3a11e786b83..00000000000
--- a/src/test/modules/libpq_pipeline/.gitignore
+++ /dev/null
@@ -1,5 +0,0 @@
-# Generated subdirectories
-/log/
-/results/
-/tmp_check/
-/libpq_pipeline
diff --git a/src/test/modules/libpq_pipeline/Makefile b/src/test/modules/libpq_pipeline/Makefile
deleted file mode 100644
index 65acc3e997e..00000000000
--- a/src/test/modules/libpq_pipeline/Makefile
+++ /dev/null
@@ -1,25 +0,0 @@
-# src/test/modules/libpq_pipeline/Makefile
-
-PGFILEDESC = "libpq_pipeline - test program for pipeline execution"
-PGAPPICON = win32
-
-PROGRAM = libpq_pipeline
-OBJS = $(WIN32RES) libpq_pipeline.o
-
-NO_INSTALL = 1
-
-PG_CPPFLAGS = -I$(libpq_srcdir)
-PG_LIBS_INTERNAL += $(libpq_pgport)
-
-TAP_TESTS = 1
-
-ifdef USE_PGXS
-PG_CONFIG = pg_config
-PGXS := $(shell $(PG_CONFIG) --pgxs)
-include $(PGXS)
-else
-subdir = src/test/modules/libpq_pipeline
-top_builddir = ../../../..
-include $(top_builddir)/src/Makefile.global
-include $(top_srcdir)/contrib/contrib-global.mk
-endif
diff --git a/src/test/modules/libpq_pipeline/README b/src/test/modules/libpq_pipeline/README
deleted file mode 100644
index d8174dd579a..00000000000
--- a/src/test/modules/libpq_pipeline/README
+++ /dev/null
@@ -1 +0,0 @@
-Test programs and libraries for libpq
--
2.34.0
On 2/24/22 07:19, Andrew Dunstan wrote:
On 2/23/22 20:52, Tom Lane wrote:
Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
On 23.02.22 23:58, Tom Lane wrote:
Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
libpq TAP tests should be in src/interfaces/libpq/t/.
That's failing to account for the fact that a libpq test can't
really be a pure-perl TAP test; you need some C code to drive the
library.Such things could be put under src/interfaces/libpq/test, or some other
subdirectory. We already have src/interfaces/ecpg/test.OK, but then the TAP scripts are under src/interfaces/libpq/test/t,
which isn't what you said. I have no great objection to moving
src/test/modules/libpq_pipeline/ to src/interfaces/libpq/test/,
though, as long as the buildfarm will cope.It won't without some adjustment.
See
<https://github.com/PGBuildFarm/client-code/commit/ffc0fc029877632e9437af51bd99ace308daf0c8>
and
<https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=crake&dt=2022-03-01%2010%3A47%3A22&stg=module-libpq-check>
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
Hi,
On March 1, 2022 7:44:27 AM PST, Andrew Dunstan <andrew@dunslane.net> wrote:
On 2/24/22 07:19, Andrew Dunstan wrote:
On 2/23/22 20:52, Tom Lane wrote:
Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
On 23.02.22 23:58, Tom Lane wrote:
Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
libpq TAP tests should be in src/interfaces/libpq/t/.
That's failing to account for the fact that a libpq test can't
really be a pure-perl TAP test; you need some C code to drive the
library.Such things could be put under src/interfaces/libpq/test, or some other
subdirectory. We already have src/interfaces/ecpg/test.OK, but then the TAP scripts are under src/interfaces/libpq/test/t,
which isn't what you said. I have no great objection to moving
src/test/modules/libpq_pipeline/ to src/interfaces/libpq/test/,
though, as long as the buildfarm will cope.It won't without some adjustment.
See
<https://github.com/PGBuildFarm/client-code/commit/ffc0fc029877632e9437af51bd99ace308daf0c8>
and
<https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=crake&dt=2022-03-01%2010%3A47%3A22&stg=module-libpq-check>
Thanks!
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
On Sat, Feb 26, 2022 at 05:46:26PM -0800, Andres Freund wrote:
Pushed. Attached is the remainder, 0003, the move of libpq_pipeline to
src/interfaces/libpq that I'm not planning to push for now.
I saw that Andrew just pushed something to start building this under MSVC.
In case it's of any interest, I had done this differently a while back.
This probably doesn't apply except on top of some other patches, but you get
the idea.
commit 923f8a1c2cbea35cb01d1599caa2a81e3186181c
Author: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Mon Feb 28 01:31:10 2022 -0600
f!
ci-os-only: windows
diff --git a/src/tools/msvc/Mkvcbuild.pm b/src/tools/msvc/Mkvcbuild.pm
index 4364ab943fd..71ec747e544 100644
--- a/src/tools/msvc/Mkvcbuild.pm
+++ b/src/tools/msvc/Mkvcbuild.pm
@@ -44,6 +44,7 @@ my $contrib_extraincludes = {};
my $contrib_extrasource = {
'uri-regress' => ['src/interfaces/libpq/test/uri-regress.c'],
'testclient' => ['src/interfaces/libpq/test/testclient.c'],
+ 'libpq_pipeline' => ['src/interfaces/libpq/test/libpq_pipeline.c'],
};
my @contrib_excludes = (
'bool_plperl', 'commit_ts',
@@ -475,7 +476,7 @@ sub mkvcbuild
push @contrib_excludes, 'uuid-ossp';
}- foreach my $subdir ('contrib', 'src/test/modules', 'src/interfaces/libpq')
+ foreach my $subdir ('contrib', 'src/test/modules') #, 'src/interfaces/libpq')
{
opendir($D, $subdir) || croak "Could not opendir on $subdir!\n";
while (my $d = readdir($D))
@@ -804,6 +805,20 @@ sub mkvcbuild
$p->AddReference($postgres);
}+ $mf = Project::read_file('src/interfaces/libpq/test/Makefile');
+ $mf =~ s{\\\r?\n}{}g;
+ $mf =~ m{PROGRAMS\s*=\s*(.*)$}m
+ || die 'Could not match in src/interfaces/libpq/test/Makefile' . "\n";
+ foreach my $prg (split /\s+/, $1)
+ {
+ my $proj = $solution->AddProject($prg, 'exe', 'bin');
+ $proj->AddFile("src/interfaces/libpq/test/$prg.c"); # implicit source file
+ $proj->AddIncludeDir('src/interfaces/libpq');
+ # XXX: pipeline needs pgcommon and ws2, but uri-regress doesn't
+ $proj->AddReference($libpq, $libpgport, $libpgcommon);
+ $proj->AddLibrary('ws2_32.lib');
+ }
+
$mf = Project::read_file('src/bin/scripts/Makefile');
$mf =~ s{\\\r?\n}{}g;
$mf =~ m{PROGRAMS\s*=\s*(.*)$}mOn 2022-04-16 Sa 10:44, Justin Pryzby wrote:
On Sat, Feb 26, 2022 at 05:46:26PM -0800, Andres Freund wrote:
Pushed. Attached is the remainder, 0003, the move of libpq_pipeline to
src/interfaces/libpq that I'm not planning to push for now.I saw that Andrew just pushed something to start building this under MSVC.
In case it's of any interest, I had done this differently a while back.
This probably doesn't apply except on top of some other patches, but you get
the idea.
I think what I have committed should be quite adequate for now. Once we
get to building with meson a lot of this ugliness should go away.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
On Sat, Feb 26, 2022 at 05:46:26PM -0800, Andres Freund wrote:
On 2022-02-25 17:52:29 -0800, Andres Freund wrote:
I'd like to commit 0001 and 0002 soon, unless somebody sees a reason not to?
Pushed.
If I'm not wrong, this isn't being run by check-world.
commit 4dc465207517c4b69a1f2b657a8ad0700c08e34c
Author: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Sat May 28 22:32:58 2022 -0500
libpq tests were not being run
See also:
ac25173cdbc40b310a7e72d9557c45a699f1f7b3
6b04abdfc5e0653542ac5d586e639185a8c61a39
diff --git a/GNUmakefile.in b/GNUmakefile.in
index 2352fc1171a..cb613086c7c 100644
--- a/GNUmakefile.in
+++ b/GNUmakefile.in
@@ -68,10 +68,10 @@ check check-tests installcheck installcheck-parallel installcheck-tests: CHECKPR
check check-tests installcheck installcheck-parallel installcheck-tests: submake-generated-headers
$(MAKE) -C src/test/regress $@-$(call recurse,check-world,src/test src/pl src/interfaces/ecpg contrib src/bin,check)
-$(call recurse,checkprep, src/test src/pl src/interfaces/ecpg contrib src/bin)
+$(call recurse,check-world,src/test src/pl src/interfaces/ecpg src/interfaces/libpq contrib src/bin,check)
+$(call recurse,checkprep, src/test src/pl src/interfaces/ecpg src/interfaces/libpq contrib src/bin)-$(call recurse,installcheck-world,src/test src/pl src/interfaces/ecpg contrib src/bin,installcheck)
+$(call recurse,installcheck-world,src/test src/pl src/interfaces/ecpg src/interfaces/libpq contrib src/bin,installcheck)
$(call recurse,install-tests,src/test/regress,install-tests)GNUmakefile: GNUmakefile.in $(top_builddir)/config.status
On Sun, May 29, 2022 at 10:18:50AM -0500, Justin Pryzby wrote:
On Sat, Feb 26, 2022 at 05:46:26PM -0800, Andres Freund wrote:
On 2022-02-25 17:52:29 -0800, Andres Freund wrote:
I'd like to commit 0001 and 0002 soon, unless somebody sees a reason not to?
Pushed.
If I'm not wrong, this isn't being run by check-world.
You are right.
-$(call recurse,check-world,src/test src/pl src/interfaces/ecpg contrib src/bin,check) -$(call recurse,checkprep, src/test src/pl src/interfaces/ecpg contrib src/bin) +$(call recurse,check-world,src/test src/pl src/interfaces/ecpg src/interfaces/libpq contrib src/bin,check) +$(call recurse,checkprep, src/test src/pl src/interfaces/ecpg src/interfaces/libpq contrib src/bin)-$(call recurse,installcheck-world,src/test src/pl src/interfaces/ecpg contrib src/bin,installcheck) +$(call recurse,installcheck-world,src/test src/pl src/interfaces/ecpg src/interfaces/libpq contrib src/bin,installcheck) $(call recurse,install-tests,src/test/regress,install-tests)
Why don't you just use src/interfaces/ instead of adding a direct
path to libpq?
--
Michael
On Tue, May 31, 2022 at 01:58:25PM +0900, Michael Paquier wrote:
Why don't you just use src/interfaces/ instead of adding a direct
path to libpq?
So, this leads to something like the attached. Does that sound fine
to you?
--
Michael
Attachments:
v2-0001-libpq-tests-were-not-being-run.patchtext/x-diff; charset=us-asciiDownload
From 893ef90ce07084c4e4837205a805c590798ac115 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Wed, 1 Jun 2022 13:57:46 +0900
Subject: [PATCH v2] libpq tests were not being run
---
GNUmakefile.in | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/GNUmakefile.in b/GNUmakefile.in
index 2352fc1171..38713b5d12 100644
--- a/GNUmakefile.in
+++ b/GNUmakefile.in
@@ -68,10 +68,10 @@ check check-tests installcheck installcheck-parallel installcheck-tests: CHECKPR
check check-tests installcheck installcheck-parallel installcheck-tests: submake-generated-headers
$(MAKE) -C src/test/regress $@
-$(call recurse,check-world,src/test src/pl src/interfaces/ecpg contrib src/bin,check)
-$(call recurse,checkprep, src/test src/pl src/interfaces/ecpg contrib src/bin)
+$(call recurse,check-world,src/test src/pl src/interfaces contrib src/bin,check)
+$(call recurse,checkprep, src/test src/pl src/interfaces contrib src/bin)
-$(call recurse,installcheck-world,src/test src/pl src/interfaces/ecpg contrib src/bin,installcheck)
+$(call recurse,installcheck-world,src/test src/pl src/interfaces contrib src/bin,installcheck)
$(call recurse,install-tests,src/test/regress,install-tests)
GNUmakefile: GNUmakefile.in $(top_builddir)/config.status
--
2.36.1
Hi,
On 2022-05-29 10:18:50 -0500, Justin Pryzby wrote:
On Sat, Feb 26, 2022 at 05:46:26PM -0800, Andres Freund wrote:
On 2022-02-25 17:52:29 -0800, Andres Freund wrote:
I'd like to commit 0001 and 0002 soon, unless somebody sees a reason not to?
Pushed.
If I'm not wrong, this isn't being run by check-world.
Oops, yes. Thanks for catching!
Hi,
On 2022-06-01 13:59:06 +0900, Michael Paquier wrote:
On Tue, May 31, 2022 at 01:58:25PM +0900, Michael Paquier wrote:
Why don't you just use src/interfaces/ instead of adding a direct
path to libpq?So, this leads to something like the attached. Does that sound fine
to you?
That looks reasonable to me. Do you want to apply it or will you?
Regards,
Andres
On Thu, Jun 02, 2022 at 09:48:25AM -0700, Andres Freund wrote:
On 2022-06-01 13:59:06 +0900, Michael Paquier wrote:
So, this leads to something like the attached. Does that sound fine
to you?That looks reasonable to me. Do you want to apply it or will you?
Thanks for double-checking! I should be able to take care of that
today.
--
Michael