plperl and pltcl installcheck targets

Started by Andrew Dunstanover 20 years ago18 messages
#1Andrew Dunstan
andrew@dunslane.net
1 attachment(s)

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

Attachments:

pltest.patchtext/x-patch; name=pltest.patchDownload
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 $@
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#1)
Re: plperl and pltcl installcheck targets

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

#3Andrew Dunstan
andrew@dunslane.net
In reply to: Tom Lane (#2)
Re: [PATCHES] plperl and pltcl installcheck targets

[redirected to -hackers]

Tom Lane wrote:

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.

I'm not sure why it would matter having it there. I would just make the
first test to load the language in question - pretty much this, right?

CREATE FUNCTION "plperl_call_handler" () RETURNS language_handler AS
'$libdir/plperl' LANGUAGE C;
CREATE TRUSTED LANGUAGE "plperl" HANDLER "plperl_call_handler";
CREATE LANGUAGE "plperlu" HANDLER "plperl_call_handler";

cheers

andrew

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#3)
Re: [PATCHES] plperl and pltcl installcheck targets

Andrew Dunstan <andrew@dunslane.net> writes:

Tom Lane wrote:

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.

I'm not sure why it would matter having it there. I would just make the
first test to load the language in question - pretty much this, right?

CREATE FUNCTION "plperl_call_handler" () RETURNS language_handler AS
'$libdir/plperl' LANGUAGE C;
CREATE TRUSTED LANGUAGE "plperl" HANDLER "plperl_call_handler";
CREATE LANGUAGE "plperlu" HANDLER "plperl_call_handler";

The point is that I'd rather test createlang than duplicate it.

(In the back of my mind also is that running createlang is a waste of
time for the contrib tests, and so it'd be nice if pg_regress didn't
load any PL unless told to.)

regards, tom lane

#5Andrew Dunstan
andrew@dunslane.net
In reply to: Tom Lane (#4)
Re: [PATCHES] plperl and pltcl installcheck targets

Tom Lane wrote:

The point is that I'd rather test createlang than duplicate it.

(In the back of my mind also is that running createlang is a waste of
time for the contrib tests, and so it'd be nice if pg_regress didn't
load any PL unless told to.)

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).

cheers

andrew

#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#5)
Re: [PATCHES] plperl and pltcl installcheck targets

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

regards, tom lane

#7Andrew Dunstan
andrew@dunslane.net
In reply to: Tom Lane (#6)
Re: [HACKERS] plperl and pltcl installcheck targets

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

#8Andrew Dunstan
andrew@dunslane.net
In reply to: Andrew Dunstan (#7)
1 attachment(s)
Re: [HACKERS] plperl and pltcl installcheck targets

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

Attachments:

regresslangs.patchtext/x-patch; name=regresslangs.patchDownload
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
  
  
#9Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#8)
Re: [HACKERS] plperl and pltcl installcheck targets

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

#10Andrew Dunstan
andrew@dunslane.net
In reply to: Tom Lane (#9)
Re: [HACKERS] plperl and pltcl installcheck targets

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

#11Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#1)
Re: [HACKERS] plperl and pltcl installcheck targets

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

#12Andrew Dunstan
andrew@dunslane.net
In reply to: Tom Lane (#11)
Re: [HACKERS] plperl and pltcl installcheck targets

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

#13Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#12)
Re: [HACKERS] plperl and pltcl installcheck targets

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

#14Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#1)
Re: [HACKERS] plperl and pltcl installcheck targets

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

#15Peter Eisentraut
peter_e@gmx.net
In reply to: Tom Lane (#14)
Re: [HACKERS] plperl and pltcl installcheck targets

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/

#16Andrew Dunstan
andrew@dunslane.net
In reply to: Peter Eisentraut (#15)
Re: [HACKERS] plperl and pltcl installcheck targets

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

#17Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#16)
Re: [HACKERS] plperl and pltcl installcheck targets

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

#18Andrew Dunstan
andrew@dunslane.net
In reply to: Tom Lane (#17)
Re: [HACKERS] plperl and pltcl installcheck targets

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