keith-turner commented on code in PR #5900:
URL: https://github.com/apache/accumulo/pull/5900#discussion_r2364621285
##########
core/src/test/java/org/apache/accumulo/core/security/NamespacePermissionsTest.java:
##########
@@ -31,18 +32,30 @@ public void testEnsureEquivalencies() {
EnumSet<NamespacePermission> set =
EnumSet.allOf(NamespacePermission.class);
for (TablePermission permission : TablePermission.values()) {
- set.remove(NamespacePermission.getEquivalent(permission));
+ var equivalent = NamespacePermission.getEquivalent(permission);
+ if (equivalent != null) {
+ assertTrue(set.remove(equivalent));
+ assertEquals(permission.name(), equivalent.name());
Review Comment:
yeah it just happens to be true now, if its not in the future because of
changes to enums then the test will fail and need to be fixed. Considered
explicitly enumerating things in the test by having a lot of statements like
`assertEquals(NamespacePermission.X,
NamespacePermission.getEquivalent(TablePermission.Y))` but this was less
tedious.
My main goal was defend against wires getting crossed because we could clear
the set before these changes even if something was mapped to the wrong
equivalent. Only checking the set was empty was a weak check of correctness.
--
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]
