Copilot commented on code in PR #7206:
URL: https://github.com/apache/ignite-3/pull/7206#discussion_r2618790508
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/prepare/PrepareServiceImplTest.java:
##########
@@ -471,11 +473,42 @@ public void planUpdatesForNonCachedTable() {
// cached table
service.statisticsChanged(table2.id());
- Awaitility.await()
- .atMost(Duration.ofMillis(2 * PLAN_UPDATER_REFRESH_PERIOD))
- .until(
- () ->
!plan2.equals(await(service.prepareAsync(parse(insertQuery),
operationContext().build())))
- );
+ runScheduledTasks();
+
+ assertNotSame(plan2, service.prepareAsync(parse(insertQuery),
operationContext().build()));
Review Comment:
The assertNotSame call is comparing a QueryPlan object with a
CompletableFuture<QueryPlan> object. The prepareAsync method returns a
CompletableFuture, so you need to await the result before comparing. This
should be:
assertNotSame(plan2, await(service.prepareAsync(parse(insertQuery),
operationContext().build())));
```suggestion
assertNotSame(plan2, await(service.prepareAsync(parse(insertQuery),
operationContext().build())));
```
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/prepare/PrepareServiceImplTest.java:
##########
@@ -317,33 +339,20 @@ public void prepareParamInPredicateAllTypes(NativeType
nativeType, int precision
}
@Test
- public void timeoutedPlanShouldBeRemovedFromCache() throws
InterruptedException {
+ public void timedoutPlanShouldBeRemovedFromCache() {
Review Comment:
The method name should be "timedOutPlanShouldBeRemovedFromCache" (camelCase
with "timed out" as two words). "Timedout" is not the correct form - "timed
out" is the correct past tense of "time out".
```suggestion
public void timedOutPlanShouldBeRemovedFromCache() {
```
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/prepare/PrepareServiceImplTest.java:
##########
@@ -417,17 +425,11 @@ public void statisticUpdatesChangePlans() {
service.statisticsChanged(table.id());
- Awaitility.await()
- .atMost(Duration.ofMillis(2 * PLAN_UPDATER_REFRESH_PERIOD))
- .until(
- () ->
!selectPlan.equals(await(service.prepareAsync(parse(selectQuery),
operationContext().build())))
- );
+ // Run update plan task.
+ runScheduledTasks();
- Awaitility.await()
- .atMost(Duration.ofMillis(2 * PLAN_UPDATER_REFRESH_PERIOD))
- .until(
- () ->
!insertPlan.equals(await(service.prepareAsync(parse(insertQuery),
operationContext().build())))
- );
+ assertNotSame(selectPlan, service.prepareAsync(parse(selectQuery),
operationContext().build()));
+ assertNotSame(insertPlan, service.prepareAsync(parse(insertQuery),
operationContext().build()));
Review Comment:
The assertNotSame calls are comparing QueryPlan objects with
CompletableFuture<QueryPlan> objects. The prepareAsync method returns a
CompletableFuture, so you need to await the result before comparing. This
should be:
assertNotSame(selectPlan, await(service.prepareAsync(parse(selectQuery),
operationContext().build())));
assertNotSame(insertPlan, await(service.prepareAsync(parse(insertQuery),
operationContext().build())));
--
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]