Thread: libpq URI and regression testing
Hi, When I committed Alex Shulgin's patch to add URI support to libpq, I included the test harness as well. However, due to it being in a separate subdirectory that did not previously had tests, it's not being run by buildfarm. It's not considered in "make installcheck-world" either. What's the preferred way to make it automatically tested as much as possible? I know the buildfarm does not run "installcheck-world", so if we want it there, it'd need a bit more code on the client side. I think it would be wise to have it also run on installcheck-world. Hmm. Just had this thought: not all platform support the same socket types. Maybe we should have separated the .in file in three parts: IPv4, IPv6, unix-domain socket. That way each platform would only run tests that pertain to it. Right now there's a single "regress.in" file that lists all the tests. Opinions? -- Álvaro Herrera <alvherre@alvh.no-ip.org>
On tis, 2012-04-17 at 10:47 -0300, Alvaro Herrera wrote: > What's the preferred way to make it automatically tested as much as > possible? I know the buildfarm does not run "installcheck-world", so if > we want it there, it'd need a bit more code on the client side. I think > it would be wise to have it also run on installcheck-world. It was agreed during the patch discussion that it shouldn't be run automatically. > Hmm. Just had this thought: not all platform support the same socket > types. Maybe we should have separated the .in file in three parts: > IPv4, IPv6, unix-domain socket. That way each platform would only run > tests that pertain to it. Right now there's a single "regress.in" file > that lists all the tests. That's one reason for that, but there are probably others in the way of making this fully portable and automatable.
Excerpts from Peter Eisentraut's message of mar abr 17 15:41:04 -0300 2012: > On tis, 2012-04-17 at 10:47 -0300, Alvaro Herrera wrote: > > What's the preferred way to make it automatically tested as much as > > possible? I know the buildfarm does not run "installcheck-world", so if > > we want it there, it'd need a bit more code on the client side. I think > > it would be wise to have it also run on installcheck-world. > > It was agreed during the patch discussion that it shouldn't be run > automatically. Oh, okay. I didn't notice that. I guess we should issue a call-for-testing, then, so that we ensure it works (FSVO works) in all (FSVO all) platforms. > > Hmm. Just had this thought: not all platform support the same socket > > types. Maybe we should have separated the .in file in three parts: > > IPv4, IPv6, unix-domain socket. That way each platform would only run > > tests that pertain to it. Right now there's a single "regress.in" file > > that lists all the tests. > > That's one reason for that, but there are probably others in the way of > making this fully portable and automatable. Hmm. -- Álvaro Herrera <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support
On 04/17/2012 02:47 PM, Alvaro Herrera wrote: > Excerpts from Peter Eisentraut's message of mar abr 17 15:41:04 -0300 2012: >> On tis, 2012-04-17 at 10:47 -0300, Alvaro Herrera wrote: >>> What's the preferred way to make it automatically tested as much as >>> possible? I know the buildfarm does not run "installcheck-world", so if >>> we want it there, it'd need a bit more code on the client side. I think >>> it would be wise to have it also run on installcheck-world. >> It was agreed during the patch discussion that it shouldn't be run >> automatically. > Oh, okay. I didn't notice that. I guess we should issue a > call-for-testing, then, so that we ensure it works (FSVO works) in all > (FSVO all) platforms. > >>> Hmm. Just had this thought: not all platform support the same socket >>> types. Maybe we should have separated the .in file in three parts: >>> IPv4, IPv6, unix-domain socket. That way each platform would only run >>> tests that pertain to it. Right now there's a single "regress.in" file >>> that lists all the tests. >> That's one reason for that, but there are probably others in the way of >> making this fully portable and automatable. This test setup also appears to labor under the illusion that we live in a Unix-only world. And for no good reason that I can tell. The shell script should be ripped out and replaced by a perl script which could actually be used on any windows build host. The MSVC build system also needs adjusting to make it build the test driver, at least. cheers andrew
Excerpts from Andrew Dunstan's message of mar abr 17 16:03:50 -0300 2012: > >> That's one reason for that, but there are probably others in the way of > >> making this fully portable and automatable. > > This test setup also appears to labor under the illusion that we live in > a Unix-only world. And for no good reason that I can tell. The shell > script should be ripped out and replaced by a perl script which could > actually be used on any windows build host. The MSVC build system also > needs adjusting to make it build the test driver, at least. I'll see about it. -- Álvaro Herrera <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Peter Eisentraut <peter_e@gmx.net> writes: > On tis, 2012-04-17 at 10:47 -0300, Alvaro Herrera wrote: >> What's the preferred way to make it automatically tested as much as >> possible? I know the buildfarm does not run "installcheck-world", so if >> we want it there, it'd need a bit more code on the client side. I think >> it would be wise to have it also run on installcheck-world. > > It was agreed during the patch discussion that it shouldn't be run > automatically. Hm... the testing program we've actually committed avoids any connection attempts and only deals with testing the parser routine. Does it change that? -- Regards, Alex
Andrew Dunstan <andrew@dunslane.net> writes: >>> That's one reason for that, but there are probably others in the way of >>> making this fully portable and automatable. > > This test setup also appears to labor under the illusion that we live > in a Unix-only world. And for no good reason that I can tell. The > shell script should be ripped out and replaced by a perl script which > could actually be used on any windows build host. The MSVC build > system also needs adjusting to make it build the test driver, at > least. Good catch. Attached is my first take at writing Perl: replaces the shell script, adds $libpq_uri_regress project to Mkvcbuild.pm. I don't have access to a win32 box unfortunately, so if anyone who does could try this out that'd be great. -- Regards, Alex diff --git a/src/interfaces/libpq/test/Makefile b/src/interfaces/libpq/test/Makefile index b9023c3..f569fc2 100644 --- a/src/interfaces/libpq/test/Makefile +++ b/src/interfaces/libpq/test/Makefile @@ -15,7 +15,7 @@ all: $(PROGS) installcheck: all SRCDIR='$(top_srcdir)' SUBDIR='$(subdir)' \ - $(SHELL) $(top_srcdir)/$(subdir)/regress.sh + $(top_srcdir)/$(subdir)/regress.pl clean distclean maintainer-clean: rm -f $(PROGS) diff --git a/src/interfaces/libpq/test/regress.pl b/src/interfaces/libpq/test/regress.pl new file mode 100755 index 0000000..74a877a --- /dev/null +++ b/src/interfaces/libpq/test/regress.pl @@ -0,0 +1,56 @@ +#!/usr/bin/env perl +use strict; + +# 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(REGRESS_IN, "<$regress_in") or die "Can't open <$regress_in: $!"; + +# save STDOUT/ERR and redirect both to regress.out +open(OLDOUT, ">&STDOUT") or die "Can't dup STDOUT: $!"; +open(OLDERR, ">&STDERR") or die "Can't dup STDERR: $!"; + +open(STDOUT, ">$regress_out") or die "Can't open >$regress_out: $!"; +open(STDERR, ">&STDOUT") or die "Can't dup STDOUT: $!"; + +# read lines from regress.in and run uri-regress on them +while (<REGRESS_IN>) { + chomp; + print "trying $_\n"; + system("./uri-regress \"$_\""); + print "\n"; +} + +# restore STDOUT/ERR so we can print the outcome to the user +open(STDERR, ">&OLDERR") or die; # can't complain as STDERR is still duped +open(STDOUT, ">&OLDOUT") or die "Can't restore STDOUT: $!"; + +# just in case +close REGRESS_IN; + +my $diff_status = system("diff -c \"$srcdir/$subdir/expected.out\" regress.out >regress.diff"); +if ($diff_status == 0) { + print <<EOF; +====================================================================== +All tests passed +EOF + 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/regress.sh b/src/interfaces/libpq/test/regress.sh deleted file mode 100644 index 298d8bd..0000000 --- a/src/interfaces/libpq/test/regress.sh +++ /dev/null @@ -1,21 +0,0 @@ -#!/bin/sh - -while read line -do - echo "trying $line" - ./uri-regress "$line" - echo "" -done < "${SRCDIR}/${SUBDIR}"/regress.in >regress.out 2>&1 - -if diff -c "${SRCDIR}/${SUBDIR}/"expected.out regress.out >regress.diff; then - echo "========================================" - echo "All tests passed" - exit 0 -else - echo "========================================" - echo "FAILED: the test result differs from the expected output" - echo - echo "Review the difference in ${SUBDIR}/regress.diff" - echo "========================================" - exit 1 -fi diff --git a/src/tools/msvc/Mkvcbuild.pm b/src/tools/msvc/Mkvcbuild.pm index f0fad43..e65971c 100644 --- a/src/tools/msvc/Mkvcbuild.pm +++ b/src/tools/msvc/Mkvcbuild.pm @@ -229,6 +229,15 @@ sub mkvcbuild $libpq->ReplaceFile('src\interfaces\libpq\libpqrc.c','src\interfaces\libpq\libpq.rc'); $libpq->AddReference($libpgport); + my $libpq_uri_regress = $solution->AddProject('libpq_uri_regress','exe','misc'); + $libpq_uri_regress->AddFile('src\interfaces\libpq\test\uri-regress.c'); + $libpq_uri_regress->AddIncludeDir('src\port'); + $libpq_uri_regress->AddIncludeDir('src\interfaces\libpq'); + $libpq_uri_regress->AddLibrary('wsock32.lib'); + $libpq_uri_regress->AddDefine('HOST_TUPLE="i686-pc-win32vc"'); + $libpq_uri_regress->AddDefine('FRONTEND'); + $libpq_uri_regress->AddReference($libpq,$libpgport); + my $libpqwalreceiver = $solution->AddProject('libpqwalreceiver', 'dll', '', 'src\backend\replication\libpqwalreceiver'); $libpqwalreceiver->AddIncludeDir('src\interfaces\libpq');
On tor, 2012-04-19 at 00:13 +0300, Alex wrote: > +#!/usr/bin/env perl Don't do that. Call the script using $(PERL) from the makefile.
Peter Eisentraut <peter_e@gmx.net> writes: > On tor, 2012-04-19 at 00:13 +0300, Alex wrote: >> +#!/usr/bin/env perl > > Don't do that. Call the script using $(PERL) from the makefile. Thank you for the suggestion. Attached v2 does just this (while keeping a more commonly found shebang line in the perl script for running it w/o the makefile.) I figure src/tools/msvc/vcregress.pl will need to be updated too, but trying to model all this after ecpg regression tests I'm stuck at replicating ecpg_regression.proj for libpq's uri-regress. I'd appreciate any help from the Windows guys at this point. -- Alex diff --git a/src/interfaces/libpq/test/Makefile b/src/interfaces/libpq/test/Makefile index b9023c3..048f092 100644 --- a/src/interfaces/libpq/test/Makefile +++ b/src/interfaces/libpq/test/Makefile @@ -15,7 +15,7 @@ all: $(PROGS) installcheck: all SRCDIR='$(top_srcdir)' SUBDIR='$(subdir)' \ - $(SHELL) $(top_srcdir)/$(subdir)/regress.sh + $(PERL) $(top_srcdir)/$(subdir)/regress.pl clean distclean maintainer-clean: rm -f $(PROGS) diff --git a/src/interfaces/libpq/test/regress.pl b/src/interfaces/libpq/test/regress.pl new file mode 100755 index 0000000..a19f793 --- /dev/null +++ b/src/interfaces/libpq/test/regress.pl @@ -0,0 +1,56 @@ +#!/usr/bin/perl +use strict; + +# 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(REGRESS_IN, "<$regress_in") or die "Can't open <$regress_in: $!"; + +# save STDOUT/ERR and redirect both to regress.out +open(OLDOUT, ">&STDOUT") or die "Can't dup STDOUT: $!"; +open(OLDERR, ">&STDERR") or die "Can't dup STDERR: $!"; + +open(STDOUT, ">$regress_out") or die "Can't open >$regress_out: $!"; +open(STDERR, ">&STDOUT") or die "Can't dup STDOUT: $!"; + +# read lines from regress.in and run uri-regress on them +while (<REGRESS_IN>) { + chomp; + print "trying $_\n"; + system("./uri-regress \"$_\""); + print "\n"; +} + +# restore STDOUT/ERR so we can print the outcome to the user +open(STDERR, ">&OLDERR") or die; # can't complain as STDERR is still duped +open(STDOUT, ">&OLDOUT") or die "Can't restore STDOUT: $!"; + +# just in case +close REGRESS_IN; + +my $diff_status = system("diff -c \"$srcdir/$subdir/expected.out\" regress.out >regress.diff"); +if ($diff_status == 0) { + print <<EOF; +====================================================================== +All tests passed +EOF + 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/regress.sh b/src/interfaces/libpq/test/regress.sh deleted file mode 100644 index 298d8bd..0000000 --- a/src/interfaces/libpq/test/regress.sh +++ /dev/null @@ -1,21 +0,0 @@ -#!/bin/sh - -while read line -do - echo "trying $line" - ./uri-regress "$line" - echo "" -done < "${SRCDIR}/${SUBDIR}"/regress.in >regress.out 2>&1 - -if diff -c "${SRCDIR}/${SUBDIR}/"expected.out regress.out >regress.diff; then - echo "========================================" - echo "All tests passed" - exit 0 -else - echo "========================================" - echo "FAILED: the test result differs from the expected output" - echo - echo "Review the difference in ${SUBDIR}/regress.diff" - echo "========================================" - exit 1 -fi diff --git a/src/tools/msvc/Mkvcbuild.pm b/src/tools/msvc/Mkvcbuild.pm index f0fad43..e65971c 100644 --- a/src/tools/msvc/Mkvcbuild.pm +++ b/src/tools/msvc/Mkvcbuild.pm @@ -229,6 +229,15 @@ sub mkvcbuild $libpq->ReplaceFile('src\interfaces\libpq\libpqrc.c','src\interfaces\libpq\libpq.rc'); $libpq->AddReference($libpgport); + my $libpq_uri_regress = $solution->AddProject('libpq_uri_regress','exe','misc'); + $libpq_uri_regress->AddFile('src\interfaces\libpq\test\uri-regress.c'); + $libpq_uri_regress->AddIncludeDir('src\port'); + $libpq_uri_regress->AddIncludeDir('src\interfaces\libpq'); + $libpq_uri_regress->AddLibrary('wsock32.lib'); + $libpq_uri_regress->AddDefine('HOST_TUPLE="i686-pc-win32vc"'); + $libpq_uri_regress->AddDefine('FRONTEND'); + $libpq_uri_regress->AddReference($libpq,$libpgport); + my $libpqwalreceiver = $solution->AddProject('libpqwalreceiver', 'dll', '', 'src\backend\replication\libpqwalreceiver'); $libpqwalreceiver->AddIncludeDir('src\interfaces\libpq');
Excerpts from Alex's message of jue abr 19 17:06:05 -0300 2012: > Peter Eisentraut <peter_e@gmx.net> writes: > > > On tor, 2012-04-19 at 00:13 +0300, Alex wrote: > >> +#!/usr/bin/env perl > > > > Don't do that. Call the script using $(PERL) from the makefile. > > Thank you for the suggestion. Attached v2 does just this (while keeping > a more commonly found shebang line in the perl script for running it w/o > the makefile.) I've applied this to 9.3. Andrew, can we have the non-MSVC buildfarm members running on the master branch also run "make installcheck" in src/interfaces/libpq? If there are platform dependencies here, it would be good to know what they are; if we don't have the tests run automatically we will never know. We'll need to figure out some way for MSVC animals to run the tests as well. Any takers? I think this is necessary, because otherwise it is quite clear that the tests are going to be kept failing forever. They were already failing because of message style changes that didn't update the expected output. (I am unsure about pushing the sh to perl test harness conversion in 9.2. If anybody thinks it should be done, please discuss.) -- Álvaro Herrera <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support