skoppu22 commented on code in PR #2648:
URL: https://github.com/apache/cassandra/pull/2648#discussion_r1321850744
##########
test/unit/org/apache/cassandra/tools/nodetool/InvalidateCIDRPermissionsCacheTest.java:
##########
@@ -118,10 +115,13 @@ public void testInvalidateSingleCidrPermission()
// ensure cidr permission is cached
role.hasAccessFromIp(ipAddr);
assertThat(originalReadsCount).isEqualTo(getCidrPermissionsReadCount());
+ assertThat(((CassandraCIDRAuthorizer)
DatabaseDescriptor.getCIDRAuthorizer()).has(ROLE_A)).isTrue();
// invalidate cidr permission
ToolRunner.ToolResult tool =
ToolRunner.invokeNodetool("invalidatecidrpermissionscache",
ROLE_A.getRoleName());
tool.assertOnCleanExit();
+ assertThat(tool.getStdout()).contains("Invalidated the role role_a
from CIDR permissions cache");
Review Comment:
I was making the test depend only on table read count to avoid being flaky.
Table read count tells us that a cache entry was loaded from the table, without
tightly depending on when/how the entry was invalidated. I believe that should
be sufficient in the unit test. Also, this particular check
cache.getIfPresent(<role>) was only to print descriptive message to the user
when nodetool command is run. It doesn't impact the functionality. So I
wouldn't be comfortable creating a flaky test disturbing circleCI/Jenkins
builds and impacting developers time/efforts just for this minor message.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
