[ELY-2411] Add test cases to MappedRegexRealmMapperTest #1756
Conversation
| realmNameMap.put("A-9999", realmName); | ||
| realmNameMap.put("A-1111", "delegateRealm"); | ||
| MappedRegexRealmMapper mappedRegexRealmMapper = new MappedRegexRealmMapper(pattern, RealmMapper.single("A-1111"), realmNameMap); | ||
| Principal principal = new NamePrincipal("A-9999"); |
There was a problem hiding this comment.
@xjusko Thank you very much for the PR and sorry for the late response.
This looks good, only this second test class does not actually makes use of the delegate realm. You can see that if you delete the RealmMapper.single("A-1111") from the constructor on line 85 and the test will still pass.
To make this test use the delegate realm, we have to use a principal that does not match the pattern, eg:
new NamePrincipal("principalThatDoesNotMatchThePattern");
Then the delegate realm will be used and the result of the delegate will be a realm A-1111. This realm will then be processed by realmNameMap which will find the mapping defined above:
realmNameMap.put("A-1111", "delegateRealm");
The resulting realm will then be "delegateRealm". So the line 85 should be:
Assert.assertEquals("delegateRealm", mappedRegexRealmMapper.getRealmMapping(principal, null));
If you update the PR accordingly it will be accepted. Thank you!
There was a problem hiding this comment.
@Skyllarr I think my intention was to write a test to determine whether it returns realmName or "delegateRealm" when both match. However, now I am not sure whether that was the correct implementation of my idea. If I update it as you mentioned, then both tests would test the same thing, which we probably do not want. If this test does not make sense, maybe it would be better to remove it and keep only the first one. What do you think?
There was a problem hiding this comment.
@xjusko I see, sorry I misunderstood your intention. I think this test is good then. I am approving this PR. Thank you!
https://issues.redhat.com/browse/ELY-2411