HDDS-5778. Client side unit tests #2810
Conversation
| filter.init(filterConfig); | ||
| filter.doFilter(request, response, filterChain); | ||
| filter.destroy(); | ||
| } |
There was a problem hiding this comment.
Can you please help me understand what is being asserted here?
There was a problem hiding this comment.
@adoroszlai I reviewed with @neils-dev and confirmed that there was an assert missing which I've added. Thanks for noticing!
There was a problem hiding this comment.
Thanks. Would it make sense to add one or more other test cases where the filter is expected to reject the request(s)? I guess the current test would pass even with a no-op filter.
There was a problem hiding this comment.
More tests added @adoroszlai Sorry it took so long.
| get(AUTHORIZATION_HEADER).replace("20210616", curDate)); | ||
|
|
||
|
|
||
| // Should not generate exception because of corrected date |
There was a problem hiding this comment.
Nit: adding each test case in a separate method would be cleaner.
There was a problem hiding this comment.
ok, @adoroszlai I split them up.
There was a problem hiding this comment.
Thanks @GeorgeJahad for updating the patch. I think the test cases are much easier to read now. 👍
|
@adoroszlai since you've approved this pr and the ci tests succeeded, would you be able to merge this into the feature branch? or is there something else that should be done? |
|
Thanks @GeorgeJahad and @neils-dev for the patch.
Thanks for the reminder. |
What changes were proposed in this pull request?
Adding a test for new ugi filter. This code was written entirely by Neil Joshi
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-5778
How was this patch tested?