Prev: [HACKERS] Clearing psql's input buffer after auto-reconnect
Next: (9.1) btree_gist support for searching on "notequals"
From: Stephen Frost on 26 May 2010 15:19
* Robert Haas (robertmhaas(a)gmail.com) wrote:
> Then, too, there's the fact that many of these tests fail on my
> machine because my username is not sfrost,

I've updated the patch to address this, it's again at:
http://snowman.net/~sfrost/psql-regress-help.patch

If the size is still an issue, I could just go through and remove the
commands which return lots of records (that would also remove most of
the info about the catalog, minimizing the effect on this set of tests
when people change the catalog..). Downside there, of course, is that
we're not testing as many cases. Still, better something than nothing.
:)

> and/or because of row-ordering differences on backslash commands
> without enough ORDER BY to fully determine the output order.

I don't believe this was ever actually an issue. At least, I've run it
a number of times locally without issue. If anyone is still getting
differences when run against 9.0 HEAD, please let me know.

commit e301c873740816c5f3fd5437dcc559c880b8f404
Author: Stephen Frost <sfrost(a)snowman.net>
Date: Wed May 26 15:02:28 2010 -0400

Add regression tests for psql backslash commands

This patch adds rather extensive regression testing
of the psql backslash commands. Hopefully this will
minimize issues such as the one which cropped up
recently with \h segfaulting. Note that we don't
currently explicit check all the \h options and that
many catalog changes will mean that this needs to also
be updated. Still, it's a start, we can reduce the
set of tests if that makes sense or they become a
problem. Also, any tests which would include the
owner of the database have been excluded.

Patch here:
http://snowman.net/~sfrost/psql-regress-help.patch

Thanks,

Stephen
From: alvherre on 26 May 2010 15:52
Excerpts from Stephen Frost's message of mié may 26 15:19:59 -0400 2010:
> * Robert Haas (robertmhaas(a)gmail.com) wrote:
> > Then, too, there's the fact that many of these tests fail on my
> > machine because my username is not sfrost,
>
> I've updated the patch to address this, it's again at:
> http://snowman.net/~sfrost/psql-regress-help.patch

Isn't this kind of test a pain to maintain? If somebody add a new SQL
command, it will affect the entire \h output and she'll have to either
apply the changes without checking them, or manually check the complete
list. I have only to add a new function to make the test fail ...

Also, having to exclude tests that mention the database owner means that
you're only testing a fraction of the commands, so any possible problem
has a large chance of going undetected. I mean, if we're going to test
this kind of thing, shouldn't we be using something that allows us to
ignore the db owner name? A simple wildcard in place of the owner name
would suffice ... or do we need a regex for some other reason?

The \h output normally depends on terminal width. Have you handled that
somehow?

(And if we want something like this, I think we should not have a single
huge file for the complete test, but a set of smaller files. I'd even
put the bunch in src/bin/psql/regress rather than the main regress dir.)

--
Álvaro Herrera <alvherre(a)commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Stephen Frost on 26 May 2010 19:57
* alvherre (alvherre(a)commandprompt.com) wrote:
> Excerpts from Stephen Frost's message of mié may 26 15:19:59 -0400 2010:
> > * Robert Haas (robertmhaas(a)gmail.com) wrote:
> > > Then, too, there's the fact that many of these tests fail on my
> > > machine because my username is not sfrost,
> >
> > I've updated the patch to address this, it's again at:
> > http://snowman.net/~sfrost/psql-regress-help.patch
>
> Isn't this kind of test a pain to maintain? If somebody add a new SQL
> command, it will affect the entire \h output and she'll have to either
> apply the changes without checking them, or manually check the complete
> list. I have only to add a new function to make the test fail ...

Erm, last I checked, we already provide a diff output for the changes to
the regression output. That doesn't help when you add a new column to a
catalog, but that's a far cry from just adding some new SQL syntax or
adding a function.

> Also, having to exclude tests that mention the database owner means that
> you're only testing a fraction of the commands, so any possible problem
> has a large chance of going undetected. I mean, if we're going to test
> this kind of thing, shouldn't we be using something that allows us to
> ignore the db owner name? A simple wildcard in place of the owner name
> would suffice ... or do we need a regex for some other reason?

Yes, being able to use a regexp or something would be nice, but we don't
have those mechanics in the regression tests now (unless I missed
something).

> The \h output normally depends on terminal width. Have you handled that
> somehow?

I don't think that'll actually be an issue, but would love to hear from
people if it is.

> (And if we want something like this, I think we should not have a single
> huge file for the complete test, but a set of smaller files. I'd even
> put the bunch in src/bin/psql/regress rather than the main regress dir.)

The actual set of tests is rather small. The output is large, but
that's just because we have alot of things in the catalog.

Thanks,

Stephen
From: Tom Lane on 26 May 2010 20:30
Stephen Frost <sfrost(a)snowman.net> writes:
> * alvherre (alvherre(a)commandprompt.com) wrote:
>> (And if we want something like this, I think we should not have a single
>> huge file for the complete test, but a set of smaller files. I'd even
>> put the bunch in src/bin/psql/regress rather than the main regress dir.)

> The actual set of tests is rather small. The output is large, but
> that's just because we have alot of things in the catalog.

It sounds to me like this is going to be like the rules regression test
writ large; specifically the part that dumps out view definitions for
all the built-in views. And that, quite frankly, has been a huge
maintenance burden and AFAIR has never once had any redeeming social
value in terms of catching a bug. If you're testing things that way,
don't. There might be some value in psql backslash command tests that
are designed to depend on just one or a few tables (or other appropriate
objects). Dumping large fractions of the catalogs will just be a net
loss.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Stephen Frost on 26 May 2010 20:36
* Tom Lane (tgl(a)sss.pgh.pa.us) wrote:
> There might be some value in psql backslash command tests that
> are designed to depend on just one or a few tables (or other appropriate
> objects). Dumping large fractions of the catalogs will just be a net
> loss.

Fair enough, I can certainly do that pretty easily. Stripping out the
unqualified \d*S commands should result in a much, much, much smaller
output, with corresponding less changes due to catalog changes.

Thanks,

Stephen
First  |  Prev  |  Next  |  Last
Pages: 1 2 3
Prev: [HACKERS] Clearing psql's input buffer after auto-reconnect
Next: (9.1) btree_gist support for searching on "notequals"