pibizza commented on code in PR #6131:
URL:
https://github.com/apache/incubator-kie-drools/pull/6131#discussion_r1957755983
##########
drools-reliability/drools-reliability-tests/src/test/java/org/drools/reliability/test/EmbeddedStorageManagerTest.java:
##########
@@ -63,54 +61,29 @@ static boolean isEmbeddedInfinispan() {
void removeAllSessionCaches_shouldLeaveNonSessionCache() {
((InfinispanStorageManager)
StorageManagerFactory.get().getStorageManager()).setEmbeddedCacheManager(new
FakeCacheManager());
-
assertThat(StorageManagerFactory.get().getStorageManager().getStorageNames()).containsExactlyInAnyOrder(
- SESSION_STORAGE_PREFIX + "0_" + "epDefault",
SESSION_STORAGE_PREFIX + "1_" + "epDefault", "METADATA_0");
-
StorageManagerFactory.get().getStorageManager().removeAllSessionStorages();
-
assertThat(StorageManagerFactory.get().getStorageManager().getStorageNames()).containsExactly("METADATA_0");
+
assertThat(StorageManagerFactory.get().getStorageManager().getStorageNames()).contains("METADATA_0");
}
Review Comment:
Why we use a weaker contains statement? do we expect to see something than
METADATA_0 in the storage? I think that in this form the test is passing EVEN
IF you do not remote the session storages, so essentially it becomes useless.
##########
drools-reliability/drools-reliability-tests/src/test/java/org/drools/reliability/test/EmbeddedStorageManagerTest.java:
##########
@@ -63,54 +61,29 @@ static boolean isEmbeddedInfinispan() {
void removeAllSessionCaches_shouldLeaveNonSessionCache() {
((InfinispanStorageManager)
StorageManagerFactory.get().getStorageManager()).setEmbeddedCacheManager(new
FakeCacheManager());
-
assertThat(StorageManagerFactory.get().getStorageManager().getStorageNames()).containsExactlyInAnyOrder(
- SESSION_STORAGE_PREFIX + "0_" + "epDefault",
SESSION_STORAGE_PREFIX + "1_" + "epDefault", "METADATA_0");
-
StorageManagerFactory.get().getStorageManager().removeAllSessionStorages();
-
assertThat(StorageManagerFactory.get().getStorageManager().getStorageNames()).containsExactly("METADATA_0");
+
assertThat(StorageManagerFactory.get().getStorageManager().getStorageNames()).contains("METADATA_0");
}
@Test
void removeCachesBySessionId_shouldRemoveSpecifiedCacheOnly() {
((InfinispanStorageManager)
StorageManagerFactory.get().getStorageManager()).setEmbeddedCacheManager(new
FakeCacheManager());
-
assertThat(StorageManagerFactory.get().getStorageManager().getStorageNames()).containsExactlyInAnyOrder(
- SESSION_STORAGE_PREFIX + "0_" + "epDefault",
SESSION_STORAGE_PREFIX + "1_" + "epDefault", "METADATA_0");
-
StorageManagerFactory.get().getStorageManager().removeStoragesBySessionId("1");
-
assertThat(StorageManagerFactory.get().getStorageManager().getStorageNames()).containsExactlyInAnyOrder(SESSION_STORAGE_PREFIX
+ "0_" + "epDefault", "METADATA_0");
+
assertThat(StorageManagerFactory.get().getStorageManager().getStorageNames()).contains(SESSION_STORAGE_PREFIX
+ "0_" + "epDefault", "METADATA_0");
}
Review Comment:
Why we use a weaker version of the contains here? do we expect to see
something more than the list of elements? As above, in this form the assertion
is true even if you do not perform the removal at all. Please justify.
--
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]
