Thread: plperl and pltcl installcheck targets
The attached patch creates installcheck targets for plperl and pltcl
(plpython laready has one). This will help in getting buildfarm to test PLs.
Is it worth rearranging things for plpython so that it follows the same
test layout as the other 2 (i.e. a test subdir with all the test files
and a script called runtest that does the work)? Especially if we bring
in other PLs as has been discussed, some standard might be useful. Maybe
a README in src/pl ?
I'm not sure that the tests currently provide very good coverage
(especially plperl) but getting them run automatically might provide
some extra incentive on that.
cheers
andrew
Index: plperl/GNUmakefile
===================================================================
RCS file: /home/cvsmirror/pgsql/src/pl/plperl/GNUmakefile,v
retrieving revision 1.18
diff -c -r1.18 GNUmakefile
*** plperl/GNUmakefile 19 Nov 2004 19:22:58 -0000 1.18
--- plperl/GNUmakefile 11 May 2005 14:02:27 -0000
***************
*** 53,58 ****
--- 53,62 ----
echo "*****"
endif
+ installcheck:
+ cd $(srcdir)/test && PATH=$(bindir):$$PATH $(SHELL) runtest
+
+
installdirs:
$(mkinstalldirs) $(DESTDIR)$(pkglibdir)
Index: tcl/Makefile
===================================================================
RCS file: /home/cvsmirror/pgsql/src/pl/tcl/Makefile,v
retrieving revision 1.44
diff -c -r1.44 Makefile
*** tcl/Makefile 16 Dec 2004 20:41:01 -0000 1.44
--- tcl/Makefile 11 May 2005 14:25:53 -0000
***************
*** 57,62 ****
--- 57,66 ----
endif
$(MAKE) -C modules $@
+ installcheck:
+ cd $(srcdir)/test && PATH=$(bindir):$$PATH $(SHELL) runtest
+
+
installdirs:
$(mkinstalldirs) $(DESTDIR)$(pkglibdir)
$(MAKE) -C modules $@
Andrew Dunstan <andrew@dunslane.net> writes:
> Is it worth rearranging things for plpython so that it follows the same
> test layout as the other 2 (i.e. a test subdir with all the test files
> and a script called runtest that does the work)? Especially if we bring
> in other PLs as has been discussed, some standard might be useful.
Actually, we have a standard: it's pg_regress. The right thing for
someone to do is migrate all these tests into the form already used
for the main backend and all of contrib.
I think this would require a small addition to the pg_regress script
to make it configurable as to which PL to install, instead of always
installing plpgsql, but that seems like a reasonable thing to do.
regards, tom lane
Tom Lane wrote: >Andrew Dunstan <andrew@dunslane.net> writes: > > >>Aha. ok. should be fairly trivial. I'm thinking of something like >> --load-languages=lang1,lang2,lang3 >>(in case we ever want more than one). >> >> > >Might be a little easier as multiple switches: > --load-language=lang1 --load-language=lang2 > > > > Ok. Here's a patch for that piece. With this, contrib regression tests don't load plpgsql, but standard core tests do. cheers andrew
Andrew Dunstan wrote:
>
>
> Tom Lane wrote:
>
>> Andrew Dunstan <andrew@dunslane.net> writes:
>>
>>
>>> Aha. ok. should be fairly trivial. I'm thinking of something like
>>> --load-languages=lang1,lang2,lang3
>>> (in case we ever want more than one).
>>>
>>
>>
>> Might be a little easier as multiple switches:
>> --load-language=lang1 --load-language=lang2
>>
>>
>>
>>
>
> Ok. Here's a patch for that piece. With this, contrib regression
> tests don't load plpgsql, but standard core tests do.
>
>
er this time with a patch attached.
cheers
andrew
Index: GNUmakefile
===================================================================
RCS file: /home/cvsmirror/pgsql/src/test/regress/GNUmakefile,v
retrieving revision 1.48
diff -c -r1.48 GNUmakefile
*** GNUmakefile 17 Nov 2004 18:05:06 -0000 1.48
--- GNUmakefile 11 May 2005 20:03:56 -0000
***************
*** 130,146 ****
check: all
-rm -rf ./testtablespace
mkdir ./testtablespace
! $(SHELL) ./pg_regress --temp-install --top-builddir=$(top_builddir) --schedule=$(srcdir)/parallel_schedule
--multibyte=$(MULTIBYTE)$(MAXCONNOPT)
installcheck: all
-rm -rf ./testtablespace
mkdir ./testtablespace
! $(SHELL) ./pg_regress --schedule=$(srcdir)/serial_schedule --multibyte=$(MULTIBYTE)
installcheck-parallel: all
-rm -rf ./testtablespace
mkdir ./testtablespace
! $(SHELL) ./pg_regress --schedule=$(srcdir)/parallel_schedule --multibyte=$(MULTIBYTE) $(MAXCONNOPT)
# old interfaces follow...
--- 130,146 ----
check: all
-rm -rf ./testtablespace
mkdir ./testtablespace
! $(SHELL) ./pg_regress --temp-install --top-builddir=$(top_builddir) --schedule=$(srcdir)/parallel_schedule
--multibyte=$(MULTIBYTE)--load-language=plpgsql $(MAXCONNOPT)
installcheck: all
-rm -rf ./testtablespace
mkdir ./testtablespace
! $(SHELL) ./pg_regress --schedule=$(srcdir)/serial_schedule --multibyte=$(MULTIBYTE) --load-language=plpgsql
installcheck-parallel: all
-rm -rf ./testtablespace
mkdir ./testtablespace
! $(SHELL) ./pg_regress --schedule=$(srcdir)/parallel_schedule --multibyte=$(MULTIBYTE) --load-language=plpgsql
$(MAXCONNOPT)
# old interfaces follow...
***************
*** 150,159 ****
runtest-parallel: installcheck-parallel
bigtest:
! $(SHELL) ./pg_regress --schedule=$(srcdir)/serial_schedule --multibyte=$(MULTIBYTE) numeric_big
bigcheck:
! $(SHELL) ./pg_regress --temp-install --top-builddir=$(top_builddir) --schedule=$(srcdir)/parallel_schedule
--multibyte=$(MULTIBYTE)$(MAXCONNOPT) numeric_big
##
--- 150,159 ----
runtest-parallel: installcheck-parallel
bigtest:
! $(SHELL) ./pg_regress --schedule=$(srcdir)/serial_schedule --multibyte=$(MULTIBYTE) --load-language=plpgsql
numeric_big
bigcheck:
! $(SHELL) ./pg_regress --temp-install --top-builddir=$(top_builddir) --schedule=$(srcdir)/parallel_schedule
--multibyte=$(MULTIBYTE) --load-language=plpgsql $(MAXCONNOPT) numeric_big
##
Index: pg_regress.sh
===================================================================
RCS file: /home/cvsmirror/pgsql/src/test/regress/pg_regress.sh,v
retrieving revision 1.53
diff -c -r1.53 pg_regress.sh
*** pg_regress.sh 15 Jan 2005 04:15:51 -0000 1.53
--- pg_regress.sh 11 May 2005 20:15:35 -0000
***************
*** 13,18 ****
--- 13,20 ----
Options:
--debug turn on debug mode in programs that are run
--inputdir=DIR take input files from DIR (default \`.')
+ --load-language=lang load the named language before running the
+ tests; can appear multiple times
--max-connections=N maximum number of concurrent connections
(default is 0 meaning unlimited)
--multibyte=ENCODING use ENCODING as the multibyte encoding, and
***************
*** 103,108 ****
--- 105,111 ----
dbname=regression
hostname=localhost
maxconnections=0
+ load_langs=""
: ${GMAKE='@GMAKE@'}
***************
*** 126,131 ****
--- 129,139 ----
--inputdir=*)
inputdir=`expr "x$1" : "x--inputdir=\(.*\)"`
shift;;
+ --load-language=*)
+ lang=`expr "x$1" : "x--load-language=\(.*\)"`
+ load_langs="$load_langs $lang"
+ unset lang
+ shift;;
--multibyte=*)
multibyte=`expr "x$1" : "x--multibyte=\(.*\)"`
shift;;
***************
*** 564,575 ****
# ----------
if [ "$enable_shared" = yes ]; then
! message "installing PL/pgSQL"
! "$bindir/createlang" -L "$pkglibdir" $psql_options plpgsql $dbname
if [ $? -ne 0 ] && [ $? -ne 2 ]; then
echo "$me: createlang failed"
(exit 2); exit
fi
fi
--- 572,586 ----
# ----------
if [ "$enable_shared" = yes ]; then
! for lang in x $load_langs ; do
! test $lang = x && continue
! message "installing $lang"
! "$bindir/createlang" -L "$pkglibdir" $psql_options $lang $dbname
if [ $? -ne 0 ] && [ $? -ne 2 ]; then
echo "$me: createlang failed"
(exit 2); exit
fi
+ done
fi
Andrew Dunstan <andrew@dunslane.net> writes:
>> Ok. Here's a patch for that piece. With this, contrib regression
>> tests don't load plpgsql, but standard core tests do.
> er this time with a patch attached.
Looks good to me, will apply shortly.
Are you planning to take a whack at fixing the PL tests to use this?
regards, tom lane
Tom Lane wrote: >Andrew Dunstan <andrew@dunslane.net> writes: > > >>>Ok. Here's a patch for that piece. With this, contrib regression >>>tests don't load plpgsql, but standard core tests do. >>> >>> > > > >>er this time with a patch attached. >> >> > >Looks good to me, will apply shortly. > >Are you planning to take a whack at fixing the PL tests to use this? > > > > Unless someone gets there before me. My goal is to have buildfarm testing PLs by the time I leave for my trip to aus on the 27th. cheers andrew
Andrew Dunstan <andrew@dunslane.net> writes:
> Is that what you had in mind?
Not entirely. We should move around the test sql script and the expected
result file so that the file structure looks exactly like one of the
test-enabled contrib modules. I realize that's a PITA to describe as a
patch --- just tell me in words what to put where (ls -lR maybe) and
it shall be so.
regards, tom lane
Tom Lane wrote: >Andrew Dunstan <andrew@dunslane.net> writes: > > >>Is that what you had in mind? >> >> > >Not entirely. We should move around the test sql script and the expected >result file so that the file structure looks exactly like one of the >test-enabled contrib modules. > That's pretty much what I thought I did except that I put it all in the test subdir. Frankly, I think that's a better arrangement - having cube/cube.sql and cube/sql/cube.sql is confusing - putting test stuff in a test subdirectory makes its purpose clear. I guess instead of changing to that directory we could pass it as the inputdir and outputdir options to pg_regress - that would probably be cleaner. >I realize that's a PITA to describe as a >patch --- just tell me in words what to put where (ls -lR maybe) and >it shall be so. > > > > ok, when we know we're both on the same sheet of music I'll do that. cheers andrew
Andrew Dunstan <andrew@dunslane.net> writes:
> Tom Lane wrote:
>> Not entirely. We should move around the test sql script and the expected
>> result file so that the file structure looks exactly like one of the
>> test-enabled contrib modules.
> That's pretty much what I thought I did except that I put it all in the
> test subdir. Frankly, I think that's a better arrangement - having
> cube/cube.sql and cube/sql/cube.sql is confusing - putting test stuff in
> a test subdirectory makes its purpose clear.
[ shrug... ] I don't see a good argument for making the PLs have a
directory layout that is different from the one in use in contrib.
If you want to argue that all of the contrib subdirectories should
be changed, bring it up in -hackers; but it is hard to get excited
about.
regards, tom lane
Andrew Dunstan <andrew@dunslane.net> writes:
> OK, here's what I have. The tgz file contains the sql and expected
> directories for the 3 PLs, plus there's a patch for the makefiles.
Applied with minor editorialization to make it more like the contrib
installcheck process.
I added a "make installcheck" target in the src/pl directory, so that
you can just fire that one make instead of having to check which PLs
are actually configured.
It occurs to me that maybe we should just add src/pl to the toplevel
installcheck target, which would make the whole thing pretty
automatic. However, contrib isn't in that target, so maybe the PLs
shouldn't be either. Thoughts?
regards, tom lane
Tom Lane wrote: > It occurs to me that maybe we should just add src/pl to the toplevel > installcheck target, which would make the whole thing pretty > automatic. However, contrib isn't in that target, so maybe the PLs > shouldn't be either. Thoughts? Contrib isn't built by default, so it isn't tested by default. But the PLs that are built should also be tested, I think. -- Peter Eisentraut http://developer.postgresql.org/~petere/
Peter Eisentraut wrote: >Tom Lane wrote: > > >>It occurs to me that maybe we should just add src/pl to the toplevel >>installcheck target, which would make the whole thing pretty >>automatic. However, contrib isn't in that target, so maybe the PLs >>shouldn't be either. Thoughts? >> >> > >Contrib isn't built by default, so it isn't tested by default. But the >PLs that are built should also be tested, I think. > > > I agree. That will also mean that buildfarm members will automatically start doing the checks (as long as they are set up to build the PLs), so it would be an extra bonus for me :-) cheers andrew
Andrew Dunstan <andrew@dunslane.net> writes:
> I agree. That will also mean that buildfarm members will automatically
> start doing the checks (as long as they are set up to build the PLs), so
> it would be an extra bonus for me :-)
The only argument I can think of against it is that the buildfarm
status page will then not distinguish core system check failures
from PL check failures. If that doesn't bother you, we can make
it so.
regards, tom lane
Tom Lane wrote: >Andrew Dunstan <andrew@dunslane.net> writes: > > >>I agree. That will also mean that buildfarm members will automatically >>start doing the checks (as long as they are set up to build the PLs), so >>it would be an extra bonus for me :-) >> >> > >The only argument I can think of against it is that the buildfarm >status page will then not distinguish core system check failures >from PL check failures. If that doesn't bother you, we can make >it so. > > > > That's probably a good point. I was just being a little too lazy. Building an extra check step in is not very difficult - it will just take a little while for all the buildfarm emebers to catch up. But since we're still 6 weeks even from feature freeze that shouldn't matter too much. I'll get onto it. cheers andrew