Skip to content

test: reach 100% runtime coverage and fix cleanup failures #23

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
ryanbarlow97 merged 5 commits into from
Oct 2, 2026
Merged

test: reach 100% runtime coverage and fix cleanup failures #23

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7ebb4f5
test: cover runtime and fix cleanup retries
ryanbarlow97 Oct 2, 2026
bf4930b
fix: report worker errors and honor proxy identities
ryanbarlow97 Oct 2, 2026
8b01804
fix: notify callers when completion scheduling fails
ryanbarlow97 Oct 2, 2026
a516597
fix: preserve clean failure notification context
ryanbarlow97 Oct 2, 2026
33618be
ci: require coverage reports
ryanbarlow97 Oct 2, 2026
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
11 changes: 11 additions & 0 deletions .github/workflows/build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ jobs:
- name: Run unit tests and build
run: mvn -B --no-transfer-progress clean verify -DskipTests=false -Dmaven.test.skip=false

- name: Verify coverage report
run: test -s target/site/jacoco/jacoco.xml

- name: Verify runtime JAR
env:
ARTIFACT_PATH: ${{ steps.dev.outputs.jar }}
Expand All @@ -74,3 +77,11 @@ jobs:
name: unit-test-reports-${{ github.run_id }}-${{ github.run_attempt }}
path: target/surefire-reports/
if-no-files-found: error

- name: Upload coverage report
if: ${{ !cancelled() && hashFiles('target/site/jacoco/jacoco.xml') != '' }}
uses: actions/upload-artifact@v7
with:
name: coverage-report-${{ github.run_id }}-${{ github.run_attempt }}
path: target/site/jacoco/
if-no-files-found: error
11 changes: 11 additions & 0 deletions .github/workflows/maven-release.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,17 @@ jobs:
echo "jar=target/$final_name.jar" >> "$GITHUB_OUTPUT"
mvn -B --no-transfer-progress clean verify -DskipTests=false -Dmaven.test.skip=false

- name: Verify coverage report
run: test -s target/site/jacoco/jacoco.xml

- name: Upload coverage report
if: ${{ !cancelled() && hashFiles('target/site/jacoco/jacoco.xml') != '' }}
uses: actions/upload-artifact@v7
with:
name: coverage-report-${{ github.run_id }}-${{ github.run_attempt }}
path: target/site/jacoco/
if-no-files-found: error

- name: Stage only the release JAR
env:
ARTIFACT_PATH: ${{ steps.maven.outputs.jar }}
Expand Down
4 changes: 4 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,7 @@ Copyright (c) 2026 TF-Minecraft contributors.
TF-Minecraft-authored material in this repository is licensed under the
[Artistic License 2.0](LICENSE). Third-party dependencies and bundled material
retain their own licenses.

## Tests and coverage

Run `mvn clean verify` with Java 21 after installing the pinned dependencies. JaCoCo checks 100% executable runtime line coverage with no production-code exclusions. Instruction and branch coverage are reported separately in `target/site/jacoco/`; CI uploads the report for every build and release.
45 changes: 38 additions & 7 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
<maven.compiler.release>21</maven.compiler.release>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<maven.compiler.proc>full</maven.compiler.proc>
<argLine></argLine>
<tlibs.version>2.0.0</tlibs.version>
</properties>

Expand Down Expand Up @@ -44,13 +45,13 @@
<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter</artifactId>
<version>5.10.2</version>
<version>5.14.0</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
<version>5.14.2</version>
<version>5.23.0</version>
<scope>test</scope>
</dependency>
<dependency>
Expand Down Expand Up @@ -106,14 +107,44 @@
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<version>3.5.2</version>
<version>3.5.4</version>
<configuration>
<argLine>-javaagent:${org.mockito:mockito-core:jar} -Xshare:off</argLine>
<classpathDependencyExcludes>
<classpathDependencyExclude>io.papermc.paper:paper-api</classpathDependencyExclude>
</classpathDependencyExcludes>
<argLine>@{argLine} -javaagent:${org.mockito:mockito-core:jar} -Xshare:off</argLine>
<failIfNoTests>true</failIfNoTests>
</configuration>
</plugin>
<plugin>
<groupId>org.jacoco</groupId>
<artifactId>jacoco-maven-plugin</artifactId>
<version>0.8.15</version>
<executions>
<execution><goals><goal>prepare-agent</goal></goals></execution>
<execution>
<id>coverage-report</id>
<phase>verify</phase>
<goals><goal>report</goal></goals>
</execution>
<execution>
<id>coverage-check</id>
<phase>verify</phase>
<goals><goal>check</goal></goals>
<configuration>
<rules>
<rule>
<element>BUNDLE</element>
<limits>
<limit>
<counter>LINE</counter>
<value>MISSEDCOUNT</value>
<maximum>0</maximum>
</limit>
</limits>
</rule>
</rules>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
</build>
</project>
90 changes: 61 additions & 29 deletions src/main/java/net/tfminecraft/permcleaner/command/PermCleanerCommand.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
import java.util.List;
import java.util.Locale;
import java.util.UUID;
import java.util.concurrent.CompletableFuture;
import java.util.function.Consumer;
import java.util.stream.Collectors;

import org.bukkit.Bukkit;
Expand All @@ -14,6 +16,7 @@
import org.bukkit.command.CommandSender;
import org.bukkit.command.TabCompleter;
import org.bukkit.entity.Player;
import org.bukkit.plugin.IllegalPluginAccessException;

import net.tfminecraft.permcleaner.PermCleaner;
import net.tfminecraft.permcleaner.lp.CleanResult;
Expand Down Expand Up @@ -43,10 +46,10 @@ public boolean onCommand(CommandSender sender, Command command, String label, St
String sub = args[0].toLowerCase(Locale.ROOT);
switch (sub) {
case "reload" -> reload(sender);
case "status" -> status(sender, args);
case "inspect" -> inspect(sender, args);
case "clean" -> clean(sender, args, false);
case "force" -> clean(sender, args, true);
case "status" -> resolveTarget(sender, args, target -> status(sender, target));
case "inspect" -> resolveTarget(sender, args, target -> inspect(sender, target));
case "clean" -> resolveTarget(sender, args, target -> clean(sender, target, false));
case "force" -> resolveTarget(sender, args, target -> clean(sender, target, true));
default -> sender.sendMessage(ChatColor.RED + USAGE);
}
return true;
Expand All @@ -64,11 +67,7 @@ private void reload(CommandSender sender) {

// Keep the existing legacy text representation, formatting, and exact-string comparisons.
@SuppressWarnings("deprecation")
private void status(CommandSender sender, String[] args) {
Target target = resolveTarget(sender, args);
if (target == null) {
return;
}
private void status(CommandSender sender, Target target) {
String stored = plugin.stamps() == null
? null
: plugin.stamps().getSeason(target.uuid).orElse(null);
Expand All @@ -86,11 +85,7 @@ private void status(CommandSender sender, String[] args) {

// Keep the existing legacy text representation, formatting, and exact-string comparisons.
@SuppressWarnings("deprecation")
private void inspect(CommandSender sender, String[] args) {
Target target = resolveTarget(sender, args);
if (target == null) {
return;
}
private void inspect(CommandSender sender, Target target) {
sender.sendMessage(ChatColor.GREEN + "Inspecting " + ChatColor.WHITE + target.name + ChatColor.GREEN + "...");
plugin.cleaner().inspect(target.uuid, result -> {
if (!sender.equals(Bukkit.getConsoleSender()) && sender instanceof Player player && !player.isOnline()) {
Expand Down Expand Up @@ -120,9 +115,9 @@ private void inspect(CommandSender sender, String[] args) {

// Keep the existing legacy text representation, formatting, and exact-string comparisons.
@SuppressWarnings("deprecation")
private void clean(CommandSender sender, String[] args, boolean force) {
Target target = resolveTarget(sender, args);
if (target == null) {
private void clean(CommandSender sender, Target target, boolean force) {
if (plugin.seasonId().isBlank()) {
sender.sendMessage(ChatColor.RED + "Cleaning is disabled: season-id is blank.");
return;
}
if (!force && plugin.stamps() != null && !plugin.stamps().needsClean(target.uuid, plugin.seasonId())) {
Expand All @@ -148,26 +143,63 @@ private void clean(CommandSender sender, String[] args, boolean force) {

// Keep the existing legacy text representation, formatting, and exact-string comparisons.
@SuppressWarnings("deprecation")
private Target resolveTarget(CommandSender sender, String[] args) {
private void resolveTarget(CommandSender sender, String[] args, Consumer<Target> resolved) {
if (args.length < 2) {
if (sender instanceof Player player) {
return new Target(player.getUniqueId(), player.getName());
resolved.accept(new Target(player.getUniqueId(), player.getName()));
} else {
sender.sendMessage(ChatColor.RED + "Console must specify a player.");
}
sender.sendMessage(ChatColor.RED + "Console must specify a player.");
return null;
return;
}
String name = args[1];
if (name.isBlank()) {
sender.sendMessage(ChatColor.RED + "Unknown player " + name);
return;
}
Player online = Bukkit.getPlayerExact(args[1]);
Player online = Bukkit.getPlayerExact(name);
if (online != null) {
return new Target(online.getUniqueId(), online.getName());
resolved.accept(new Target(online.getUniqueId(), online.getName()));
return;
}
@SuppressWarnings("deprecation")
OfflinePlayer offline = Bukkit.getOfflinePlayer(args[1]);
OfflinePlayer cached = Bukkit.getOfflinePlayerIfCached(name);
if (cached != null) {
resolveOffline(sender, name, cached, resolved);
return;
}

try {
// Paper's resolver also honors proxy online-mode and offline UUID policy.
CompletableFuture.supplyAsync(() -> Bukkit.getPlayerUniqueId(name),
task -> Bukkit.getScheduler().runTaskAsynchronously(plugin, task)).whenComplete((uuid, failure) -> {
if (!plugin.isEnabled()) return;
try {
Bukkit.getScheduler().runTask(plugin, () -> {
if (!plugin.isEnabled() || (sender instanceof Player player && !player.isOnline())) return;
if (failure != null) {
sender.sendMessage(ChatColor.RED + "Player lookup failed for " + name + ". Please try again.");
} else if (uuid == null) {
sender.sendMessage(ChatColor.RED + "Unknown player " + name);
} else {
resolveOffline(sender, name, Bukkit.getOfflinePlayer(uuid), resolved);
}
});
} catch (IllegalPluginAccessException ignored) {
// The plugin was disabled between completing the lookup and scheduling the result.
}
});
} catch (IllegalPluginAccessException ignored) {
// The plugin was disabled before the lookup could be submitted.
}
}

@SuppressWarnings("deprecation")
private void resolveOffline(CommandSender sender, String name, OfflinePlayer offline, Consumer<Target> resolved) {
if (offline.hasPlayedBefore() && offline.getUniqueId() != null) {
String name = offline.getName() != null ? offline.getName() : args[1];
return new Target(offline.getUniqueId(), name);
resolved.accept(new Target(offline.getUniqueId(), offline.getName() != null ? offline.getName() : name));
} else {
sender.sendMessage(ChatColor.RED + "Unknown player " + name);
}
sender.sendMessage(ChatColor.RED + "Unknown player " + args[1]);
return null;
}

@Override
Expand Down
Loading