Copilot commented on code in PR #9942:
URL: https://github.com/apache/gravitino/pull/9942#discussion_r2787015218
##########
catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/integration/test/CatalogDorisIT.java:
##########
@@ -564,10 +564,14 @@ void testAlterDorisTable() {
.pollInterval(WAIT_INTERVAL_IN_SECONDS, TimeUnit.SECONDS)
.untilAsserted(
() -> assertEquals(5,
tableCatalog.loadTable(tableIdentifier).columns().length));
-
- ITUtils.assertColumn(
- Column.of("col_5", Types.VarCharType.of(255), "col_5_comment"),
- tableCatalog.loadTable(tableIdentifier).columns()[4]);
+ Awaitility.await()
+ .atMost(MAX_WAIT_IN_SECONDS, TimeUnit.SECONDS)
+ .pollInterval(WAIT_INTERVAL_IN_SECONDS, TimeUnit.SECONDS)
+ .untilAsserted(
+ () ->
+ ITUtils.assertColumn(
+ Column.of("col_5", Types.VarCharType.of(255),
"col_5_comment"),
+ tableCatalog.loadTable(tableIdentifier).columns()[4]));
Review Comment:
This adds a second, separate Awaitility wait right after the existing wait
for `columns().length == 5`. Because the waits are sequential, the test can now
block for up to `2 * MAX_WAIT_IN_SECONDS` (e.g., 60s) before failing. Consider
combining the length check and `assertColumn` into a single `untilAsserted` (or
just await the `assertColumn`, which already implies the length) so the overall
timeout remains bounded by `MAX_WAIT_IN_SECONDS`.
##########
catalogs/catalog-jdbc-starrocks/src/test/java/org/apache/gravitino/catalog/starrocks/integration/test/CatalogStarRocksIT.java:
##########
@@ -530,10 +530,14 @@ void testAlterStarRocksTable() {
.pollInterval(WAIT_INTERVAL_IN_SECONDS, TimeUnit.SECONDS)
.untilAsserted(
() -> assertEquals(5,
tableCatalog.loadTable(tableIdentifier).columns().length));
-
- ITUtils.assertColumn(
- Column.of("col_5", Types.VarCharType.of(255), "col_5_comment"),
- tableCatalog.loadTable(tableIdentifier).columns()[4]);
+ Awaitility.await()
+ .atMost(MAX_WAIT_IN_SECONDS, TimeUnit.SECONDS)
+ .pollInterval(WAIT_INTERVAL_IN_SECONDS, TimeUnit.SECONDS)
+ .untilAsserted(
+ () ->
+ ITUtils.assertColumn(
+ Column.of("col_5", Types.VarCharType.of(255),
"col_5_comment"),
+ tableCatalog.loadTable(tableIdentifier).columns()[4]));
Review Comment:
This introduces a second Awaitility wait immediately after waiting for
`columns().length == 5`. Since the waits are sequential, the test can now spend
up to `2 * MAX_WAIT_IN_SECONDS` before timing out. Consider folding the size
assertion and `ITUtils.assertColumn(...)` into a single `untilAsserted` (or
await only the `assertColumn`, which implicitly requires the 5th column to
exist) to keep the total timeout bounded by `MAX_WAIT_IN_SECONDS`.
--
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]
