Log In - Home Page

Patch: Add more regression tests for ROLE (USER)

Edit Patch - Move To Another CommitFest - Delete Patch

CommitFest 2013-06
Topic Regression Tests
Patch Status Returned with Feedback
Author Robins Tharakan
Reviewers Fabien Coelho
Committer Nobody
Close Date 2013-07-16
Patch by robins on 2013-03-20 12:08:57 PM: Initial version.
Review by calvin on 2013-05-03 01:47:35 PM: Looked at and checked, ok. Suggestions: always use different role names for different tests? why role "asdf"? 2030/1/1 -> 2030-01-01? maybe use a larger date?
Comment by robins on 2013-05-07 11:50:27 PM: 1. Different names for different tests was a learning that I got as a feedback from other submissions. So continued the pattern here too.
2. ASDF a clear identifier that a query is going to fail. I used the same role name at places, to convey that the role name though syntactically valid, its going to fail the test. This makes reading test code easier. Let me know if you want this to be named as per other names as well.
3. 2030/1/1 date is not a failure point of the test. It needs to be a valid date (but sufficiently distant that so that tests don't fail). I tried setting this to 2200/1/1 and I get the same error message. Let me know if this still needs to be a large date.
Comment by calvin on 2013-05-08 09:05:43 AM: 1: role_ro2/3 used twice?
2: I do not understand why "asdf" conveys anything about an expected failure. Association of Scientists, Developers and Faculties? :-)
3: VALID UNTIL '9999-12-31' works for me...
Patch by calvin on 2013-05-09 08:16:29 AM: version 2
Review by calvin on 2013-05-09 08:30:23 AM: Ok!
Review by rhaas on 2013-07-03 03:52:05 PM: too many unnecessary create/drop cycles
Patch by robins on 2013-07-15 02:37:40 PM: Updated patch after removing most of CREATE ROLE / DROP ROLE changes.

Add Comment

Please log in to comment on this patch.