Re: [HACKERS] Regression testing for psql
On Wed, May 26, 2010 at 6:25 PM, Stephen Frost sfr...@snowman.net wrote: * Tom Lane (t...@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). Updated, much much smaller, patch attached. Also available, again, at http://snowman.net/~sfrost/psql-regress-help.patch Basically, I removed anything that would produce data directly from the catalogs by trying to find a 'none' object which matched. This still goes through alot of the same setup and query, it's just that there aren't any results. Is this something to be added to 2010-07 commitfest? -selena -- http://chesnok.com/daily - me -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Regression testing for psql
On tis, 2010-05-25 at 06:23 -0400, Stephen Frost wrote: * Robert Haas (robertmh...@gmail.com) wrote: Of course, if people want to suggest tests that just shouldn't be included, I can go through and strip things out. Well... I'm a little reluctant to believe that we should have 3.3M of tests for the entire backend and 5M of tests just for psql. Then, too, there's the fact that many of these tests fail on my machine because my username is not sfrost, and/or because of row-ordering differences on backslash commands without enough ORDER BY to fully determine the output order. Yeah, you know, I had fully intended to go grepping through the output last night to check for things like that, but my wife decided I needed sleep instead. :) Sorry about that. Still, it's more of a general proposal than something I think should be committed as-is. Should we try to deal with those kinds of differences, or just eliminate the tests which are dependent on username, etc? It definitely strikes me that there's a fair bit of code in psql we're not exercising in some fashion in the regression suite... :/ Maybe pg_regress is not the right framework to test that sort of thing. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Regression testing for psql
* Peter Eisentraut (pete...@gmx.net) wrote: Maybe pg_regress is not the right framework to test that sort of thing. Perhaps, but if not, then what? And how can we avoid writing a bunch of new code that would then need to be checked itself..? Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Regression testing for psql
* Robert Haas (robertmh...@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 sfr...@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 signature.asc Description: Digital signature
Re: [HACKERS] Regression testing for psql
Excerpts from Stephen Frost's message of mié may 26 15:19:59 -0400 2010: * Robert Haas (robertmh...@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 alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Regression testing for psql
* alvherre (alvhe...@commandprompt.com) wrote: Excerpts from Stephen Frost's message of mié may 26 15:19:59 -0400 2010: * Robert Haas (robertmh...@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 signature.asc Description: Digital signature
Re: [HACKERS] Regression testing for psql
Stephen Frost sfr...@snowman.net writes: * alvherre (alvhe...@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@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Regression testing for psql
* Tom Lane (t...@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 signature.asc Description: Digital signature
Re: [HACKERS] Regression testing for psql
* Tom Lane (t...@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). Updated, much much smaller, patch attached. Also available, again, at http://snowman.net/~sfrost/psql-regress-help.patch Basically, I removed anything that would produce data directly from the catalogs by trying to find a 'none' object which matched. This still goes through alot of the same setup and query, it's just that there aren't any results. Thanks, Stephen psql-regress-help.patch.gz Description: Binary data signature.asc Description: Digital signature
Re: [HACKERS] Regression testing for psql
* Robert Haas (robertmh...@gmail.com) wrote: Of course, if people want to suggest tests that just shouldn't be included, I can go through and strip things out. Well... I'm a little reluctant to believe that we should have 3.3M of tests for the entire backend and 5M of tests just for psql. Then, too, there's the fact that many of these tests fail on my machine because my username is not sfrost, and/or because of row-ordering differences on backslash commands without enough ORDER BY to fully determine the output order. Yeah, you know, I had fully intended to go grepping through the output last night to check for things like that, but my wife decided I needed sleep instead. :) Sorry about that. Still, it's more of a general proposal than something I think should be committed as-is. Should we try to deal with those kinds of differences, or just eliminate the tests which are dependent on username, etc? It definitely strikes me that there's a fair bit of code in psql we're not exercising in some fashion in the regression suite... :/ Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Regression testing for psql
* Stephen Frost (sfr...@snowman.net) wrote: Add regression testing 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 pretty much any 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. And.. it's way too big to send to the list. The patch is available here: http://snowman.net/~sfrost/psql-regress-help.patch Of course, if people want to suggest tests that just shouldn't be included, I can go through and strip things out. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Regression testing for psql
On Mon, May 24, 2010 at 10:51 PM, Stephen Frost sfr...@snowman.net wrote: * Stephen Frost (sfr...@snowman.net) wrote: Add regression testing 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 pretty much any 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. And.. it's way too big to send to the list. The patch is available here: http://snowman.net/~sfrost/psql-regress-help.patch Of course, if people want to suggest tests that just shouldn't be included, I can go through and strip things out. Well... I'm a little reluctant to believe that we should have 3.3M of tests for the entire backend and 5M of tests just for psql. Then, too, there's the fact that many of these tests fail on my machine because my username is not sfrost, and/or because of row-ordering differences on backslash commands without enough ORDER BY to fully determine the output order. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
