zeroflag commented on code in PR #597:
URL: https://github.com/apache/knox/pull/597#discussion_r905386669
##########
gateway-provider-security-authz-composite/src/main/java/org/apache/knox/gateway/deploy/impl/CompositeAuthzDeploymentContributor.java:
##########
@@ -71,7 +71,7 @@ public void contributeFilter( DeploymentContext context,
Provider provider, Serv
}
List parseProviderNames(String providerNames) {
Review Comment:
What's the reason for not having a type parameter for the List? (E.g.
List<String>)
##########
gateway-provider-security-authz-composite/src/test/java/org/apache/knox/gateway/deploy/impl/CompositeAuthzProviderTest.java:
##########
@@ -83,13 +85,19 @@ public void testingNullandEmptyProviderNames() throws
Exception {
String testnames = "";
CompositeAuthzDeploymentContributor c = new
CompositeAuthzDeploymentContributor();
List providerNames = c.parseProviderNames(testnames);
- assertEquals(providerNames.size(), 1);
- assertEquals(providerNames.get(0), "");
+ assertSame(providerNames.size(),0);
Review Comment:
Here the order of parameters are reversed. The first is the expected and the
second is the actual.
This is not a correctness issue, but the error message will misleading if
the test fails.
For example if providerNames has a size of 3 it'll say: expected 3 got 0,
but it is the other way around.
More importantly what's the reason for using assertSame instead of
assertEquals? My understanding is that assertSame checks identity (`==`) not
equality (`.equals`).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
