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

[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

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

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

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

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

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