From 62d234bb263b584785fd1a444a7832991ae69a64 Mon Sep 17 00:00:00 2001
From: Andrew Steinborn <git@steinborn.me>
Date: Sat, 24 Jul 2021 18:15:14 -0400
Subject: [PATCH] Improve event system tests

Sleeping in a test is considered bad form. Replace the delay with checks to ensure that threads are spawned appropriately and a new test to ensure that continuation threads do run before other listeners.
---
 .../proxy/event/EventTest.java                | 57 +++++++++++++++----
 1 file changed, 45 insertions(+), 12 deletions(-)

diff --git a/proxy/src/test/java/com/velocitypowered/proxy/event/EventTest.java b/proxy/src/test/java/com/velocitypowered/proxy/event/EventTest.java
index aec0fa12e..4a066ef68 100644
--- a/proxy/src/test/java/com/velocitypowered/proxy/event/EventTest.java
+++ b/proxy/src/test/java/com/velocitypowered/proxy/event/EventTest.java
@@ -21,6 +21,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import com.google.common.reflect.TypeToken;
+import com.velocitypowered.api.event.AwaitingEventExecutor;
 import com.velocitypowered.api.event.Continuation;
 import com.velocitypowered.api.event.EventTask;
 import com.velocitypowered.api.event.PostOrder;
@@ -36,6 +37,7 @@ import org.junit.jupiter.api.TestInstance;
 @TestInstance(TestInstance.Lifecycle.PER_CLASS)
 public class EventTest {
 
+  public static final String CONTINUATION_TEST_THREAD_NAME = "Continuation test thread";
   private final VelocityEventManager eventManager =
       new VelocityEventManager(new FakePluginManager());
 
@@ -51,6 +53,10 @@ public class EventTest {
     assertTrue(thread.getName().contains("Velocity Async Event Executor"));
   }
 
+  static void assertContinuationThread(final Thread thread) {
+    assertEquals(CONTINUATION_TEST_THREAD_NAME, thread.getName());
+  }
+
   private void handleMethodListener(final Object listener) throws Exception {
     eventManager.register(FakePluginManager.PLUGIN_A, listener);
     try {
@@ -87,6 +93,37 @@ public class EventTest {
     assertTrue(listenerBInvoked.get() < listenerCInvoked.get(), "Listener C invoked before B!");
   }
 
+  @Test
+  void listenerOrderPreservedWithContinuation() throws Exception {
+    final AtomicLong listenerAInvoked = new AtomicLong();
+    final AtomicLong listenerBInvoked = new AtomicLong();
+    final AtomicLong listenerCInvoked = new AtomicLong();
+
+    eventManager.register(FakePluginManager.PLUGIN_A, TestEvent.class, event -> {
+      listenerAInvoked.set(System.nanoTime());
+    });
+    eventManager.register(FakePluginManager.PLUGIN_B, TestEvent.class,
+        (AwaitingEventExecutor<TestEvent>) event -> EventTask.withContinuation(continuation -> {
+          new Thread(() -> {
+            listenerBInvoked.set(System.nanoTime());
+            continuation.resume();
+          }).start();
+        }));
+    eventManager.register(FakePluginManager.PLUGIN_A, TestEvent.class, event -> {
+      listenerCInvoked.set(System.nanoTime());
+    });
+
+    try {
+      eventManager.fire(new TestEvent()).get();
+    } finally {
+      eventManager.unregisterListeners(FakePluginManager.PLUGIN_A);
+    }
+
+    // Check that the order is A < B < C. Check only that A < B and B < C as B < C and A < B => A < C.
+    assertTrue(listenerAInvoked.get() < listenerBInvoked.get(), "Listener B invoked before A!");
+    assertTrue(listenerBInvoked.get() < listenerCInvoked.get(), "Listener C invoked before B!");
+  }
+
   @Test
   void testAlwaysAsync() throws Exception {
     final AlwaysAsyncListener listener = new AlwaysAsyncListener();
@@ -129,6 +166,7 @@ public class EventTest {
     handleMethodListener(listener);
     assertAsyncThread(listener.threadA);
     assertAsyncThread(listener.threadB);
+    assertContinuationThread(listener.threadBCustom);
     assertAsyncThread(listener.threadC);
     assertEquals(2, listener.value.get());
   }
@@ -137,6 +175,7 @@ public class EventTest {
 
     @MonotonicNonNull Thread threadA;
     @MonotonicNonNull Thread threadB;
+    @MonotonicNonNull Thread threadBCustom;
     @MonotonicNonNull Thread threadC;
 
     final AtomicInteger value = new AtomicInteger();
@@ -148,14 +187,10 @@ public class EventTest {
         value.incrementAndGet();
         threadB = Thread.currentThread();
         new Thread(() -> {
-          try {
-            Thread.sleep(100);
-          } catch (InterruptedException e) {
-            e.printStackTrace();
-          }
+          threadBCustom = Thread.currentThread();
           value.incrementAndGet();
           continuation.resume();
-        }).start();
+        }, CONTINUATION_TEST_THREAD_NAME).start();
       });
     }
 
@@ -206,6 +241,7 @@ public class EventTest {
     handleMethodListener(listener);
     assertAsyncThread(listener.threadA);
     assertAsyncThread(listener.threadB);
+    assertContinuationThread(listener.threadBCustom);
     assertAsyncThread(listener.threadC);
     assertEquals(3, listener.result.get());
   }
@@ -214,6 +250,7 @@ public class EventTest {
 
     @MonotonicNonNull Thread threadA;
     @MonotonicNonNull Thread threadB;
+    @MonotonicNonNull Thread threadBCustom;
     @MonotonicNonNull Thread threadC;
 
     final AtomicInteger result = new AtomicInteger();
@@ -229,14 +266,10 @@ public class EventTest {
     void resumeFromCustomThread(TestEvent event, Continuation continuation) {
       threadB = Thread.currentThread();
       new Thread(() -> {
-        try {
-          Thread.sleep(100);
-        } catch (InterruptedException e) {
-          e.printStackTrace();
-        }
+        threadBCustom = Thread.currentThread();
         result.incrementAndGet();
         continuation.resume();
-      }).start();
+      }, CONTINUATION_TEST_THREAD_NAME).start();
     }
 
     @Subscribe(order = PostOrder.LAST)