kbendick commented on a change in pull request #3801:
URL: https://github.com/apache/iceberg/pull/3801#discussion_r774845284



##########
File path: core/src/test/java/org/apache/iceberg/hadoop/TestCachingCatalog.java
##########
@@ -252,9 +254,9 @@ public void 
testCacheExpirationEagerlyRemovesMetadataTables() throws IOException
     ticker.advance(HALF_OF_EXPIRATION);
     
Assertions.assertThat(catalog.cache().asMap()).doesNotContainKey(tableIdent);
 
-    Arrays.stream(metadataTables(tableIdent)).forEach(metadataTable ->
-        Assert.assertFalse("When a data table expires, its metadata tables 
should expire regardless of age",
-            catalog.cache().asMap().containsKey(metadataTable)));
+    // Removal of metadata tables from cache is async, use awaitility
+    await().untilAsserted(() ->
+        
Assertions.assertThat(catalog.cache().asMap()).doesNotContainKeys(metadataTables(tableIdent)));

Review comment:
       Actually, this will balloon up the PR and this is an important fix. I'd 
say let's move everything in this test suite to awaitility in a separate PR 
(though the use of awaitlity is fine here). The priority should be on fixing 
the possible deadlock :) 
   
   That way we can review this change and get it out without having to review 
the much larger diff that will come from adding `awaitility` to a lot of this 
file.




-- 
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]

Reply via email to