DomGarguilo commented on code in PR #6426:
URL: https://github.com/apache/accumulo/pull/6426#discussion_r3405444117
##########
test/src/main/java/org/apache/accumulo/test/ListCompactionsIT.java:
##########
@@ -105,12 +106,20 @@ public void testListRunningCompactions() throws Exception
{
ExternalCompactionTestUtils.getRunningCompactions(getCluster().getServerContext());
}
- final List<RunningCompactionSummary> running =
- new
ListCompactionsWrapper().getRunningCompactions(getCluster().getServerContext(),
true);
+ final List<RunningCompactionSummary> running = new ArrayList<>();
final Map<String,RunningCompactionSummary> compactionsByEcid = new
HashMap<>();
- running.forEach(rcs -> compactionsByEcid.put(rcs.getEcid(), rcs));
+ final var expectedCompactions = expected;
+
+ Wait.waitFor(() -> {
+ running.clear();
+ compactionsByEcid.clear();
+
+ running.addAll(new
ListCompactionsWrapper().getRunningCompactions(getCluster().getServerContext(),
true));
+ running.forEach(rcs -> compactionsByEcid.put(rcs.getEcid(), rcs));
+
+ return expectedCompactions.size() == compactionsByEcid.size();
+ }, 10000);
- assertEquals(expected.size(), compactionsByEcid.size());
expected.values().forEach(tec -> {
Review Comment:
```suggestion
expectedCompactions.values().forEach(tec -> {
```
We should use the updated collection here.
##########
test/src/main/java/org/apache/accumulo/test/conf/ResourceGroupConfigIT.java:
##########
@@ -274,8 +274,11 @@ public void testDuplicateCreatesRemovals()
assertEquals(rgid, rgs2.iterator().next());
client.resourceGroupOperations().create(rgid); // creating again
succeeds doing nothing
client.resourceGroupOperations().remove(rgid);
- rgs = client.resourceGroupOperations().list();
- assertEquals(1, rgs.size());
+
+ Wait.waitFor(() -> {
+ final Set<ResourceGroupId> finalrgs =
client.resourceGroupOperations().list();
+ return finalrgs.size() == 1;
+ });
assertEquals(ResourceGroupId.DEFAULT, rgs.iterator().next());
Review Comment:
```suggestion
return finalrgs.size() == 1 &&
finalrgs.contains(ResourceGroupId.DEFAULT);
});
```
We should replace this assert with its equivalent check in the wait block so
we are checking the updated collection as opposed to the stale `rgs`
##########
test/src/main/java/org/apache/accumulo/test/ListCompactionsIT.java:
##########
@@ -105,12 +106,20 @@ public void testListRunningCompactions() throws Exception
{
ExternalCompactionTestUtils.getRunningCompactions(getCluster().getServerContext());
}
- final List<RunningCompactionSummary> running =
- new
ListCompactionsWrapper().getRunningCompactions(getCluster().getServerContext(),
true);
+ final List<RunningCompactionSummary> running = new ArrayList<>();
final Map<String,RunningCompactionSummary> compactionsByEcid = new
HashMap<>();
- running.forEach(rcs -> compactionsByEcid.put(rcs.getEcid(), rcs));
+ final var expectedCompactions = expected;
+
+ Wait.waitFor(() -> {
+ running.clear();
+ compactionsByEcid.clear();
+
+ running.addAll(new
ListCompactionsWrapper().getRunningCompactions(getCluster().getServerContext(),
true));
+ running.forEach(rcs -> compactionsByEcid.put(rcs.getEcid(), rcs));
+
+ return expectedCompactions.size() == compactionsByEcid.size();
Review Comment:
```suggestion
return expectedCompactions.size() == compactionsByEcid.size() &&
expectedCompactions.keySet().stream().allMatch(compactionsByEcid::containsKey);
```
This was here before you started making changes but it would probably be
good to make sure we have the compactions that we think we do.
--
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]
