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]

Reply via email to