Skip to content

Standardize test packages #367

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

Merged
BenCodez merged 1 commit into from
Oct 16, 2026
Merged

Standardize test packages #367

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
2 changes: 2 additions & 0 deletions AGENTS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ mvn -B -f AdvancedCore/pom.xml test
mvn -B -f AdvancedCore/pom.xml package
```

All test classes and test-only helpers must live under `com.bencodez.advancedcore.tests` or its subpackages, with matching directories under `AdvancedCore/src/test/java`. Keep tests out of production packages. Preserve production visibility when relocating tests; use narrowly scoped test-only reflective helpers when package-private access is required. Preserve test discovery and the package-phase artifact tests.

Confirm the current workflow and POM before relying on these commands. Do not use deployment, install, or developer-copy profiles for routine validation. Verify that the package invocation produced a fresh JAR, record discovered test counts, and run `git diff --check`.

## Architecture and compatibility boundaries
Expand Down
16 changes: 12 additions & 4 deletions ...ncedCoreUuidTabCompletionRefreshTest.java → ...ncedCoreUuidTabCompletionRefreshTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
package com.bencodez.advancedcore;
package com.bencodez.advancedcore.tests;

import com.bencodez.advancedcore.tests.support.ReflectiveAccess;

import com.bencodez.advancedcore.AdvancedCoreConfigOptions;
import com.bencodez.advancedcore.AdvancedCorePlugin;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.mockito.ArgumentMatchers.any;
Expand Down Expand Up @@ -46,15 +51,18 @@ void storageEnumerationIsDeferredAndTheLatestRefreshWins() {
return null;
}).when(scheduler).runTask(any(), any());

AdvancedCorePlugin.UuidTabCompletionRefresh refresh = new AdvancedCorePlugin.UuidTabCompletionRefresh();
Class<?> refreshType = ReflectiveAccess.type(AdvancedCorePlugin.class.getName() + "$UuidTabCompletionRefresh");
Object refresh = ReflectiveAccess.construct(refreshType, new Class<?>[0]);
try (MockedStatic<Bukkit> bukkit = org.mockito.Mockito.mockStatic(Bukkit.class)) {
bukkit.when(Bukkit::getOnlinePlayers).thenReturn(List.of());

refresh.request(plugin, handle);
ReflectiveAccess.invoke(refreshType, refresh, "request",
new Class<?>[] { AdvancedCorePlugin.class, TabCompleteHandle.class }, plugin, handle);
verify(users, never()).getAllUUIDs();
assertEquals(1, workers.size(), "request must enqueue, not enumerate on the caller thread");

refresh.request(plugin, handle);
ReflectiveAccess.invoke(refreshType, refresh, "request",
new Class<?>[] { AdvancedCorePlugin.class, TabCompleteHandle.class }, plugin, handle);
assertEquals(1, workers.size(), "overlapping requests must be coalesced");

workers.remove(0).run();
Expand Down
4 changes: 3 additions & 1 deletion ...api/command/PlayerCommandHandlerTest.java → ...api/command/PlayerCommandHandlerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
package com.bencodez.advancedcore.api.command;
package com.bencodez.advancedcore.tests.api.command;

import com.bencodez.advancedcore.api.command.PlayerCommandHandler;

import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
Expand Down
15 changes: 12 additions & 3 deletions ...ript/JavascriptEngineConcurrencyTest.java → ...ript/JavascriptEngineConcurrencyTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
package com.bencodez.advancedcore.api.javascript;
package com.bencodez.advancedcore.tests.api.javascript;

import com.bencodez.advancedcore.tests.support.ReflectiveAccess;

import com.bencodez.advancedcore.api.javascript.JavascriptEngine;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
Expand Down Expand Up @@ -33,11 +37,11 @@ void sharedEngineBindingsAndEvaluationAreAtomic() throws Exception {
try {
Future<Object> first = executor.submit(() -> {
start.await();
return JavascriptEngine.evaluateWithBindings(engine, "value", Map.of("value", "first"));
return evaluate(engine, Map.of("value", "first"));
});
Future<Object> second = executor.submit(() -> {
start.await();
return JavascriptEngine.evaluateWithBindings(engine, "value", Map.of("value", "second"));
return evaluate(engine, Map.of("value", "second"));
});

start.countDown();
Expand All @@ -49,6 +53,11 @@ void sharedEngineBindingsAndEvaluationAreAtomic() throws Exception {
}
}

private static Object evaluate(javax.script.ScriptEngine engine, Map<String, Object> bindings) {
return ReflectiveAccess.invoke(JavascriptEngine.class, null, "evaluateWithBindings",
new Class<?>[] { javax.script.ScriptEngine.class, String.class, Map.class }, engine, "value", bindings);
}

private static final class LockCheckingScriptEngine extends AbstractScriptEngine {
private final AtomicBoolean allEvaluationsHeldLock = new AtomicBoolean(true);

Expand Down
47 changes: 29 additions & 18 deletions ...ript/JavascriptPlaceholderBinderTest.java → ...ript/JavascriptPlaceholderBinderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
package com.bencodez.advancedcore.api.javascript;
package com.bencodez.advancedcore.tests.api.javascript;

import com.bencodez.advancedcore.tests.support.ReflectiveAccess;

import com.bencodez.advancedcore.api.javascript.JavascriptPlaceholderBinder;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
Expand All @@ -10,13 +14,20 @@
import org.junit.jupiter.api.Test;

class JavascriptPlaceholderBinderTest {
private static String bind(String expression, java.util.function.Function<String, String> resolver,
java.util.function.BiConsumer<String, Object> bindings) {
return ReflectiveAccess.invoke(JavascriptPlaceholderBinder.class, null, "bind",
new Class<?>[] { String.class, java.util.function.Function.class, java.util.function.BiConsumer.class },
expression, resolver, bindings);
}


@Test
void barePlaceholderIsBoundAsData() {
HashMap<String, Object> bindings = new HashMap<>();
String injection = "Bukkit.dispatchCommand(Console, 'op attacker')";

String prepared = JavascriptPlaceholderBinder.bind("%value% == true", ignored -> injection, bindings::put);
String prepared = bind("%value% == true", ignored -> injection, bindings::put);

assertEquals("__advancedCorePlaceholder0 == true", prepared);
assertEquals(injection, bindings.get("__advancedCorePlaceholder0"));
Expand All @@ -27,7 +38,7 @@ void barePlaceholderIsBoundAsData() {
void numericAndBooleanValuesKeepExpressionTypes() {
HashMap<String, Object> bindings = new HashMap<>();

String prepared = JavascriptPlaceholderBinder.bind("%count% > 1 && %allowed% == true",
String prepared = bind("%count% > 1 && %allowed% == true",
token -> token.equals("%count%") ? "2.5" : "true", bindings::put);

assertEquals("__advancedCorePlaceholder0 > 1 && __advancedCorePlaceholder1 == true", prepared);
Expand All @@ -39,7 +50,7 @@ void numericAndBooleanValuesKeepExpressionTypes() {
void ordinaryCompactModuloExpressionIsNotTreatedAsAPlaceholder() {
HashMap<String, Object> bindings = new HashMap<>();

String prepared = JavascriptPlaceholderBinder.bind("10%3%2", token -> token, bindings::put);
String prepared = bind("10%3%2", token -> token, bindings::put);

assertEquals("10%3%2", prepared);
assertTrue(bindings.isEmpty());
Expand All @@ -49,7 +60,7 @@ void ordinaryCompactModuloExpressionIsNotTreatedAsAPlaceholder() {
void unresolvedPercentTokenRemainsUntouched() {
HashMap<String, Object> bindings = new HashMap<>();

String prepared = JavascriptPlaceholderBinder.bind("'%unknown%'", token -> token, bindings::put);
String prepared = bind("'%unknown%'", token -> token, bindings::put);

assertEquals("'%unknown%'", prepared);
assertTrue(bindings.isEmpty());
Expand All @@ -59,7 +70,7 @@ void unresolvedPercentTokenRemainsUntouched() {
void compactModuloCanAppearBesideAResolvedPlaceholder() {
HashMap<String, Object> bindings = new HashMap<>();

String prepared = JavascriptPlaceholderBinder.bind("10%3%2 + %value%",
String prepared = bind("10%3%2 + %value%",
token -> token.equals("%value%") ? "4" : token, bindings::put);

assertEquals("10%3%2 + __advancedCorePlaceholder0", prepared);
Expand All @@ -70,7 +81,7 @@ void compactModuloCanAppearBesideAResolvedPlaceholder() {
void moduloCandidateCannotHideAnOverlappingResolvedPlaceholder() {
HashMap<String, Object> bindings = new HashMap<>();

String prepared = JavascriptPlaceholderBinder.bind("10%3 == 1 && %value% == true",
String prepared = bind("10%3 == 1 && %value% == true",
token -> token.equals("%value%") ? "true" : token, bindings::put);

assertEquals("10%3 == 1 && __advancedCorePlaceholder0 == true", prepared);
Expand All @@ -81,7 +92,7 @@ void moduloCandidateCannotHideAnOverlappingResolvedPlaceholder() {
void placeholderApiStyleTokenMayContainWhitespace() {
HashMap<String, Object> bindings = new HashMap<>();

String prepared = JavascriptPlaceholderBinder.bind("%exp_argument with spaces% == true",
String prepared = bind("%exp_argument with spaces% == true",
ignored -> "true", bindings::put);

assertEquals("__advancedCorePlaceholder0 == true", prepared);
Expand All @@ -92,7 +103,7 @@ void placeholderApiStyleTokenMayContainWhitespace() {
void quotedAndEmbeddedPlaceholdersStayStrings() {
HashMap<String, Object> bindings = new HashMap<>();

String prepared = JavascriptPlaceholderBinder.bind("'%code%' === '001' && 'Hello %name%!' === 'Hello Ben!'",
String prepared = bind("'%code%' === '001' && 'Hello %name%!' === 'Hello Ben!'",
token -> token.equals("%code%") ? "001" : "Ben", bindings::put);

assertEquals("'001' === '001' && 'Hello Ben!' === 'Hello Ben!'", prepared);
Expand All @@ -104,7 +115,7 @@ void stringInjectionIsEscaped() {
HashMap<String, Object> bindings = new HashMap<>();
String injection = "'; Bukkit.dispatchCommand(Console, 'op attacker'); '";

String prepared = JavascriptPlaceholderBinder.bind("'%name%' == 'safe'", ignored -> injection, bindings::put);
String prepared = bind("'%name%' == 'safe'", ignored -> injection, bindings::put);

assertEquals("'\\'; Bukkit.dispatchCommand(Console, \\'op attacker\\'); \\'' == 'safe'", prepared);
assertTrue(bindings.isEmpty());
Expand All @@ -113,7 +124,7 @@ void stringInjectionIsEscaped() {
@Test
void templateTextAndExpressionUseDifferentContexts() {
HashMap<String, Object> bindings = new HashMap<>();
String prepared = JavascriptPlaceholderBinder.bind("`Hello %name% ${%count% + 1}`",
String prepared = bind("`Hello %name% ${%count% + 1}`",
token -> token.equals("%name%") ? "${attack()}`" : "2", bindings::put);

assertEquals("`Hello \\${attack()}\\` ${__advancedCorePlaceholder0 + 1}`", prepared);
Expand All @@ -124,7 +135,7 @@ void templateTextAndExpressionUseDifferentContexts() {
void oddBackslashBeforeTemplatePlaceholderCannotReactivateInterpolation() {
HashMap<String, Object> bindings = new HashMap<>();

String prepared = JavascriptPlaceholderBinder.bind("`\\%value%`", ignored -> "${attack()}", bindings::put);
String prepared = bind("`\\%value%`", ignored -> "${attack()}", bindings::put);

assertEquals("`\\${attack()}`", prepared);
assertTrue(bindings.isEmpty());
Expand All @@ -134,7 +145,7 @@ void oddBackslashBeforeTemplatePlaceholderCannotReactivateInterpolation() {
void evenBackslashesBeforeTemplatePlaceholderRemainLiteralAndSafe() {
HashMap<String, Object> bindings = new HashMap<>();

String prepared = JavascriptPlaceholderBinder.bind("`\\\\%value%`", ignored -> "${attack()}", bindings::put);
String prepared = bind("`\\\\%value%`", ignored -> "${attack()}", bindings::put);

assertEquals("`\\\\\\${attack()}`", prepared);
assertTrue(bindings.isEmpty());
Expand All @@ -145,7 +156,7 @@ void oddBackslashBeforeStringPlaceholderCannotReactivateQuote() {
HashMap<String, Object> bindings = new HashMap<>();
String injection = "'; attack(); '";

String prepared = JavascriptPlaceholderBinder.bind("'\\%value%'", ignored -> injection, bindings::put);
String prepared = bind("'\\%value%'", ignored -> injection, bindings::put);

assertEquals("'\\'; attack(); \\''", prepared);
assertTrue(bindings.isEmpty());
Expand All @@ -155,7 +166,7 @@ void oddBackslashBeforeStringPlaceholderCannotReactivateQuote() {
void regexPlaceholderIsEscapedAsLiteralPatternText() {
HashMap<String, Object> bindings = new HashMap<>();

String prepared = JavascriptPlaceholderBinder.bind("/^%name%$/i.test(value)",
String prepared = bind("/^%name%$/i.test(value)",
ignored -> "Ben.*", bindings::put);

assertEquals("/^Ben\\.\\*$/i.test(value)", prepared);
Expand All @@ -166,7 +177,7 @@ void regexPlaceholderIsEscapedAsLiteralPatternText() {
void oddBackslashBeforeRegexPlaceholderCannotReactivateDelimiter() {
HashMap<String, Object> bindings = new HashMap<>();

String prepared = JavascriptPlaceholderBinder.bind("/\\%value%/.test(input)",
String prepared = bind("/\\%value%/.test(input)",
ignored -> "/; attack() //", bindings::put);

assertEquals("/\\/; attack\\(\\) \\/\\//.test(input)", prepared);
Expand All @@ -177,7 +188,7 @@ void oddBackslashBeforeRegexPlaceholderCannotReactivateDelimiter() {
void placeholdersInCommentsAreNotEvaluatedOrRewritten() {
HashMap<String, Object> bindings = new HashMap<>();

String prepared = JavascriptPlaceholderBinder.bind("obj?.name; /* %name% */ true",
String prepared = bind("obj?.name; /* %name% */ true",
ignored -> "danger()", bindings::put);

assertEquals("obj?.name; /* %name% */ true", prepared);
Expand All @@ -189,7 +200,7 @@ void invalidPlaceholderBearingJavascriptFailsClosed() {
HashMap<String, Object> bindings = new HashMap<>();

assertThrows(IllegalArgumentException.class,
() -> JavascriptPlaceholderBinder.bind("if ( %name%", ignored -> "Ben", bindings::put));
() -> bind("if ( %name%", ignored -> "Ben", bindings::put));
assertTrue(bindings.isEmpty());
}
}
2 changes: 1 addition & 1 deletion ...ascriptSecurityFindingRegressionTest.java → ...ascriptSecurityFindingRegressionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package com.bencodez.advancedcore.api.javascript;
package com.bencodez.advancedcore.tests.api.javascript;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.mockito.ArgumentMatchers.anyString;
Expand Down
4 changes: 3 additions & 1 deletion ...avascript/JavascriptTextTemplateTest.java → ...avascript/JavascriptTextTemplateTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
package com.bencodez.advancedcore.api.javascript;
package com.bencodez.advancedcore.tests.api.javascript;

import com.bencodez.advancedcore.api.javascript.JavascriptTextTemplate;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
Expand Down
4 changes: 3 additions & 1 deletion ...aceholderUtilsJavascriptBoundaryTest.java → ...aceholderUtilsJavascriptBoundaryTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
package com.bencodez.advancedcore.api.messages;
package com.bencodez.advancedcore.tests.api.messages;

import com.bencodez.advancedcore.api.messages.PlaceholderUtils;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
Expand Down
19 changes: 15 additions & 4 deletions ...api/misc/PlayerManagerDamageItemTest.java → ...api/misc/PlayerManagerDamageItemTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
package com.bencodez.advancedcore.api.misc;
package com.bencodez.advancedcore.tests.api.misc;

import com.bencodez.advancedcore.tests.support.ReflectiveAccess;

import com.bencodez.advancedcore.api.misc.MiscUtils;
import com.bencodez.advancedcore.api.misc.PlayerManager;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
Expand All @@ -25,6 +30,12 @@
import org.mockito.MockedStatic;

class PlayerManagerDamageItemTest {
private static int sampleDamage(int attempts, int chance, long cap, java.util.random.RandomGenerator random) {
return ReflectiveAccess.invoke(PlayerManager.class, null, "sampleDamage",
new Class<?>[] { int.class, int.class, long.class, java.util.random.RandomGenerator.class },
attempts, chance, cap, random);
}

private final PlayerManager manager = PlayerManager.getInstance();

@Test
Expand Down Expand Up @@ -104,16 +115,16 @@ void boundedSamplerRetainsBinomialMoments() {
double sum = 0;
double squared = 0;
for (int i = 0; i < count; i++) {
int hits = PlayerManager.sampleDamage(1_000, 50, 1_001, random);
int hits = sampleDamage(1_000, 50, 1_001, random);
sum += hits;
squared += (double) hits * hits;
}
double mean = sum / count;
double variance = squared / count - mean * mean;
assertEquals(500, mean, 1);
assertEquals(250, variance, 10);
assertEquals(0, PlayerManager.sampleDamage(100, 0, 10, random));
assertEquals(10, PlayerManager.sampleDamage(Integer.MAX_VALUE, 100, 10, random));
assertEquals(0, sampleDamage(100, 0, 10, random));
assertEquals(10, sampleDamage(Integer.MAX_VALUE, 100, 10, random));
}

private static final class Fixture {
Expand Down
40 changes: 40 additions & 0 deletions ...dCore/src/test/java/com/bencodez/advancedcore/tests/api/permissions/PermissionAccess.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package com.bencodez.advancedcore.tests.api.permissions;

import java.util.UUID;

import com.bencodez.advancedcore.api.permissions.PermissionHandler;
import com.bencodez.advancedcore.api.permissions.PlayerPermissionHandler;
import com.bencodez.advancedcore.tests.support.ReflectiveAccess;

/** Typed calls also support Mockito stubbing and verification of the original methods. */
final class PermissionAccess {
private PermissionAccess() { }

static boolean isExpirationCurrent(PlayerPermissionHandler handler, String permission, long expiry) {
return ReflectiveAccess.invoke(PlayerPermissionHandler.class, handler, "isExpirationCurrent",
new Class<?>[] { String.class, long.class }, permission, expiry);
}

static void expirePermission(PlayerPermissionHandler handler, String permission, long expiry, boolean attachment) {
ReflectiveAccess.invoke(PlayerPermissionHandler.class, handler, "expirePermission",
new Class<?>[] { String.class, long.class, boolean.class }, permission, expiry, attachment);
}

static void scheduleExpiration(PermissionHandler handler, PlayerPermissionHandler player, String permission,
long expiry, long delay) {
ReflectiveAccess.invoke(PermissionHandler.class, handler, "scheduleExpiration",
new Class<?>[] { PlayerPermissionHandler.class, String.class, long.class, long.class },
player, permission, expiry, delay);
}

static void dispatchExpiration(PermissionHandler handler, PlayerPermissionHandler player, String permission, long expiry) {
ReflectiveAccess.invoke(PermissionHandler.class, handler, "dispatchExpiration",
new Class<?>[] { PlayerPermissionHandler.class, String.class, long.class }, player, permission, expiry);
}

static void removePermissionIfEmpty(PermissionHandler handler, UUID uuid, PlayerPermissionHandler player,
boolean offline) {
ReflectiveAccess.invoke(PermissionHandler.class, handler, "removePermissionIfEmpty",
new Class<?>[] { UUID.class, PlayerPermissionHandler.class, boolean.class }, uuid, player, offline);
}
}
Loading
Loading