Re: [HACKERS] Add regression tests for ROLE (USER)

2013-07-15 Thread Robins Tharakan
On 6 July 2013 20:25, Robins Tharakan thara...@gmail.com wrote: Do let me know your view on this second point, so that I can remove these tests if so required. Hi, Please find attached the updated patch. It address the first issue regarding reducing the repeated CREATE / DROP ROLEs. It

Re: [HACKERS] Add regression tests for ROLE (USER)

2013-07-15 Thread Robert Haas
On Mon, Jul 15, 2013 at 10:23 AM, Robins Tharakan thara...@gmail.com wrote: It still doesn't address the excessive (syntactical) checks tough. I am still unclear as to how to identify which checks to skip. (As in, although I have a personal preference of checking everything, my question

Re: [HACKERS] Add regression tests for ROLE (USER)

2013-07-15 Thread Fabien COELHO
I simply don't understand how we can be getting any meaningful test coverage out of those cases. I mean, if we want to check every bit of syntax that could lead to a syntax error, we could probably come up with a near-infinite number of test cases: I think that it would be enough to check

Re: [HACKERS] Add regression tests for ROLE (USER)

2013-07-15 Thread Robert Haas
On Mon, Jul 15, 2013 at 11:48 AM, Fabien COELHO coe...@cri.ensmp.fr wrote: I simply don't understand how we can be getting any meaningful test coverage out of those cases. I mean, if we want to check every bit of syntax that could lead to a syntax error, we could probably come up with a

Re: [HACKERS] Add regression tests for ROLE (USER)

2013-07-09 Thread Fabien COELHO
I think that it is not that simple: it is a good value to check that the syntax error message conveys a useful information for the user, and that changes to the parser rules do not alter good quality error messages. It's good to check those things when a feature is implemented. However, once

Re: [HACKERS] Add regression tests for ROLE (USER)

2013-07-07 Thread Fabien COELHO
Generally, I think that the tests which return a syntax error are of limited value and should probably be dropped. I think that it is not that simple: it is a good value to check that the syntax error message conveys a useful information for the user, and that changes to the parser rules do

Re: [HACKERS] Add regression tests for ROLE (USER)

2013-07-07 Thread Tom Lane
Fabien COELHO coe...@cri.ensmp.fr writes: Generally, I think that the tests which return a syntax error are of limited value and should probably be dropped. I think that it is not that simple: it is a good value to check that the syntax error message conveys a useful information for the

Re: [HACKERS] Add regression tests for ROLE (USER)

2013-07-07 Thread Andres Freund
On 2013-07-07 11:11:49 -0400, Tom Lane wrote: Fabien COELHO coe...@cri.ensmp.fr writes: Generally, I think that the tests which return a syntax error are of limited value and should probably be dropped. I think that it is not that simple: it is a good value to check that the syntax

Re: [HACKERS] Add regression tests for ROLE (USER)

2013-07-06 Thread Robins Tharakan
However, before it can get committed, I think this set of tests needs streamlining. It does seem to me valuable, but I think it's wasteful in terms of runtime to create so many roles, do just one thing with them, and then drop them. I recommend consolidating some of the tests. For example:

Re: [HACKERS] Add regression tests for ROLE (USER)

2013-07-03 Thread Robert Haas
On Thu, May 9, 2013 at 4:29 AM, Fabien COELHO coe...@cri.ensmp.fr wrote: This updated version works for me and addresses previous comments. I think that such tests are definitely valuable, especially as many corner cases which must trigger errors are covered. I recommend to apply it. I'm

Re: [HACKERS] Add regression tests for ROLE (USER)

2013-05-09 Thread Fabien COELHO
This updated version works for me and addresses previous comments. I think that such tests are definitely valuable, especially as many corner cases which must trigger errors are covered. I recommend to apply it. Please find an updated patch as per comments on Commitfest (comments

Re: [HACKERS] Add regression tests for ROLE (USER)

2013-05-08 Thread Robins Tharakan
Hi, Please find an updated patch as per comments on Commitfest (comments replicated below for ease of understanding). Feedback 1: fc: role_ro2/3 used twice? rt: Corrected in this update. Feedback 2: fc: I do not understand why asdf conveys anything about an expected failure. Association of

[HACKERS] Add regression tests for ROLE (USER)

2013-03-19 Thread Robins Tharakan
Hi, Please find attached a patch to take 'make check' code-coverage of ROLE (USER) from 59% to 91%. Any feedback is more than welcome. -- Robins Tharakan regress_user.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your