Skip to content

[Tests] Add coverage for PlayerLoadTask and PlayerUnloadTask event dispatch and onCancel paths #124

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
DiamondDagger590 wants to merge 1 commit into from
Closed

[Tests] Add coverage for PlayerLoadTask and PlayerUnloadTask event dispatch and onCancel paths #124

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 74 additions & 0 deletions src/test/java/com/diamonddagger590/mccore/task/player/PlayerLoadTaskTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@
import org.mockito.junit.jupiter.MockitoSettings;
import org.mockito.quality.Strictness;

import org.bukkit.plugin.PluginManager;
import org.mockito.ArgumentCaptor;

import java.sql.Connection;
import java.sql.SQLException;
import java.time.Clock;
Expand All @@ -34,6 +37,8 @@
import java.util.UUID;
import java.util.concurrent.CompletableFuture;

import com.diamonddagger590.mccore.event.player.PlayerLoadEvent;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
Expand All @@ -43,6 +48,7 @@
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.mockStatic;
Expand Down Expand Up @@ -293,4 +299,72 @@ void onIntervalComplete_runtimeException_propagates() {
// runLoadPlayerTask only catches SQLException, so RuntimeException propagates
org.junit.jupiter.api.Assertions.assertThrows(RuntimeException.class, task::onIntervalComplete);
}

@Test
@DisplayName("Given successful load, when event dispatch Runnable executes, then PlayerLoadEvent is fired with correct player")
void onPlayerLoadSuccessfully_eventDispatchRunnable_firesPlayerLoadEvent() throws SQLException {
when(mockPlayerManager.getPlayer(playerUUID)).thenReturn(Optional.empty());
when(mockDatabase.getConnection()).thenReturn(mockConnection);
when(mockCorePlayer.useMutex()).thenReturn(false);
doNothing().when(mockScheduler).cancelTask(anyInt());

PluginManager mockPluginManager = mock(PluginManager.class);
bukkitStatic.when(Bukkit::getPluginManager).thenReturn(mockPluginManager);

ArgumentCaptor<Runnable> runnableCaptor = ArgumentCaptor.forClass(Runnable.class);
when(mockScheduler.scheduleSyncDelayedTask(eq(mockPlugin), runnableCaptor.capture())).thenReturn(1);

loadPlayerResult = true;
PlayerLoadTask task = createTask();
task.onIntervalComplete();

assertTrue(task.getResult().isDone());
assertTrue(task.getResult().join());

runnableCaptor.getValue().run();

ArgumentCaptor<PlayerLoadEvent> eventCaptor = ArgumentCaptor.forClass(PlayerLoadEvent.class);
verify(mockPluginManager).callEvent(eventCaptor.capture());
assertSame(mockCorePlayer, eventCaptor.getValue().getCorePlayer());
}

@Test
@DisplayName("Given task completed successfully, when run triggers onCancel, then result remains true")
void onCancel_alreadyCompleted_resultRemainsTrue() throws SQLException {

@github-actions github-actions Bot Sep 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test asserts task.getResult().join() is still true after run() triggers onCancel(), but CompletableFuture.complete() is a no-op once already completed — PlayerLoadTask.onPlayerLoadSuccessfully() already called result.complete(true) before onCancel() runs. So even if the if (!completed) guard in PlayerLoadTask.onCancel() (lines 105-109) were deleted and onPlayerLoadFail() called unconditionally, result.complete(false) would also just no-op and this assertion would still pass. The test doesn't actually exercise the guard it's meant to verify.

Suggest asserting via a spy instead, e.g.:

PlayerLoadTask spyTask = spy(task);
spyTask.run();
verify(spyTask, never()).onPlayerLoadFail();

when(mockPlayerManager.getPlayer(playerUUID)).thenReturn(Optional.empty());
when(mockDatabase.getConnection()).thenReturn(mockConnection);
when(mockCorePlayer.useMutex()).thenReturn(false);
doNothing().when(mockScheduler).cancelTask(anyInt());
when(mockScheduler.scheduleSyncDelayedTask(eq(mockPlugin), any(Runnable.class))).thenReturn(1);

loadPlayerResult = true;
PlayerLoadTask task = createTask();
task.onIntervalComplete();

assertTrue(task.getResult().isDone());
assertTrue(task.getResult().join());

task.run();

assertTrue(task.getResult().isDone());
assertTrue(task.getResult().join());
}

@Test
@DisplayName("Given connection close throws SQLException, when onIntervalComplete, then exception is caught gracefully")
void onIntervalComplete_sqlExceptionOnClose_caughtGracefully() throws SQLException {
when(mockPlayerManager.getPlayer(playerUUID)).thenReturn(Optional.empty());
when(mockDatabase.getConnection()).thenReturn(mockConnection);
when(mockCorePlayer.useMutex()).thenReturn(false);
doNothing().when(mockScheduler).cancelTask(anyInt());
when(mockScheduler.scheduleSyncDelayedTask(eq(mockPlugin), any(Runnable.class))).thenReturn(1);
doThrow(new SQLException("Connection close failed")).when(mockConnection).close();

loadPlayerResult = true;
PlayerLoadTask task = createTask();
task.onIntervalComplete();

assertTrue(task.getResult().isDone());
assertTrue(task.getResult().join());
}
}
72 changes: 72 additions & 0 deletions src/test/java/com/diamonddagger590/mccore/task/player/PlayerUnloadTaskTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@
import org.mockito.junit.jupiter.MockitoSettings;
import org.mockito.quality.Strictness;

import org.bukkit.plugin.PluginManager;
import org.mockito.ArgumentCaptor;

import java.sql.Connection;
import java.sql.SQLException;
import java.time.Clock;
Expand All @@ -33,6 +36,8 @@
import java.util.Optional;
import java.util.UUID;

import com.diamonddagger590.mccore.event.player.PlayerUnloadEvent;

import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertSame;
Expand All @@ -42,7 +47,9 @@
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.mockStatic;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
Expand Down Expand Up @@ -283,4 +290,69 @@ void onIntervalComplete_runtimeException_propagates() {
// runUnloadPlayerTask only catches SQLException, so RuntimeException propagates
org.junit.jupiter.api.Assertions.assertThrows(RuntimeException.class, task::onIntervalComplete);
}

@Test
@DisplayName("Given successful unload, when event dispatch Runnable executes, then PlayerUnloadEvent is fired with correct player")
void onPlayerUnloadSuccessfully_eventDispatchRunnable_firesPlayerUnloadEvent() throws SQLException {
when(mockDatabase.getConnection()).thenReturn(mockConnection);
when(mockCorePlayer.useMutex()).thenReturn(false);
doNothing().when(mockScheduler).cancelTask(anyInt());

PluginManager mockPluginManager = mock(PluginManager.class);
bukkitStatic.when(Bukkit::getPluginManager).thenReturn(mockPluginManager);

ArgumentCaptor<Runnable> runnableCaptor = ArgumentCaptor.forClass(Runnable.class);
when(mockScheduler.scheduleSyncDelayedTask(eq(mockPlugin), runnableCaptor.capture())).thenReturn(1);

unloadPlayerResult = true;
PlayerUnloadTask task = createTask();
task.onIntervalComplete();

assertTrue(task.getResult().isDone());
assertTrue(task.getResult().join());

runnableCaptor.getValue().run();

ArgumentCaptor<PlayerUnloadEvent> eventCaptor = ArgumentCaptor.forClass(PlayerUnloadEvent.class);
verify(mockPluginManager).callEvent(eventCaptor.capture());
assertSame(mockCorePlayer, eventCaptor.getValue().getCorePlayer());
}

@Test
@DisplayName("Given task completed successfully, when run triggers onCancel, then result remains true")
void onCancel_alreadyCompleted_resultRemainsTrue() throws SQLException {

@github-actions github-actions Bot Sep 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same structural issue as its PlayerLoadTaskTest counterpart: this asserts getResult().join() is true after onCancel() fires, but PlayerUnloadTask.onPlayerUnloadFail() (lines 123-124) just calls the already-no-op result.complete(false) once the future is completed. This can't detect a removed/inverted if (!completed) guard in PlayerUnloadTask.onCancel() (lines 90-94).

Suggest verifying via a spy that onPlayerUnloadFail() is never invoked on the already-completed task, rather than relying on the unchanged getResult() value.

when(mockDatabase.getConnection()).thenReturn(mockConnection);
when(mockCorePlayer.useMutex()).thenReturn(false);
doNothing().when(mockScheduler).cancelTask(anyInt());
when(mockScheduler.scheduleSyncDelayedTask(eq(mockPlugin), any(Runnable.class))).thenReturn(1);

unloadPlayerResult = true;
PlayerUnloadTask task = createTask();
task.onIntervalComplete();

assertTrue(task.getResult().isDone());
assertTrue(task.getResult().join());

task.run();

assertTrue(task.getResult().isDone());
assertTrue(task.getResult().join());
}

@Test
@DisplayName("Given connection close throws SQLException, when onIntervalComplete, then exception is caught gracefully")
void onIntervalComplete_sqlExceptionOnClose_caughtGracefully() throws SQLException {
when(mockDatabase.getConnection()).thenReturn(mockConnection);
when(mockCorePlayer.useMutex()).thenReturn(false);
doNothing().when(mockScheduler).cancelTask(anyInt());
when(mockScheduler.scheduleSyncDelayedTask(eq(mockPlugin), any(Runnable.class))).thenReturn(1);
doThrow(new SQLException("Connection close failed")).when(mockConnection).close();

unloadPlayerResult = true;
PlayerUnloadTask task = createTask();
task.onIntervalComplete();

assertTrue(task.getResult().isDone());
assertTrue(task.getResult().join());
}
}
Loading