Copilot commented on code in PR #7581:
URL: https://github.com/apache/ignite-3/pull/7581#discussion_r2797236352
##########
modules/catalog-compaction/src/integrationTest/java/org/apache/ignite/internal/catalog/compaction/ItCatalogCompactionTest.java:
##########
@@ -157,14 +157,18 @@ void testGlobalMinimumTxRequiredTime() {
debug.recordCatalogState("init");
debug.recordMinTxTimesState("init");
- Catalog catalog1 = getLatestCatalog(node2);
+ Catalog catalog1 = getLatestCatalog(node0);
+ assertThat(getLatestCatalog(node1).version(), is(catalog1.version()));
+ assertThat(getLatestCatalog(node2).version(), is(catalog1.version()));
Review Comment:
The test now asserts catalog versions across nodes immediately after
`dropAllTables()` ran in `@BeforeEach`. Since `dropAllTables()` executes a SQL
script but does not wait for catalog publication on all nodes, this can be
flaky if nodes haven't yet applied the latest catalog version. Consider
awaiting catalog version convergence here as well (e.g., read the current
version from one node and use the new await helper before asserting/using
`catalog1`).
```suggestion
Catalog catalog1 =
awaitCatalogPublicationOnAllNodesAndGet(getLatestCatalog(node0).version());
```
##########
modules/catalog-compaction/src/integrationTest/java/org/apache/ignite/internal/catalog/compaction/ItCatalogCompactionTest.java:
##########
@@ -334,6 +336,20 @@ private static void expectEarliestCatalogVersion(int
expectedVersion) {
});
}
+ private Catalog awaitCatalogPublicationOnAllNodesAndGet(int
expectedVersion) {
+ Awaitility.await().untilAsserted(() -> {
+ for (int i = 0; i < initialNodes(); i++) {
+ assertThat("node#" + i, getLatestCatalogVersion(node(i)),
is(expectedVersion));
+ }
+ });
Review Comment:
`awaitCatalogPublicationOnAllNodesAndGet` relies on Awaitility defaults
(timeout/poll settings). This can make the test flaky on slower CI or change
behavior if defaults are adjusted. Consider setting an explicit timeout
(consistent with the rest of this test, e.g. `COMPACTION_INTERVAL_MS`) and, if
needed for consistency, use `pollInSameThread()` as in
`expectEarliestCatalogVersion()`.
--
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]
