Add annotation for skipping classes/method in test coverage, few bits…

… of test cleanup

gradle/libs.versions.toml

@@ -27,7 +27,7 @@ jsvg = "1.6.1"
 llzip = "2.6.2"
 logback-classic = { strictly = "1.4.11" } # newer releases break in jar releases
 mapping-io = "0.6.1"
-mockito = "5.13.0"
+mockito = "5.14.2"
 natural-order = "1.1"
 openrewrite = "8.37.1"
 picocli = "4.7.6"

recaf-core/src/main/java/software/coley/recaf/services/decompile/BaseDecompilerConfig.java

@@ -17,7 +17,7 @@ public class BaseDecompilerConfig extends BasicConfigContainer implements Decomp
 	 * @param id
 	 * 		Container ID.
 	 */
-	public BaseDecompilerConfig( @Nonnull String id) {
+	public BaseDecompilerConfig(@Nonnull String id) {
 		super(SERVICE_DECOMPILE_IMPL, id);
 	}
 

recaf-core/src/main/java/software/coley/recaf/services/decompile/NoopDecompilerConfig.java

@@ -1,19 +1,19 @@
 package software.coley.recaf.services.decompile;
 
-import software.coley.recaf.config.BasicConfigContainer;
-import software.coley.recaf.config.ConfigGroups;
+import software.coley.recaf.util.ExcludeFromJacocoGeneratedReport;
 
 /**
  * Dummy config for {@link NoopJvmDecompiler} and {@link NoopAndroidDecompiler}.
  *
  * @author Matt Coley
  */
-public class NoopDecompilerConfig extends BasicConfigContainer implements DecompilerConfig {
+@ExcludeFromJacocoGeneratedReport(justification = "Config POJO")
+public class NoopDecompilerConfig extends BaseDecompilerConfig implements DecompilerConfig {
 	/**
 	 * New dummy config.
 	 */
 	public NoopDecompilerConfig() {
-		super(ConfigGroups.SERVICE_DECOMPILE, "decompiler-noop" + CONFIG_SUFFIX);
+		super("noop");
 	}
 
 	@Override

recaf-core/src/main/java/software/coley/recaf/services/decompile/cfr/CfrConfig.java

@@ -13,6 +13,7 @@
 import software.coley.recaf.config.BasicConfigValue;
 import software.coley.recaf.config.ConfigGroups;
 import software.coley.recaf.services.decompile.BaseDecompilerConfig;
+import software.coley.recaf.util.ExcludeFromJacocoGeneratedReport;
 import software.coley.recaf.util.ReflectUtil;
 import software.coley.recaf.util.StringUtil;
 
@@ -28,6 +29,7 @@
  */
 @ApplicationScoped
 @SuppressWarnings("all") // ignore unused refs / typos
+@ExcludeFromJacocoGeneratedReport(justification = "Config POJO")
 public class CfrConfig extends BaseDecompilerConfig {
 	private final ObservableObject<BooleanOption> stringbuffer = new ObservableObject<>(BooleanOption.DEFAULT);
 	private final ObservableObject<BooleanOption> stringbuilder = new ObservableObject<>(BooleanOption.DEFAULT);

recaf-core/src/main/java/software/coley/recaf/services/decompile/fallback/FallbackConfig.java

@@ -3,13 +3,15 @@
 import jakarta.enterprise.context.ApplicationScoped;
 import jakarta.inject.Inject;
 import software.coley.recaf.services.decompile.BaseDecompilerConfig;
+import software.coley.recaf.util.ExcludeFromJacocoGeneratedReport;
 
 /**
  * Config for {@link FallbackDecompiler}
  *
  * @author Matt Coley
  */
 @ApplicationScoped
+@ExcludeFromJacocoGeneratedReport(justification = "Config POJO")
 public class FallbackConfig extends BaseDecompilerConfig {
 	@Inject
 	public FallbackConfig() {

recaf-core/src/main/java/software/coley/recaf/services/decompile/fallback/print/ClassPrinter.java

@@ -456,7 +456,7 @@ private void appendStaticInitializer(@Nonnull Printer out, @Nonnull MethodMember
 			@Override
 			protected void buildDeclarationFlags(@Nonnull StringBuilder sb) {
 				// Force only printing the modifier 'static' even if other flags are present
-				sb.append("static ");
+				sb.append("static");
 			}
 
 			@Override

recaf-core/src/main/java/software/coley/recaf/services/decompile/procyon/ProcyonConfig.java

@@ -12,13 +12,15 @@
 import software.coley.recaf.config.BasicConfigValue;
 import software.coley.recaf.config.ConfigGroups;
 import software.coley.recaf.services.decompile.BaseDecompilerConfig;
+import software.coley.recaf.util.ExcludeFromJacocoGeneratedReport;
 
 /**
  * Config for {@link ProcyonDecompiler}
  *
  * @author Matt Coley
  */
 @ApplicationScoped
+@ExcludeFromJacocoGeneratedReport(justification = "Config POJO")
 public class ProcyonConfig extends BaseDecompilerConfig {
 	private final ObservableBoolean includeLineNumbersInBytecode = new ObservableBoolean(true);
 	private final ObservableBoolean showSyntheticMembers = new ObservableBoolean(false);

recaf-core/src/main/java/software/coley/recaf/services/decompile/vineflower/DummyResultSaver.java

@@ -1,6 +1,7 @@
 package software.coley.recaf.services.decompile.vineflower;
 
 import org.jetbrains.java.decompiler.main.extern.IResultSaver;
+import software.coley.recaf.util.ExcludeFromJacocoGeneratedReport;
 
 import java.util.jar.Manifest;
 
@@ -9,6 +10,7 @@
  *
  * @author therathatter
  */
+@ExcludeFromJacocoGeneratedReport(justification = "We don't use VF file IO, everything stays in memory")
 public class DummyResultSaver implements IResultSaver {
     @Override
     public void saveFolder(String s) {

recaf-core/src/main/java/software/coley/recaf/services/decompile/vineflower/VineflowerConfig.java

@@ -10,6 +10,7 @@
 import software.coley.observables.ObservableObject;
 import software.coley.recaf.config.BasicConfigValue;
 import software.coley.recaf.services.decompile.BaseDecompilerConfig;
+import software.coley.recaf.util.ExcludeFromJacocoGeneratedReport;
 
 import java.lang.reflect.Field;
 import java.util.HashMap;
@@ -23,6 +24,7 @@
  */
 @ApplicationScoped
 @SuppressWarnings("all") // ignore unused refs / typos
+@ExcludeFromJacocoGeneratedReport(justification = "Config POJO")
 public class VineflowerConfig extends BaseDecompilerConfig {
 	private final ObservableObject<Level> loggingLevel = new ObservableObject<>(Level.WARN);
 	private final ObservableBoolean removeBridge = new ObservableBoolean(true);

recaf-core/src/main/java/software/coley/recaf/util/ExcludeFromJacocoGeneratedReport.java

@@ -0,0 +1,28 @@
+package software.coley.recaf.util;
+
+import jakarta.annotation.Nonnull;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+/**
+ * JaCoCo excludes coverage metrics from classes and methods annotated with names including {@code "Generated"}.
+ * <p>
+ * <h1>USE THIS CLASS SPARINGLY</h1>
+ * <b>Only annotate things with this if they are POJO's!</b>
+ * Some classes like data models and config objects contribute to coverage metrics with things like getter/setters
+ * that do not realistically need to be covered.
+ *
+ * @author Matt Coley
+ */
+@Retention(RetentionPolicy.RUNTIME)
+@Target({ElementType.METHOD, ElementType.TYPE})
+public @interface ExcludeFromJacocoGeneratedReport {
+	/**
+	 * @return Reason why we exclude the annotated element.
+	 */
+	@Nonnull
+	String justification();
+}

recaf-core/src/test/java/software/coley/recaf/services/decompile/DecompileManagerTest.java

@@ -2,7 +2,9 @@
 
 import jakarta.annotation.Nonnull;
 import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import software.coley.observables.ObservableBoolean;
 import software.coley.recaf.info.ClassInfo;
 import software.coley.recaf.info.JvmClassInfo;
 import software.coley.recaf.services.decompile.cfr.CfrDecompiler;
@@ -14,6 +16,7 @@
 import software.coley.recaf.test.TestBase;
 import software.coley.recaf.test.TestClassUtils;
 import software.coley.recaf.test.dummy.HelloWorld;
+import software.coley.recaf.util.ReflectUtil;
 import software.coley.recaf.workspace.model.Workspace;
 
 import java.io.IOException;
@@ -28,20 +31,37 @@
  * Tests for {@link DecompilerManager}.
  */
 public class DecompileManagerTest extends TestBase {
+	private static final ObservableBoolean OB_TRUE = new ObservableBoolean(true);
+	private static final ObservableBoolean OB_FALSE = new ObservableBoolean(false);
 	static final TestJvmBytecodeFilter bytecodeFilter = new TestJvmBytecodeFilter();
 	static final TestOutputTextFilter textFilter = new TestOutputTextFilter();
 	static DecompilerManager decompilerManager;
+	static DecompilerManagerConfig decompilerManagerConfig;
 	static Workspace workspace;
-	static JvmClassInfo classToDecompile;
+	static JvmClassInfo classHelloWorld;
 
 	@BeforeAll
 	static void setup() throws IOException {
 		decompilerManager = recaf.get(DecompilerManager.class);
-		classToDecompile = TestClassUtils.fromRuntimeClass(HelloWorld.class);
-		workspace = TestClassUtils.fromBundle(TestClassUtils.fromClasses(classToDecompile));
+
+		// Setup workspace to pull from
+		classHelloWorld = TestClassUtils.fromRuntimeClass(HelloWorld.class);
+		workspace = TestClassUtils.fromBundle(TestClassUtils.fromClasses(classHelloWorld));
 		workspaceManager.setCurrent(workspace);
 	}
 
+	@BeforeEach
+	void setupEach() {
+		// We don't want to cache decompilations for this test, but we also
+		// do not want to edit the shared config in tests.
+		// Thus, we will make new config instances each test run so there's no cross-test pollution.
+		decompilerManagerConfig = new DecompilerManagerConfig();
+		decompilerManagerConfig.getCacheDecompilations().setValue(false);
+		assertDoesNotThrow(() -> ReflectUtil.quietSet(unwrapProxy(decompilerManager),
+				DecompilerManager.class.getDeclaredField("config"),
+				decompilerManagerConfig));
+	}
+
 	@Test
 	void testCfr() {
 		JvmDecompiler decompiler = decompilerManager.getJvmDecompiler(CfrDecompiler.NAME);
@@ -81,7 +101,7 @@ void testFiltersUsed() {
 			decompilerManager.addOutputTextFilter(textFilterSpy);
 
 			// Decompile
-			decompilerManager.decompile(decompiler, workspace, classToDecompile).get();
+			decompilerManager.decompile(decompiler, workspace, classHelloWorld).get();
 
 			// Verify each filter was called once
 			verify(bytecodeFilterSpy, times(1)).filter(any(), any(), any());
@@ -94,11 +114,64 @@ void testFiltersUsed() {
 		}
 	}
 
+	@Test
+	void testCaching() {
+		decompilerManagerConfig.getCacheDecompilations().setValue(true);
+		JvmDecompiler decompiler = decompilerManager.getJvmDecompiler(CfrDecompiler.NAME);
+		DecompileResult firstResult = assertDoesNotThrow(() -> decompilerManager.decompile(decompiler, workspace, classHelloWorld).get(1, TimeUnit.DAYS));
+
+		// Assert that repeated decompiles use the same result (caching, should be handled by abstract base)
+		// Only the manager will cache results. Using decompilers direcrly will not cache.
+		assertTrue(decompilerManagerConfig.getCacheDecompilations().getValue(), "Cache config not 'true'");
+		DecompileResult newResult = assertDoesNotThrow(() -> decompilerManager.decompile(decompiler, workspace, classHelloWorld).get(1, TimeUnit.SECONDS));
+		assertSame(firstResult, newResult, "Decompiler did not cache results");
+
+		// Change the decompiler hash. The decompiler result should change.
+		decompiler.getConfig().setHash(-1);
+		newResult = assertDoesNotThrow(() -> decompilerManager.decompile(decompiler, workspace, classHelloWorld).get(1, TimeUnit.SECONDS));
+		assertNotSame(firstResult, newResult, "Decompiler used cached result even though config hash changed");
+
+		// Verify direct decompiler usage does not cache
+		DecompileResult direct1 = decompiler.decompile(workspace, classHelloWorld);
+		DecompileResult direct2 = decompiler.decompile(workspace, classHelloWorld);
+		assertNotSame(direct1, direct2, "Direct decompiler use cached results unexpectedly");
+	}
+
+	@Test
+	void testFilterHollow() {
+		String decompilationBefore = assertDoesNotThrow(() -> decompilerManager.decompile(workspace, classHelloWorld).get().getText());
+		assertTrue(decompilationBefore.contains("\"Hello world\""));
+
+		decompilerManagerConfig.getFilterHollow().setValue(true);
+
+		// Hollowing will remove method bodies, so the 'println' call should no longer exist in the output
+		String decompilationAfter = assertDoesNotThrow(() -> decompilerManager.decompile(workspace, classHelloWorld).get().getText());
+		assertFalse(decompilationAfter.contains("\"Hello world\""));
+	}
+
+	@Test
+	void testDisplay() {
+		for (JvmDecompiler decompiler : decompilerManager.getJvmDecompilers()) {
+			assertTrue(decompiler.toString().contains(decompiler.getName()));
+			assertTrue(decompiler.toString().contains(decompiler.getVersion()));
+		}
+	}
+
+	@Test
+	void testComparison() {
+		JvmDecompiler cfr = decompilerManager.getJvmDecompiler(CfrDecompiler.NAME);
+		JvmDecompiler pro = decompilerManager.getJvmDecompiler(ProcyonDecompiler.NAME);
+		assertNotNull(cfr);
+		assertNotNull(pro);
+		assertNotEquals(cfr, pro);
+		assertNotEquals(cfr.hashCode(), pro.hashCode());
+	}
+
 	private static void runJvmDecompilation(@Nonnull JvmDecompiler decompiler) {
 		try {
 			// Generally, you'd handle results like this, with a when-complete.
 			// The blocking 'get' at the end is just so our test works.
-			DecompileResult firstResult = decompilerManager.decompile(decompiler, workspace, classToDecompile)
+			DecompileResult firstResult = decompilerManager.decompile(decompiler, workspace, classHelloWorld)
 					.whenComplete((result, throwable) -> {
 						assertNull(throwable);
 
@@ -107,23 +180,12 @@ private static void runJvmDecompilation(@Nonnull JvmDecompiler decompiler) {
 						assertNotNull(result.getText(), "Decompile result missing text");
 						assertTrue(result.getText().contains("\"Hello world\""), "Decompilation seems to be wrong");
 					}) // Block on this thread until we have the value.
-					.get(1, TimeUnit.DAYS);
-
-			// Assert that repeated decompiles use the same result (caching, should be handled by abstract base)
-			// Only the manager will cache results. Using decompilers direcrly will not cache.
-			assertTrue(decompilerManager.getServiceConfig().getCacheDecompilations().getValue(), "Default cache config not 'true'");
-			DecompileResult newResult = decompilerManager.decompile(decompiler, workspace, classToDecompile).get(1, TimeUnit.SECONDS);
-			assertSame(firstResult, newResult, "Decompiler did not cache results");
-
-			// Change the decompiler hash. The decompiler result should change.
-			decompiler.getConfig().setHash(-1);
-			newResult = decompilerManager.decompile(decompiler, workspace, classToDecompile).get(1, TimeUnit.SECONDS);
-			assertNotSame(firstResult, newResult, "Decompiler used cached result even though config hash changed");
+					.get(5, TimeUnit.SECONDS);
 
 			// Verify direct decompiler usage does not cache
-			DecompileResult direct1 = decompiler.decompile(workspace, classToDecompile);
-			DecompileResult direct2 = decompiler.decompile(workspace, classToDecompile);
-			assertNotSame(direct1, direct2, "Direct decompiler use cached results unexpectedly");
+			DecompileResult result = decompiler.decompile(workspace, classHelloWorld);
+			assertNull(result.getException(), "No exceptions should be reported during decompilation");
+			assertNotNull(result.getText(), "Missing decompilation output");
 		} catch (InterruptedException e) {
 			fail("Decompile was interrupted", e);
 		} catch (ExecutionException e) {
@@ -148,5 +210,4 @@ public String filter(@Nonnull Workspace workspace, @Nonnull ClassInfo classInfo,
 			return code;
 		}
 	}
-
 }