Skip to content

Fix the CI failures from #320: SpotBugs findings and a GPU-only assumption on pocl - #321

Merged
bernardladenthin merged 1 commit into
mainfrom
claude/ci-spotbugs-and-pocl-gpu-assume
Jul 27, 2026
Merged

Fix the CI failures from #320: SpotBugs findings and a GPU-only assumption on pocl#321
bernardladenthin merged 1 commit into
mainfrom
claude/ci-spotbugs-and-pocl-gpu-assume

Conversation

@bernardladenthin

Copy link
Copy Markdown
Owner

Summary

Fixes the two failures that turned main red immediately after #320. They are unrelated to each other, and both were reachable on a developer machine — I verified #320 with mvn test, spotless:check and reuse lint, and never once with spotbugs:check.

  • SpotBugs: 14 findings → 0. Ten are fixed rather than silenced; four are suppressed with rationale, following the precedent already in spotbugs-exclude.xml.
  • One test failed on the pocl CI job because it assumed "an OpenCL device is available" where it needed "a GPU is available". Auto-discovery deliberately sweeps GPUs only — pocl exposes a CPU device — so the test asked the tuner to find something it correctly filters out.

What was actually wrong

Finding The real defect
EI_EXPOSE_REP / REP2 DeviceSweep took a live slice of the sweep's own accumulator and handed the same list back out. Its compact constructor now copies.
OCP_OVERLY_CONCRETE ×3 Parameters widened to Collection / Iterable — all the bodies ever needed.
WEM_WEAK_EXCEPTION ×3 Exceptions now carry the device, the platform/device indices or the configured key-producer counts instead of a fixed sentence.
FORMAT_STRING_MANIPULATION withSignificantDigits assembled "%." + decimals + "f". A switch over literal formats also spells out that only three shapes exist.
PZLA_PREFER_ZERO_LENGTH_ARRAYS The device-info query returned null on refusal; an empty array is rejected by the same length checks, so the null bought nothing.

Suppressed with rationale: three EXS_EXCEPTION_SOFTENING_NO_CONSTRAINTS where Jackson's checked exception is wrapped while serialising POJOs this project owns end to end — a failure there means the POJO shape and the mapper have gone out of step, a programming error no caller could act on — and one ENMI_ONE_ENUM_VALUE for a stateless singleton, where a second constant would have no meaning.

The test failure is the better lesson

run_noProducerConfiguredButGpuPresent_sweepsTheDetectedDevices was guarded only by an OpenCL 2.0+ device is available. But an OpenCL device is not necessarily a GPU, and excluding CPU runtimes from auto-discovery is deliberate and tested behaviour — a CPU runtime duplicates what producerJava already does. This project's own OpenCL CI job runs on pocl, which exposes exactly one CPU device.

So the test failed on its own premise, not on the behaviour under test. It now also assumes hasGpuOpenCLDevice() — a helper that already existed in OpenCLPlatformAssume and that I did not look for.

Test plan

  • spotbugs:check reports 0 bugs (was 14), reproduced locally with the same command the code-style job uses: mvn -DskipTests -Denforcer.skip=true compile spotbugs:check
  • 127 tests pass across TuneConfigurationTest, StatisticsTest, OpenCLDeviceTopologyTest, ConsumerJavaTest, ConfigFixturesParseTest and the architecture rules
  • spotbugs-exclude.xml parses as XML — a malformed filter silently disables every suppression
  • REUSE 487/487, spotless:check clean
  • CI is green on this branch

Not verified here: the pocl job itself. There is no pocl runtime on this machine, so the assumption fix is reasoned from pocl exposing CL_DEVICE_TYPE_CPU and from the failure message, not observed. This is exactly the class of gap that produced the bug, so it is worth watching the test-opencl job on this branch rather than assuming.

Related issues / PRs

Refs #320

Checklist

  • I have read CONTRIBUTING.md and CODE_OF_CONDUCT.md
  • My commits follow Conventional Commits
  • No security-sensitive changes

…there is no GPU

CI went red on main immediately after the multi-GPU merge, with two unrelated
failures. Both were reachable on this machine and neither was run before merging:
I verified with mvn test, spotless:check and reuse lint, and never once with
spotbugs:check, which the code-style job runs early precisely so a finding fails
a push rather than surfacing at release.

SpotBugs: 14 findings, all in code added by that merge. Ten are fixed rather than
silenced:

  EI_EXPOSE_REP/REP2  DeviceSweep took a live slice of the sweep's own
                      accumulator and handed the same list back out; its compact
                      constructor now copies
  OCP_OVERLY_CONCRETE three parameters widened to Collection/Iterable, which is
                      all the bodies need
  WEM_WEAK_EXCEPTION  three exceptions now carry the device, the indices or the
                      configured counts instead of a fixed sentence
  FORMAT_STRING       withSignificantDigits assembled "%." + decimals + "f"; a
                      switch over literal formats also spells out that only three
                      shapes exist
  PZLA_PREFER_ZERO    the device-info query returned null on refusal; an empty
                      array is rejected by the same length checks, so the null
                      bought nothing

The remaining four are suppressed with rationale, following the precedent already
in spotbugs-exclude.xml: three EXS_EXCEPTION_SOFTENING where Jackson's checked
exception is wrapped while serialising POJOs this project owns end to end (a
failure there is a programming error no caller could act on), and one
ENMI_ONE_ENUM_VALUE for a stateless singleton, where a second constant would have
no meaning.

The test failure is a better lesson. run_noProducerConfiguredButGpuPresent_
sweepsTheDetectedDevices asserted that auto-discovery finds a device, guarded only
by "an OpenCL 2.0+ device is available". But an OpenCL device is not necessarily a
GPU, and auto-discovery deliberately sweeps GPUs only — a CPU runtime duplicates
what producerJava already does. The project's own OpenCL CI job runs on pocl,
which exposes exactly one CPU device. So the test asked the tuner to discover
something it correctly filters out, and failed on its own premise.

It now also assumes hasGpuOpenCLDevice(), a helper that already existed in
OpenCLPlatformAssume and that I did not look for. Worth noting that the exclusion
this tripped over is itself deliberate and tested — the failure was the test's
assumption, not the behaviour.

Verified: spotbugs:check reports 0 bugs (was 14), and 127 tests pass across
TuneConfigurationTest, StatisticsTest, OpenCLDeviceTopologyTest, ConsumerJavaTest,
ConfigFixturesParseTest and the architecture rules. REUSE 487/487, spotless clean,
spotbugs-exclude.xml parses.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@bernardladenthin
bernardladenthin merged commit 3c4fb30 into main Jul 27, 2026
9 of 13 checks passed
@bernardladenthin
bernardladenthin deleted the claude/ci-spotbugs-and-pocl-gpu-assume branch July 27, 2026 07:25
@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
52.2% Coverage on New Code (required ≥ 80%)
C Maintainability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant