You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@juneau.apache.org by ja...@apache.org on 2026/10/12 22:03:12 UTC
(juneau) branch juneau-release-manager updated: Fix SonarQube findings in the release manager tests and coverage.py
This is an automated email from the ASF dual-hosted git repository.
jamesbognar pushed a commit to branch juneau-release-manager
in repository https://gitbox.apache.org/repos/asf/juneau.git
The following commit(s) were added to refs/heads/juneau-release-manager by this push:
new 6818845163 Fix SonarQube findings in the release manager tests and coverage.py
6818845163 is described below
commit 68188451633772caa6f56a7871a75a2e054f40d1
Author: James Bognar <ja...@apache.org>
AuthorDate: Mon Oct 12 18:03:06 2026 -0400
Fix SonarQube findings in the release manager tests and coverage.py
Split coverage.py's detail report into helpers and drop its constant return
and malformed suppression comment, keeping CLI output identical. In tests,
hoist setup out of assertThrows lambdas, split an oversized credential test,
replace a Thread.sleep with a bounded poll (suppressing the two where the
delay is what is tested), add a missing assertion on recorded gh calls, and
remove unused imports and throws clauses.
Co-Authored-By: Claude Opus 5.5 (1M context) <no...@anthropic.com>
---
scripts/coverage.py | 87 +++++++++++++---------
.../juneau/releng/AppConfigurationBeansTest.java | 1 -
.../releng/credential/CredentialBranchesTest.java | 22 ++++--
.../releng/engine/EngineSupportBranchesTest.java | 7 +-
.../releng/engine/steps/StepApplyBranchesTest.java | 5 +-
.../apache/juneau/releng/log/LogBranchesTest.java | 11 ++-
.../releng/nexus/MavenSettingsCredentialsTest.java | 3 +-
.../releng/release/ReleaseVersionExtraTest.java | 7 +-
.../releng/rest/ReleaseRunRestBranchesTest.java | 3 +-
.../juneau/releng/rest/SmallRestResourcesTest.java | 1 +
.../releng/util/DefaultProcessRunnerTest.java | 10 ++-
11 files changed, 101 insertions(+), 56 deletions(-)
diff --git a/scripts/coverage.py b/scripts/coverage.py
index d4e7716d0f..421cf1a79d 100755
--- a/scripts/coverage.py
+++ b/scripts/coverage.py
@@ -312,60 +312,74 @@ def by_module_report(xml_path: Path):
print()
-def detail_report(xml_path: Path, target: str, branches_only: bool):
- """Print per-file coverage and uncovered lines for a top-level package name or a source path."""
- pkg_filter, file_filter = None, None
+def parse_detail_target(target: str) -> tuple[str, str | None, str | None]:
+ """Split a detail-report target into (normalized target, package filter, file filter)."""
t = target.replace("\\", "/")
marker = "src/main/java/"
- if marker in t:
- rest = t.split(marker, 1)[1].rstrip("/")
- if rest.endswith(".java"):
- pkg_filter, file_filter = rest.rsplit("/", 1)[0], rest.rsplit("/", 1)[1]
- else:
- pkg_filter = rest
+ if marker not in t:
+ return t, None, None
+ rest = t.split(marker, 1)[1].rstrip("/")
+ if rest.endswith(".java"):
+ pkg_filter, file_filter = rest.rsplit("/", 1)
+ return t, pkg_filter, file_filter
+ return t, rest, None
+
+
+def package_matches(pkg_name: str, t: str, pkg_filter: str | None) -> bool:
+ """True if a JaCoCo package belongs in the detail report for the given filters."""
+ if pkg_filter is not None:
+ return pkg_name == pkg_filter or pkg_name.startswith(pkg_filter + "/")
+ return top_package(pkg_name) == t
+
+
+def print_file_detail(pkg_name: str, fname: str, stats) -> None:
+ """Print the coverage block for one source file."""
+ mb, cb, mi, ci, uncovered, excluded = stats
+ note = " (HTT-adjusted)" if excluded else ""
+ print(f"\n{'=' * 70}\n {pkg_name.replace('/', '.')}.{fname.removesuffix('.java')}\n{'=' * 70}")
+ print(f" Branches: {bar(cb, mb + cb)} {pct(cb, mb + cb):>4} ({cb}/{mb + cb} covered, {mb} missed){note}")
+ print(f" Instructions: {bar(ci, mi + ci)} {pct(ci, mi + ci):>4} ({ci}/{mi + ci} covered, {mi} missed){note}")
+ if excluded:
+ print(f" Excluded {len(excluded)} hard-to-test line(s) (HTT):")
+ for ln, reason in sorted(excluded):
+ print(f" L{ln}: {reason}")
+ if not uncovered:
+ print("\n All lines covered!")
+ return
+ print("\n Uncovered lines:")
+ for ln, lmb, total, lmi in sorted(uncovered):
+ parts = []
+ if lmb > 0:
+ parts.append(f"{lmb}/{total} branches missed")
+ if lmi > 0:
+ parts.append(f"{lmi} instructions missed")
+ print(f" line {ln:4d}: {', '.join(parts)}")
+
+
+def detail_report(xml_path: Path, target: str, branches_only: bool):
+ """Print per-file coverage and uncovered lines for a top-level package name or a source path."""
+ t, pkg_filter, file_filter = parse_detail_target(target)
root = ET.parse(xml_path).getroot()
shown = 0
for pkg in sorted(root.findall("package"), key=lambda p: p.get("name", "")):
pkg_name = pkg.get("name", "")
- if pkg_filter is not None:
- if not (pkg_name == pkg_filter or pkg_name.startswith(pkg_filter + "/")):
- continue
- elif top_package(pkg_name) != t:
+ if not package_matches(pkg_name, t, pkg_filter):
continue
for sf in sorted(pkg.findall("sourcefile"), key=lambda s: s.get("name", "")):
fname = sf.get("name", "")
if file_filter and fname != file_filter:
continue
- mb, cb, mi, ci, uncovered, excluded = file_stats(sf, pkg_name, fname, branches_only)
+ print_file_detail(pkg_name, fname, file_stats(sf, pkg_name, fname, branches_only))
shown += 1
- note = " (HTT-adjusted)" if excluded else ""
- print(f"\n{'=' * 70}\n {pkg_name.replace('/', '.')}.{fname.removesuffix('.java')}\n{'=' * 70}")
- print(f" Branches: {bar(cb, mb + cb)} {pct(cb, mb + cb):>4} ({cb}/{mb + cb} covered, {mb} missed){note}")
- print(f" Instructions: {bar(ci, mi + ci)} {pct(ci, mi + ci):>4} ({ci}/{mi + ci} covered, {mi} missed){note}")
- if excluded:
- print(f" Excluded {len(excluded)} hard-to-test line(s) (HTT):")
- for ln, reason in sorted(excluded):
- print(f" L{ln}: {reason}")
- if uncovered:
- print("\n Uncovered lines:")
- for ln, lmb, total, lmi in sorted(uncovered):
- parts = []
- if lmb > 0:
- parts.append(f"{lmb}/{total} branches missed")
- if lmi > 0:
- parts.append(f"{lmi} instructions missed")
- print(f" line {ln:4d}: {', '.join(parts)}")
- else:
- print("\n All lines covered!")
if shown == 0:
die(f"No JaCoCo data found for '{target}'.")
-def main(): # NOSONAR: always returns 0 by design
+def main():
args = sys.argv[1:]
if not args or "--help" in args or "-h" in args:
print(__doc__)
- return 0
+ return
target, do_run, branches_only, by_module = None, False, False, False
for arg in args:
if arg in ("--run", "-r"):
@@ -387,8 +401,7 @@ def main(): # NOSONAR: always returns 0 by design
detail_report(xml_path, target, branches_only)
else:
die("No package or path specified (or use --by-module).")
- return 0
if __name__ == "__main__":
- sys.exit(main())
+ main()
diff --git a/src/test/java/org/apache/juneau/releng/AppConfigurationBeansTest.java b/src/test/java/org/apache/juneau/releng/AppConfigurationBeansTest.java
index e386bc019b..0796c15f52 100644
--- a/src/test/java/org/apache/juneau/releng/AppConfigurationBeansTest.java
+++ b/src/test/java/org/apache/juneau/releng/AppConfigurationBeansTest.java
@@ -30,7 +30,6 @@ import org.apache.juneau.releng.config.TargetProfile;
import org.apache.juneau.releng.credential.CredentialSpec;
import org.apache.juneau.releng.engine.ReleaseEngine;
import org.apache.juneau.releng.util.DefaultProcessRunner;
-import org.apache.juneau.releng.util.ProcessRunner;
import org.apache.juneau.rest.server.filter.SynchronizerToken;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
diff --git a/src/test/java/org/apache/juneau/releng/credential/CredentialBranchesTest.java b/src/test/java/org/apache/juneau/releng/credential/CredentialBranchesTest.java
index f23cff550a..dfe7264acf 100644
--- a/src/test/java/org/apache/juneau/releng/credential/CredentialBranchesTest.java
+++ b/src/test/java/org/apache/juneau/releng/credential/CredentialBranchesTest.java
@@ -23,7 +23,6 @@ import java.nio.file.Files;
import java.nio.file.Path;
import java.util.EnumMap;
import java.util.List;
-import java.util.Map;
import org.apache.juneau.commons.secret.InMemorySecretStore;
import org.apache.juneau.commons.secret.SecretStore;
import org.apache.juneau.releng.credential.Validator.ValidationResult;
@@ -94,7 +93,7 @@ class CredentialBranchesTest {
// ---- AccountStore ----
@Test
- void a04_accountStoreEdgeCases(@TempDir Path dir) throws Exception {
+ void a04_accountStoreEdgeCases(@TempDir Path dir) {
var store = new AccountStore(dir);
store.remove(CredentialSpec.GPG); // nothing stored: no save
assertFalse(Files.exists(dir.resolve("accounts.properties")));
@@ -119,8 +118,9 @@ class CredentialBranchesTest {
Files.createDirectories(d2);
var f = Files.writeString(d2.resolve("accounts.properties"), "gpg=K\n");
Files.setPosixFilePermissions(f, java.nio.file.attribute.PosixFilePermissions.fromString("---------"));
+ var unreadable = new AccountStore(d2);
try {
- assertThrows(RuntimeException.class, () -> new AccountStore(d2).get(CredentialSpec.GPG));
+ assertThrows(RuntimeException.class, () -> unreadable.get(CredentialSpec.GPG));
} finally {
Files.setPosixFilePermissions(f, java.nio.file.attribute.PosixFilePermissions.fromString("rw-------"));
}
@@ -133,20 +133,32 @@ class CredentialBranchesTest {
}
@Test
- void a06_githubReasons() {
+ void a06a_githubNotInstalled() {
assertTrue(gh(127, "").contains("not installed"));
assertTrue(gh(1, "bash: gh: No such file or directory").contains("not installed"));
- assertTrue(gh(1, "HTTP 401: Bad credentials").contains("401"));
assertTrue(gh(1, "gh: command not found").contains("not installed"));
+ }
+
+ @Test
+ void a06b_githubHttpStatusReasons() {
+ assertTrue(gh(1, "HTTP 401: Bad credentials").contains("401"));
assertTrue(gh(1, "HTTP 403").contains("403"));
assertTrue(gh(1, "insufficient permissions").contains("403"));
assertTrue(gh(1, "missing scope").contains("403"));
assertTrue(gh(1, "HTTP 404").contains("404"));
+ }
+
+ @Test
+ void a06c_githubUnreachable() {
assertTrue(gh(1, "dial tcp: lookup").contains("unreachable"));
assertTrue(gh(1, "could not resolve host").contains("unreachable"));
assertTrue(gh(1, "connection refused").contains("unreachable"));
assertTrue(gh(1, "i/o timeout").contains("unreachable"));
assertTrue(gh(1, "network is unreachable").contains("unreachable"));
+ }
+
+ @Test
+ void a06d_githubExitCodeAndSuccess() {
assertTrue(gh(7, "weird").contains("exit code 7"));
assertTrue(gh(7, null).contains("exit code 7"));
assertEquals("GitHub token OK.", gh(0, null));
diff --git a/src/test/java/org/apache/juneau/releng/engine/EngineSupportBranchesTest.java b/src/test/java/org/apache/juneau/releng/engine/EngineSupportBranchesTest.java
index e0629ae35b..2ff1724746 100644
--- a/src/test/java/org/apache/juneau/releng/engine/EngineSupportBranchesTest.java
+++ b/src/test/java/org/apache/juneau/releng/engine/EngineSupportBranchesTest.java
@@ -103,7 +103,9 @@ class EngineSupportBranchesTest {
@Test
void a05_storeIoFailures(@TempDir Path dir) throws Exception {
var file = Files.writeString(dir.resolve("afile"), "x");
- assertThrows(RuntimeException.class, () -> new RunStateStore(file).save(RunState.create("1.0.0", "b", List.of())));
+ var badStore = new RunStateStore(file);
+ var state = RunState.create("1.0.0", "b", List.of());
+ assertThrows(RuntimeException.class, () -> badStore.save(state));
var store = new RunStateStore(dir);
store.save(RunState.create("1.0.0", "b", List.of()));
@@ -131,6 +133,9 @@ class EngineSupportBranchesTest {
}
@Test
+ @SuppressWarnings({
+ "java:S2925" // The sleep guarantees distinct updatedAt timestamps; displayRun() picks the newest by that time.
+ })
void a06_storeFiltersAndPicksRuns(@TempDir Path dir) throws Exception {
var store = new RunStateStore(dir);
assertTrue(new RunStateStore(dir.resolve("missing")).loadAll().isEmpty());
diff --git a/src/test/java/org/apache/juneau/releng/engine/steps/StepApplyBranchesTest.java b/src/test/java/org/apache/juneau/releng/engine/steps/StepApplyBranchesTest.java
index f119317ad8..0e7d86cba9 100644
--- a/src/test/java/org/apache/juneau/releng/engine/steps/StepApplyBranchesTest.java
+++ b/src/test/java/org/apache/juneau/releng/engine/steps/StepApplyBranchesTest.java
@@ -339,7 +339,7 @@ class StepApplyBranchesTest {
}
@Test
- void a16_previewsAndEdgeBranches(@TempDir Path dir) throws Exception {
+ void a16_previewsAndEdgeBranches(@TempDir Path dir) {
var r = new Runner();
var c = ctx(r, dir, "9.2.1");
// Preflight preview with every tool missing.
@@ -400,7 +400,8 @@ class StepApplyBranchesTest {
var rcDir = dir.resolve("dist/source/juneau-9.2.1-RC1");
Files.createDirectories(rcDir.getParent());
Files.writeString(rcDir, "blocker");
- assertThrows(IllegalStateException.class, () -> new BinaryArtifactsStageStep().apply(c));
+ var step = new BinaryArtifactsStageStep();
+ assertThrows(IllegalStateException.class, () -> step.apply(c));
}
@Test
diff --git a/src/test/java/org/apache/juneau/releng/log/LogBranchesTest.java b/src/test/java/org/apache/juneau/releng/log/LogBranchesTest.java
index 062e3f5109..9af3ac980f 100644
--- a/src/test/java/org/apache/juneau/releng/log/LogBranchesTest.java
+++ b/src/test/java/org/apache/juneau/releng/log/LogBranchesTest.java
@@ -66,6 +66,9 @@ class LogBranchesTest {
}
@Test
+ @SuppressWarnings({
+ "java:S2925" // Asserts that nothing fires; the sleep is the observation window for the (absent) timer callback.
+ })
void a01_timerSkipsWithoutWebhookOrWithPastDeadline() throws Exception {
var seen = new java.util.concurrent.CopyOnWriteArrayList<List<String>>();
new VoteDeadlineTimer(capturing(seen, new CountDownLatch(1)), "").arm("9.2.1", Instant.now().plusMillis(50));
@@ -92,7 +95,8 @@ class LogBranchesTest {
@Test
void a03_runLogErrorsAndSize(@TempDir Path dir) throws Exception {
var blocker = Files.writeString(dir.resolve("blocker"), "x");
- assertThrows(RuntimeException.class, () -> new RunLog(blocker.resolve("sub/x.log")));
+ var blockedLog = blocker.resolve("sub/x.log");
+ assertThrows(RuntimeException.class, () -> new RunLog(blockedLog));
var log = new RunLog(dir.resolve("a.log"));
assertEquals(dir.resolve("a.log"), log.file());
@@ -145,7 +149,10 @@ class LogBranchesTest {
var t = new Thread(() -> servlet.doGet(req("9.2.1/state"), resp));
t.setDaemon(true);
t.start();
- Thread.sleep(200);
+ var deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(5);
+ while (t.getState() != Thread.State.TIMED_WAITING && System.nanoTime() < deadline)
+ Thread.onSpinWait();
+ assertEquals(Thread.State.TIMED_WAITING, t.getState(), "servlet is tailing the broadcaster queue");
t.interrupt();
t.join(2000);
assertFalse(t.isAlive());
diff --git a/src/test/java/org/apache/juneau/releng/nexus/MavenSettingsCredentialsTest.java b/src/test/java/org/apache/juneau/releng/nexus/MavenSettingsCredentialsTest.java
index c665b4ca1a..e9f650c5a9 100644
--- a/src/test/java/org/apache/juneau/releng/nexus/MavenSettingsCredentialsTest.java
+++ b/src/test/java/org/apache/juneau/releng/nexus/MavenSettingsCredentialsTest.java
@@ -44,8 +44,9 @@ class MavenSettingsCredentialsTest {
@Test
void a02_missingFileFails(@TempDir Path dir) {
+ var missing = dir.resolve("nope.xml");
var e = assertThrows(IllegalStateException.class,
- () -> MavenSettingsCredentials.resolve("s", dir.resolve("nope.xml")));
+ () -> MavenSettingsCredentials.resolve("s", missing));
assertTrue(e.getMessage().contains("not found"), e.getMessage());
}
diff --git a/src/test/java/org/apache/juneau/releng/release/ReleaseVersionExtraTest.java b/src/test/java/org/apache/juneau/releng/release/ReleaseVersionExtraTest.java
index 6c8043cf60..3a9f5cacd6 100644
--- a/src/test/java/org/apache/juneau/releng/release/ReleaseVersionExtraTest.java
+++ b/src/test/java/org/apache/juneau/releng/release/ReleaseVersionExtraTest.java
@@ -58,8 +58,11 @@ class ReleaseVersionExtraTest {
assertEquals(a.hashCode(), b.hashCode());
assertEquals(a, a);
assertNotEquals(a, ReleaseVersion.of("9.2.1"));
- assertNotEquals(a, "9.2");
- assertNotEquals(a, null);
+ // Typed as Object (not literal) so the call keeps exercising ReleaseVersion.equals rather than String.equals.
+ Object notAVersion = "9.2";
+ Object nothing = null;
+ assertNotEquals(a, notAVersion);
+ assertNotEquals(a, nothing);
assertTrue(ReleaseVersion.of("9.3").compareTo(a) > 0);
assertTrue(ReleaseVersion.of("9.2.0.1").compareTo(a) > 0);
assertTrue(a.compareTo(ReleaseVersion.of("9.2.0.1")) < 0);
diff --git a/src/test/java/org/apache/juneau/releng/rest/ReleaseRunRestBranchesTest.java b/src/test/java/org/apache/juneau/releng/rest/ReleaseRunRestBranchesTest.java
index fe4891f327..1f888a0f35 100644
--- a/src/test/java/org/apache/juneau/releng/rest/ReleaseRunRestBranchesTest.java
+++ b/src/test/java/org/apache/juneau/releng/rest/ReleaseRunRestBranchesTest.java
@@ -101,7 +101,8 @@ class ReleaseRunRestBranchesTest {
void a03_startWhileAnotherRunIsActiveIsConflict(@TempDir Path dir) {
var rest = rest(dir);
rest.start(start("9.2.1"));
- assertThrows(Conflict.class, () -> rest.start(start("9.2.2")));
+ var second = start("9.2.2");
+ assertThrows(Conflict.class, () -> rest.start(second));
}
@Test
diff --git a/src/test/java/org/apache/juneau/releng/rest/SmallRestResourcesTest.java b/src/test/java/org/apache/juneau/releng/rest/SmallRestResourcesTest.java
index 4d2a14a8af..cd4094ffcf 100644
--- a/src/test/java/org/apache/juneau/releng/rest/SmallRestResourcesTest.java
+++ b/src/test/java/org/apache/juneau/releng/rest/SmallRestResourcesTest.java
@@ -100,6 +100,7 @@ class SmallRestResourcesTest {
var rest = new MilestoneRest(new MilestoneService(), new GithubPrSource(r, "apache/juneau"), r, "/repo");
var prs = rest.prs("9.3.0");
+ assertEquals(List.of("gh", "pr", "list"), calls.get(0).subList(0, 3));
assertEquals(2, prs.size());
assertEquals(7, prs.get(0).number);
assertEquals("jb", prs.get(0).authorLogin);
diff --git a/src/test/java/org/apache/juneau/releng/util/DefaultProcessRunnerTest.java b/src/test/java/org/apache/juneau/releng/util/DefaultProcessRunnerTest.java
index b59627effb..7775499583 100644
--- a/src/test/java/org/apache/juneau/releng/util/DefaultProcessRunnerTest.java
+++ b/src/test/java/org/apache/juneau/releng/util/DefaultProcessRunnerTest.java
@@ -38,7 +38,8 @@ class DefaultProcessRunnerTest {
@Test
void a02_runTextThrowsOnNonZeroExit() {
- var e = assertThrows(IllegalStateException.class, () -> runner.runText(List.of("sh", "-c", "echo boom; exit 3")));
+ var cmd = List.of("sh", "-c", "echo boom; exit 3");
+ var e = assertThrows(IllegalStateException.class, () -> runner.runText(cmd));
assertTrue(e.getMessage().contains("exit 3"), e.getMessage());
assertTrue(e.getMessage().contains("boom"), e.getMessage());
}
@@ -76,8 +77,8 @@ class DefaultProcessRunnerTest {
@Test
void c01_missingExecutableIsReportedAsError() {
- var e = assertThrows(IllegalStateException.class,
- () -> runner.run(List.of("/nonexistent/rm-no-such-binary"), null, null));
+ var cmd = List.of("/nonexistent/rm-no-such-binary");
+ var e = assertThrows(IllegalStateException.class, () -> runner.run(cmd, null, null));
assertTrue(e.getMessage().contains("Error running"), e.getMessage());
}
@@ -85,7 +86,8 @@ class DefaultProcessRunnerTest {
void c02_interruptIsPropagatedAndFlagRestored() {
Thread.currentThread().interrupt();
try {
- var e = assertThrows(IllegalStateException.class, () -> runner.run(List.of("sleep", "5"), null, null));
+ var cmd = List.of("sleep", "5");
+ var e = assertThrows(IllegalStateException.class, () -> runner.run(cmd, null, null));
assertTrue(e.getMessage().contains("Interrupted"), e.getMessage());
assertTrue(Thread.currentThread().isInterrupted());
} finally {