Merge pull request jakartaee#362 from KyleAure/fix-tck-errors

tck/src/main/java/ee/jakarta/tck/concurrent/api/Trigger/TriggerTests.java

@@ -16,8 +16,6 @@
 
 package ee.jakarta.tck.concurrent.api.Trigger;
 
-import static org.junit.jupiter.api.Assertions.assertFalse;
-
 import java.util.Date;
 import java.util.concurrent.Future;
 import java.util.concurrent.ScheduledFuture;
@@ -63,13 +61,19 @@ public void reset() {
     public void triggerGetNextRunTimeTest() throws Exception {
         Future<?> result = scheduledExecutor.schedule(new CounterRunnableTask(),
                 new CommonTriggers.TriggerFixedRate(new Date(), TestConstants.pollInterval.toMillis()));
-
-        assertFalse(StaticCounter.getCount() == 0, "The first trigger is too fast.");
+
+        /**
+         * This test is susceptible to timing issues depending on the hardware running the test.
+         * Therefore, if immediately after scheduling this task the count is already > 0
+         * Then, be more liberal with the range of acceptable values that signify a passing test.
+         */
+        int rangeOffset = StaticCounter.getCount() == 0 ? 2 : 3;
 
         try {
             Wait.sleep(TestConstants.waitTimeout);
-            Assertions.assertBetween(StaticCounter.getCount(), TestConstants.pollsPerTimeout - 2,
-                    TestConstants.pollsPerTimeout + 2);
+            Assertions.assertBetween(StaticCounter.getCount(),
+                    TestConstants.pollsPerTimeout - rangeOffset,
+                    TestConstants.pollsPerTimeout + rangeOffset);
         } finally {
             Wait.waitForTaskComplete(result);
         }

tck/src/main/java/ee/jakarta/tck/concurrent/common/counter/CounterSingleton.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2013, 2022 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2013, 2023 Oracle and/or its affiliates. All rights reserved.
  *
  * This program and the accompanying materials are made available under the
  * terms of the Eclipse Public License v. 2.0, which is available at
@@ -16,22 +16,28 @@
 
 package ee.jakarta.tck.concurrent.common.counter;
 
+import java.util.concurrent.atomic.AtomicInteger;
+
 import jakarta.ejb.Singleton;
 
 @Singleton
 public class CounterSingleton implements CounterInterface {
 
-    private int count = 0;
+    /**
+     * Test will spawn a thread that will increment count, and JUnit will
+     * spawn a timeout thread to get the count.
+     */
+    private AtomicInteger count = new AtomicInteger();
 
     public void inc() {
-        count++;
+        count.incrementAndGet();
     }
 
     public int getCount() {
-        return count;
+        return count.get();
     }
 
     public void reset() {
-        count = 0;
+        count.set(0);
     }
 }

tck/src/main/java/ee/jakarta/tck/concurrent/common/fixed/counter/StaticCounter.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2013, 2022 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2013, 2023 Oracle and/or its affiliates. All rights reserved.
  *
  * This program and the accompanying materials are made available under the
  * terms of the Eclipse Public License v. 2.0, which is available at
@@ -54,13 +54,4 @@ public static void waitTill(final int expected, final String message) {
             }
         }, message);
     }
-
-    public static void waitTillSurpassed(final int expected) {
-        assertTimeoutPreemptively(TestConstants.waitTimeout, () -> {
-            for (; expected <= StaticCounter.getCount(); Wait.sleep(TestConstants.pollInterval)) {
-                //empty
-            }
-        });
-    }
-
 }

tck/src/main/java/ee/jakarta/tck/concurrent/common/transaction/CancelledTransactedTask.java

@@ -16,8 +16,6 @@
 
 package ee.jakarta.tck.concurrent.common.transaction;
 
-import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively;
-
 import java.sql.Connection;
 import java.sql.PreparedStatement;
 import java.util.concurrent.atomic.AtomicBoolean;
@@ -42,12 +40,18 @@ public CancelledTransactedTask(final String sqlTemplate) {
         this.sqlTemplate = sqlTemplate;
     }
 
+    /**
+     * Avoid using assertTimeoutPreemptively here as it could lead to a leaked thread
+     * generated by JUnit if the task takes close to the waitTimeout period to complete.
+     */
     private void waitForRun() {
-        assertTimeoutPreemptively(TestConstants.waitTimeout, () -> {
-            for (; !runQuery.get(); Wait.sleep(TestConstants.pollInterval)) {
-                //empty
+        long startTime = System.currentTimeMillis();
+        while (!runQuery.get()) {
+            if (System.currentTimeMillis() > startTime + TestConstants.waitTimeout.toMillis()) {
+                throw new RuntimeException("runQuery flag was not set to true before timeout.");
             }
-        });
+            Wait.sleep(TestConstants.pollInterval);
+        }
     }
 
     @Override

tck/src/main/java/ee/jakarta/tck/concurrent/framework/junit/extensions/Wait.java

@@ -1,3 +1,18 @@
+/*
+ * Copyright (c) 2023 Oracle and/or its affiliates. All rights reserved.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v. 2.0, which is available at
+ * http://www.eclipse.org/legal/epl-2.0.
+ *
+ * This Source Code may also be made available under the following Secondary
+ * Licenses when the conditions for such availability set forth in the
+ * Eclipse Public License v. 2.0 are satisfied: GNU General Public License,
+ * version 2 with the GNU Classpath Exception, which is available at
+ * https://www.gnu.org/software/classpath/license.html.
+ *
+ * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
+ */
 package ee.jakarta.tck.concurrent.framework.junit.extensions;
 
 import static org.junit.jupiter.api.Assertions.assertThrows;
@@ -6,6 +21,7 @@
 import java.time.Duration;
 import java.util.concurrent.Future;
 
+import ee.jakarta.tck.concurrent.common.counter.CounterInterface;
 import ee.jakarta.tck.concurrent.common.managed.task.listener.ListenerEvent;
 import ee.jakarta.tck.concurrent.common.managed.task.listener.ManagedTaskListenerImpl;
 import ee.jakarta.tck.concurrent.common.transaction.CancelledTransactedTask;
@@ -90,6 +106,13 @@ public static <T extends Throwable> void waitTillFutureThrowsException(final Fut
         });
     }
 
+    /**
+     * Calls future.cancel(true), and then waits for future.done() to return true,
+     * but will timeout after {@link TestConstants#waitTimeout}, and will be polled every
+     * {@link TestConstants#pollInterval}
+     *
+     * @param future - the future to wait for
+     */
     public static void waitCancelFuture(final Future<?> future) {
         assertTimeoutPreemptively(TestConstants.waitTimeout, () -> {
             for (future.cancel(true); !future.isDone(); sleep(TestConstants.pollInterval)) {
@@ -113,13 +136,36 @@ public static void waitTillThreadFinish(final Thread thread) {
         });
     }
 
+    /**
+     * Waits for task to report the transaction has begun, but will timeout after
+     * {@link TestConstants#waitTimeout}, and will be polled every
+     * {@link TestConstants#pollInterval}
+     *
+     * @param task - the task to wait for
+     */
     public static void waitForTransactionBegan(final CancelledTransactedTask task) {
         assertTimeoutPreemptively(TestConstants.waitTimeout, () -> {
             for (; !task.getBeginTransaction().get(); sleep(TestConstants.pollInterval)) {
                 //empty
             }
         });
     }
+
+    /**
+     * Waits for a counter to report an expected value, but will timeout after
+     * {@link TestConstants#waitTimeout}, and will be polled every
+     * {@link TestConstants#pollInterval}
+     *
+     * @param counter
+     * @param expected
+     */
+    public static void waitForCounter(final CounterInterface counter, final int expected) {
+        assertTimeoutPreemptively(TestConstants.waitTimeout, () -> {
+            for (; expected != counter.getCount(); Wait.sleep(TestConstants.pollInterval)) {
+                //empty
+            }
+        });
+    }
 
     public static void sleep(final Duration time) {
         try {

tck/src/main/java/ee/jakarta/tck/concurrent/spec/ManagedExecutorService/managed/forbiddenapi/ForbiddenAPIServletTests.java

@@ -51,14 +51,14 @@ public void testAwaitTermination() {
     @Assertion(id = "SPEC:23 SPEC:24 SPEC:24.2", strategy = "Test basic function for ManagedExecutorService: isShutdown")
     public void testIsShutdown() {
         assertThrows(IllegalStateException.class, () -> {
-
+            mes.isShutdown();
         });
     }
 
     @Assertion(id = "SPEC:23 SPEC:24 SPEC:24.3", strategy = "Test basic function for ManagedExecutorService: isTerminated")
     public void testIsTerminated() {
         assertThrows(IllegalStateException.class, () -> {
-
+            mes.isTerminated();
         });
     }
 

tck/src/main/java/ee/jakarta/tck/concurrent/spec/ManagedScheduledExecutorService/inheritedapi/InheritedAPIFullTests.java

@@ -35,8 +35,7 @@ public static EnterpriseArchive createDeployment() {
         JavaArchive inheritedJAR = ShrinkWrap.create(JavaArchive.class, "inheritedapi.jar")
                 .addClasses(InheritedAPIFullTests.class, CounterEJBProvider.class, TestEjb.class,
                         TestEjbInterface.class)
-                .addPackages(true, PACKAGE.TASKS.getPackageName(), PACKAGE.COUNTER.getPackageName(),
-                        PACKAGE.FIXED_COUNTER.getPackageName())
+                .addPackages(true, PACKAGE.TASKS.getPackageName(), PACKAGE.COUNTER.getPackageName())
                 .addAsServiceProvider(EJBJNDIProvider.class, CounterEJBProvider.FullProvider.class);
 
         EnterpriseArchive ear = ShrinkWrap.create(EnterpriseArchive.class, "inheritedapi.ear")

tck/src/main/java/ee/jakarta/tck/concurrent/spec/ManagedScheduledExecutorService/inheritedapi/TestEjb.java

@@ -31,7 +31,6 @@
 
 import ee.jakarta.tck.concurrent.common.counter.CounterInterface;
 import ee.jakarta.tck.concurrent.common.counter.CounterRunnableTask;
-import ee.jakarta.tck.concurrent.common.fixed.counter.StaticCounter;
 import ee.jakarta.tck.concurrent.common.tasks.CommonTasks;
 import ee.jakarta.tck.concurrent.framework.EJBJNDIProvider;
 import ee.jakarta.tck.concurrent.framework.TestConstants;
@@ -70,7 +69,7 @@ public void testApiExecute() {
         try {
             EJBJNDIProvider nameProvider = ServiceLoader.load(EJBJNDIProvider.class).findFirst().orElseThrow();
             scheduledExecutor.execute(new CounterRunnableTask(nameProvider.getEJBJNDIName()));
-            StaticCounter.waitTillSurpassed(1);
+            Wait.waitForCounter(counter, 1);
         } finally {
             counter.reset();
         }