Allow mixed case in pgtle.clientauth_users_to_skip and pgtle.clientauth_databases_to_skip #268
Conversation
…th_databases_to_skip
test/t/004_pg_tle_clientauth.pl
Outdated
| @@ -255,6 +255,46 @@ | |||
| END | |||
| $$ LANGUAGE plpgsql], on_error_die => 1); | |||
| $node->psql('postgres', qq[SELECT pgtle.register_feature('reject_testuser', 'clientauth')], on_error_die => 1); | |||
| ### 16. Allow mixedCase in pgtle.clientauth_users_to_skip | |||
| $node->psql('postgres', 'CREATE ROLE \"testUser3\" LOGIN', stderr => \$psql_err); | |||
There was a problem hiding this comment.
I think this line is failing because the double-quotes do not need to be escaped, i.e. CREATE ROLE "testUser3".
Can you also add on_error_die => 1 to each psql command that is not supposed to error? By default psql errors don't cause tap tests to fail which can be confusing.
There was a problem hiding this comment.
I digged into this a bit and I think stderr => \$psql_err is a bit better because on_error_die => 1 just fails the test but doesn't offer any output or tell us which line the test fails on
There was a problem hiding this comment.
In that case you'll need to verify that psql_err is empty after each psql invocation, otherwise it will populate psql_err with stderr but won't throw an error
There was a problem hiding this comment.
Cant we have both stderr and on_error_die as arguments in the psql call?
There was a problem hiding this comment.
If we specify on_error_die and psql throws an error, it won't print psql_err by default iiuc. We still need a line such as like($psql_err, '')
There was a problem hiding this comment.
on_error_die also emits the psql error to the regress logs. They should be uploaded as artifacts if a test run fails
|
Please also remove |
|
It's not very clear to me why we need to move the new tests to a separate file. i dont see a strong reason on why we need to split them into different files for this specific edge case. |
|
Do you mind adding your test case and updating the numbers at the top of the file as well: https://github.com/aws/pg_tle/blob/main/test/t/004_pg_tle_clientauth.pl#L24 |
+1 on this |
|
LGTM |
Issue #, if available: 265
Description of changes: Allow mixed case in pgtle.clientauth_users_to_skip and pgtle.clientauth_databases_to_skip
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.