Re: [HACKERS] Add regression tests for COLLATE
On Sat, Jul 6, 2013 at 9:27 AM, Andrew Dunstan and...@dunslane.net wrote: Or maybe just invent a magic result file name such as none or do_not_run. I'm not keen to have us build up a large patchwork of regression tests that run on some platforms and not on others, though. Me, neither. But I don't really see a plausible alternative in this particular case, since we're trying to test a feature that only works in the presence of specific platform support. I don't think that having no buildfarm testing of collation support is better. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] Add regression tests for COLLATE
On Fri, Jul 5, 2013 at 6:56 PM, Andres Freund and...@2ndquadrant.com wrote: On 2013-07-03 14:17:20 -0400, Robert Haas wrote: I agree. I think it'd be a good idea to get the buildfarm to run the existing collate.utf8.linux test regularly on platforms where it passes, but this particular approach is valuable mostly because (supposedly) it was going to work everywhere. However, it doesn't. Hm. What about extending the existing resultmap logic to make that possible in general? E.g. don't run the test if a file is mapped to an empty expected file. Not a bad thought. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] Add regression tests for COLLATE
On 07/06/2013 09:12 AM, Robert Haas wrote: On Fri, Jul 5, 2013 at 6:56 PM, Andres Freund and...@2ndquadrant.com wrote: On 2013-07-03 14:17:20 -0400, Robert Haas wrote: I agree. I think it'd be a good idea to get the buildfarm to run the existing collate.utf8.linux test regularly on platforms where it passes, but this particular approach is valuable mostly because (supposedly) it was going to work everywhere. However, it doesn't. Hm. What about extending the existing resultmap logic to make that possible in general? E.g. don't run the test if a file is mapped to an empty expected file. Not a bad thought. Or maybe just invent a magic result file name such as none or do_not_run. I'm not keen to have us build up a large patchwork of regression tests that run on some platforms and not on others, though. cheers andrew -- 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] Add regression tests for COLLATE
On 07/03/2013 03:16 PM, Andrew Dunstan wrote: On 07/03/2013 03:00 PM, Alvaro Herrera wrote: Robert Haas escribió: I agree. I think it'd be a good idea to get the buildfarm to run the existing collate.utf8.linux test regularly on platforms where it passes, +1 I can probably whip up a module for it. (I created the buildfarm module system so people could create their own addons, but sadly nobody seems to have risen to the bait.) See https://github.com/PGBuildFarm/client-code/blob/626a537034f89022b133badf6093ba243f6072aa/PGBuild/Modules/TestCollateLinuxUTF8.pm I have added it to crake's set of modules to run, See http://www.pgbuildfarm.org/cgi-bin/show_stage_log.pl?nm=crakedt=2013-07-05%2023%3A26%3A17stg=FileTextArrayFDW-installcheck-en_US.utf8, and it will be in the next buildfarm client release. cheers andrew -- 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] Add regression tests for COLLATE
On 07/05/2013 07:44 PM, Andrew Dunstan wrote: On 07/03/2013 03:16 PM, Andrew Dunstan wrote: On 07/03/2013 03:00 PM, Alvaro Herrera wrote: Robert Haas escribió: I agree. I think it'd be a good idea to get the buildfarm to run the existing collate.utf8.linux test regularly on platforms where it passes, +1 I can probably whip up a module for it. (I created the buildfarm module system so people could create their own addons, but sadly nobody seems to have risen to the bait.) See https://github.com/PGBuildFarm/client-code/blob/626a537034f89022b133badf6093ba243f6072aa/PGBuild/Modules/TestCollateLinuxUTF8.pm I have added it to crake's set of modules to run, See http://www.pgbuildfarm.org/cgi-bin/show_stage_log.pl?nm=crakedt=2013-07-05%2023%3A26%3A17stg=FileTextArrayFDW-installcheck-en_US.utf8, and it will be in the next buildfarm client release. Er I meant http://www.pgbuildfarm.org/cgi-bin/show_stage_log.pl?nm=crakedt=2013-07-05%2023%3A26%3A17stg=install-check-collate-en_US.utf8 cheers andrew -- 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] Add regression tests for COLLATE
On 2013-07-03 14:17:20 -0400, Robert Haas wrote: I agree. I think it'd be a good idea to get the buildfarm to run the existing collate.utf8.linux test regularly on platforms where it passes, but this particular approach is valuable mostly because (supposedly) it was going to work everywhere. However, it doesn't. Hm. What about extending the existing resultmap logic to make that possible in general? E.g. don't run the test if a file is mapped to an empty expected file. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- 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] Add regression tests for COLLATE
On Thu, May 9, 2013 at 9:27 PM, Robins Tharakan thara...@gmail.com wrote: One workaround is to use DROP COLLATION IF EXISTS, that conveys the message without UTF8 in the message string. But three other tests use ALTER COLLATION and I see no way but to comment / disable them. Currently have them disabled (with comments as to what they do, and why they're disabled). This updated patch doesn't have UTF8 string anywhere. Let me know if you prefer to remove the commented out tests completely. I did remove those, plus made some other cleanups, and committed this. I think that there's now some duplication between this and the collate.linux.utf8 test that should be cleaned up by removing the duplicative stuff from that test, assuming this holds up OK when the buildfarm - and other developers - test it out. Could you prepare a patch towards that end? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] Add regression tests for COLLATE
Robert Haas robertmh...@gmail.com writes: I did remove those, plus made some other cleanups, and committed this. I think that there's now some duplication between this and the collate.linux.utf8 test that should be cleaned up by removing the duplicative stuff from that test, assuming this holds up OK when the buildfarm - and other developers - test it out. I don't even need to test it: regression=# CREATE COLLATION collate_coll2 FROM C; ERROR: nondefault collations are not supported on this platform Please revert. 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] Add regression tests for COLLATE
On Wed, Jul 3, 2013 at 1:11 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: I did remove those, plus made some other cleanups, and committed this. I think that there's now some duplication between this and the collate.linux.utf8 test that should be cleaned up by removing the duplicative stuff from that test, assuming this holds up OK when the buildfarm - and other developers - test it out. I don't even need to test it: regression=# CREATE COLLATION collate_coll2 FROM C; ERROR: nondefault collations are not supported on this platform Please revert. OK, sure, but just for my education and general enlightenment ... what platform is that? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] Add regression tests for COLLATE
On Wed, Jul 3, 2013 at 1:26 PM, Robert Haas robertmh...@gmail.com wrote: Please revert. OK, sure, but just for my education and general enlightenment ... what platform is that? Ah, never mind. I see that the buildfarm is quickly turning red - NetBSD, OmniOS, and HP/UX are all unhappy. I think that's a killer blow for this particular patch. I'm going to set this to rejected in the CF app. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] Add regression tests for COLLATE
On 2013-07-03 13:29:18 -0400, Robert Haas wrote: On Wed, Jul 3, 2013 at 1:26 PM, Robert Haas robertmh...@gmail.com wrote: Please revert. OK, sure, but just for my education and general enlightenment ... what platform is that? Ah, never mind. I see that the buildfarm is quickly turning red - NetBSD, OmniOS, and HP/UX are all unhappy. I think that's a killer blow for this particular patch. I'm going to set this to rejected in the CF app. Can't we use a alternative expected file for those? Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- 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] Add regression tests for COLLATE
Andres Freund and...@2ndquadrant.com writes: On 2013-07-03 13:29:18 -0400, Robert Haas wrote: I think that's a killer blow for this particular patch. I'm going to set this to rejected in the CF app. Can't we use a alternative expected file for those? Alternative expected files are a PITA to maintain, at least for committers who don't have a platform that can reproduce the alternative behavior. If this test were of somewhat higher value I'd be in favor of fixing it that way, but given that it's been seriously constrained by the portability issues that were already considered, I'm not sure it's worth our trouble. (There's also no very strong reason to believe that we found out all the remaining portability issues. Maybe we should have left it in there for a day, just to see if the buildfarm would show any other symptoms besides this one.) 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] Add regression tests for COLLATE
On Wed, Jul 3, 2013 at 1:38 PM, Tom Lane t...@sss.pgh.pa.us wrote: Andres Freund and...@2ndquadrant.com writes: On 2013-07-03 13:29:18 -0400, Robert Haas wrote: I think that's a killer blow for this particular patch. I'm going to set this to rejected in the CF app. Can't we use a alternative expected file for those? Alternative expected files are a PITA to maintain, at least for committers who don't have a platform that can reproduce the alternative behavior. If this test were of somewhat higher value I'd be in favor of fixing it that way, but given that it's been seriously constrained by the portability issues that were already considered, I'm not sure it's worth our trouble. (There's also no very strong reason to believe that we found out all the remaining portability issues. Maybe we should have left it in there for a day, just to see if the buildfarm would show any other symptoms besides this one.) I agree. I think it'd be a good idea to get the buildfarm to run the existing collate.utf8.linux test regularly on platforms where it passes, but this particular approach is valuable mostly because (supposedly) it was going to work everywhere. However, it doesn't. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] Add regression tests for COLLATE
Robert Haas escribió: I agree. I think it'd be a good idea to get the buildfarm to run the existing collate.utf8.linux test regularly on platforms where it passes, +1 -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- 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] Add regression tests for COLLATE
On 07/03/2013 03:00 PM, Alvaro Herrera wrote: Robert Haas escribió: I agree. I think it'd be a good idea to get the buildfarm to run the existing collate.utf8.linux test regularly on platforms where it passes, +1 I can probably whip up a module for it. (I created the buildfarm module system so people could create their own addons, but sadly nobody seems to have risen to the bait.) cheers andrew -- 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] Add regression tests for COLLATE
Fabien pointed out that currently does not check for non-trivial locales. Indeed, but although it was not very from my point (my wish), and as pointed out by Tom, it is not quite possible to test non trivial locales because you cannot assume that a given locale is available on any test machine. Maybe some tests could be applied under some condition, say a given locale is indeed available, but ISTM that it would require to change the test infrastructure in a portable way to add such feature. -- Fabien. -- 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] Add regression tests for COLLATE
Maybe some tests could be applied under some condition, say a given locale is indeed available, but ISTM that it would require to change the test infrastructure in a portable way to add such feature. I just noticed that there is a src/test/locale directory with some stuff about german, greek and korean, so ISTM that the infrastructure is there, and could be populated with other locales, especially something with UTF8 which is quite commonplace, would be nice. However, it is not clear whether these tests run automatically or only if they are explicitely called. The README seems to suggest that it is the later. If so, maybe having them invoked automatically if possible would ensure that they are run from time to time. -- Fabien. -- 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] Add regression tests for COLLATE
On Thu, May 9, 2013 at 4:19 PM, Fabien COELHO coe...@cri.ensmp.fr wrote: However, it is not clear whether these tests run automatically or only if they are explicitely called. The README seems to suggest that it is the later. If so, maybe having them invoked automatically if possible would ensure that they are run from time to time. Only isolation and regress are run automatically inside the buildfarm. -- Michael
Re: [HACKERS] Add regression tests for COLLATE
One workaround is to use DROP COLLATION IF EXISTS, that conveys the message without UTF8 in the message string. But three other tests use ALTER COLLATION and I see no way but to comment / disable them. Currently have them disabled (with comments as to what they do, and why they're disabled). This updated patch doesn't have UTF8 string anywhere. Let me know if you prefer to remove the commented out tests completely. -- Robins Tharakan On 9 May 2013 07:44, Tom Lane t...@sss.pgh.pa.us wrote: Robins Tharakan thara...@gmail.com writes: Fabien pointed out that currently does not check for non-trivial locales. I am still on the learning curve about LOCALEs and so, let me know if this is a show-stopper. I guess I could look at it and get back in some time with more tests as Fabien points out. You really can't, because there is no guarantee that any given machine will have anything except C and POSIX. But there's another problem: I believe this test will fail on any machine where the database is created with an encoding different from UTF8, because that encoding is named in some of the error messages in the expected output. This stuff is not easy to test in a portable way. regards, tom lane regress_collate_v3.patch Description: Binary data -- 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] Add regression tests for COLLATE
Hi, Please find attached the updated patch. Fabien pointed out that currently does not check for non-trivial locales. I am still on the learning curve about LOCALEs and so, let me know if this is a show-stopper. I guess I could look at it and get back in some time with more tests as Fabien points out. (Apologies for the delay though. An update to the patch was mostly done back in April, but since most of the other Code-Coverage patches (SCHEMA/ROLE/etc.) had no other feedback, I worked on all of them together just this week). -- Robins Tharakan On 12 April 2013 09:28, Michael Paquier michael.paqu...@gmail.com wrote: On Thu, Apr 11, 2013 at 4:14 PM, Robins Tharakan thara...@gmail.comwrote: Hi, Please find attached a patch to take 'make check' code-coverage of COLLATE (/src/backend/commands/collationcmds) from 0% to 96%. Any feedback is more than welcome. Also posting this to Commitfest-next. Just by having a quick look at the patch, using object names of the type cX is too generic even if the tests are done in a private schema. Why not using a name like collate_obj_X or similar? -- Michael regress_collate_v2.patch Description: Binary data -- 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] Add regression tests for COLLATE
Robins Tharakan thara...@gmail.com writes: Fabien pointed out that currently does not check for non-trivial locales. I am still on the learning curve about LOCALEs and so, let me know if this is a show-stopper. I guess I could look at it and get back in some time with more tests as Fabien points out. You really can't, because there is no guarantee that any given machine will have anything except C and POSIX. But there's another problem: I believe this test will fail on any machine where the database is created with an encoding different from UTF8, because that encoding is named in some of the error messages in the expected output. This stuff is not easy to test in a portable way. 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
[HACKERS] Add regression tests for COLLATE
Hi, Please find attached a patch to take 'make check' code-coverage of COLLATE (/src/backend/commands/collationcmds) from 0% to 96%. Any feedback is more than welcome. Also posting this to Commitfest-next. -- Robins Tharakan regress_collate_v1.patch Description: Binary data -- 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] Add regression tests for COLLATE
On Thu, Apr 11, 2013 at 4:14 PM, Robins Tharakan thara...@gmail.com wrote: Hi, Please find attached a patch to take 'make check' code-coverage of COLLATE (/src/backend/commands/collationcmds) from 0% to 96%. Any feedback is more than welcome. Also posting this to Commitfest-next. Just by having a quick look at the patch, using object names of the type cX is too generic even if the tests are done in a private schema. Why not using a name like collate_obj_X or similar? -- Michael
