Skip to content

Updates module dependencies#11

Closed
jottinger wants to merge 2 commits intoDemchaAV:mainfrom
jottinger:10/fix/update-bytebuddy-version
Closed

Updates module dependencies#11
jottinger wants to merge 2 commits intoDemchaAV:mainfrom
jottinger:10/fix/update-bytebuddy-version

Conversation

@jottinger
Copy link
Copy Markdown
Contributor

  • Main focus is on bytebuddy, BUT
  • Also updates other dependencies to update CVE issues

jottinger added 2 commits May 8, 2026 16:31
* Main focus is on bytebuddy, BUT
* Also updates other dependencies to update CVE issues
@jottinger
Copy link
Copy Markdown
Contributor Author

Found it: it's the currentspeedbenchmark, being run from the wrong project, from the looks of it.

DemchaAV added a commit that referenced this pull request May 9, 2026
…ench to v2

The Performance Smoke Check has been silently broken on every PR
since v1.6 Phase F moved CurrentSpeedBenchmark from src/test/ to
the new benchmarks/ Maven module. The job survived only because it
runs on `pull_request` events, not `push`, so my v1.6.0 release
commits never tripped it. The first external PR (jottinger #11)
exposed the breakage with a misleading-looking ClassNotFoundException.

Two layers of breakage:

1. Workflow targeting — the perf-smoke and benchmark-diff jobs ran
   `mvnw -DskipTests test-compile` against the root module and
   invoked exec-maven-plugin with `-Dexec.classpathScope=test`,
   expecting the benchmark class on the root project's test
   classpath. After Phase F the class lives in
   benchmarks/src/main/java/, so the load fails. Both jobs now run
   from `-f benchmarks/pom.xml` (compile, not test-compile; main
   scope, no classpathScope=test). Artifact upload + cache paths
   move from `target/benchmarks/...` to
   `benchmarks/target/benchmarks/...`. An additional `Install root
   artifact` step puts io.github.demchaav:graphcompose:1.6.0 into
   the runner's local m2 so benchmarks/ can resolve its dependency.

2. Stale CV references in benchmarks/ source — CurrentSpeedBenchmark
   and FullCvBenchmark imported `CvTemplateV1` (deleted in Templates
   v2 along with the legacy CV / cover-letter classes) and
   `CvDocumentSpec` for benchmark fixtures. CanonicalBenchmarkSupport
   now exposes a v2 `canonicalCv()` returning a `CvSpec` (mirroring
   the canonical spec used by PresetVisualParityTest, including the
   "Projects" module that ModernProfessional's `main` slot
   declares). Both benchmarks switch to
   `ModernProfessional.create(BusinessTheme.modern())` driving a
   `DocumentTemplate<CvSpec>`. Invoice + proposal scenarios are
   unchanged — InvoiceTemplateV1 / ProposalTemplateV1 and their V1
   spec types still exist on the canonical surface.

Verified locally: `./mvnw -f benchmarks/pom.xml clean compile`
green; `./mvnw -f benchmarks/pom.xml exec:java
-Dexec.mainClass=com.demcha.compose.CurrentSpeedBenchmark
-Dgraphcompose.benchmark.profile=smoke` reports "Performance gate
passed for profile smoke" and writes a CSV report to
benchmarks/target/benchmarks/current-speed/.

Architecture + documentation guard suite still 30 / 0 / 0 / 0.
@DemchaAV
Copy link
Copy Markdown
Owner

DemchaAV commented May 9, 2026

Update — the Performance Smoke Check failure on this PR was actually a pre-existing issue in our CI workflow, not anything in your dependency bumps.

Root cause. During v1.6 we moved the benchmark suite (including CurrentSpeedBenchmark) from src/test/ into a new sibling benchmarks/ Maven module. The CI workflow's perf-smoke job didn't follow — it kept running test-compile against the root project and looking for the benchmark on the root's test classpath. Because the job is gated to pull_request events, my own post-v1.6 push commits never tripped it; your PR was the first one to surface the breakage. The benchmark sources also still imported the legacy CvTemplateV1 class that Templates v2 deleted, so the benchmarks/ module wasn't compilable as-is.

Fix landed on main (745b86cf):

  • .github/workflows/ci.yml perf-smoke + benchmark-diff jobs now run from -f benchmarks/pom.xml with the right paths.
  • CanonicalBenchmarkSupport.canonicalCv() and CurrentSpeedBenchmark / FullCvBenchmark migrated to the Templates v2 surface (ModernProfessional.create(BusinessTheme.modern()) driving a DocumentTemplate<CvSpec>).
  • Verified locally: mvnw -f benchmarks/pom.xml clean compile green, smoke benchmark reports "Performance gate passed for profile smoke".

Could you rebase this PR on the latest main? That'll pull both the workflow change and the benchmark migration into your branch, and CI should go green for the dependency bumps you actually wanted to ship. If you'd rather, I can rebase + force-push for you — let me know.

Two follow-up questions on the diff itself, unrelated to the CI red:

  1. benchmarks/pom.xml <maven.compiler.release> 21 → 17 — is that intentional in this PR, or a leftover from rebasing on top of 8/migrate to java 17 #12? The Java 17 baseline question is being tracked in Backmigrate to Java 17 #8 / 8/migrate to java 17 #12 separately; would prefer to keep the dep-bumps PR focused on dep bumps and let the Java baseline land (or not) on its own merit.
  2. Do you have a stack trace from running mockito 5.23 / byteBuddy 1.18.7 locally on Java 25, or do the existing tests pass cleanly under your setup? Asking just to know what regression coverage we're picking up here.

@DemchaAV
Copy link
Copy Markdown
Owner

DemchaAV commented May 9, 2026

Hi @jottinger — quick update. Based on issue #8 and the work in this PR + #12, I've decided to commit GraphCompose to a Java 17 baseline for the v1.7 release.

To get there cleanly I've consolidated everything into a single feature branch — PR #14 (feature/java-17-baseline). The first three commits are your work, cherry-picked verbatim with original authorship preserved (you'll see the @jottinger avatar on 5b7d6d4f, 2e32eb5d, af1f82af in git log). The fourth commit closes the gaps the consolidated branch needed: examples/pom.xml <maven.compiler.release> flip, WeeklyScheduleRenderer switch-with-deconstruction rewrites, BenchmarkMedianTool.java getFirst()get(0), ComparativeBenchmark.java threadId()getId(), CI workflow setup-java to Java 17, README badge, and a v1.7.0 — Planned CHANGELOG entry crediting you upfront.

CI on Temurin 17 is green: 785 / 0 / 0 / 0 across architecture/documentation guards, build+test, performance smoke, and examples generation.

I'm closing this PR in favour of #14 — your work isn't lost, it's the foundation of #14. Thank you for the clean diff and the patient debugging on the byte-buddy / Java 25 piece. Looking forward to more PRs.

Tracking issue: #8. Maven Central distribution: #7.

@DemchaAV
Copy link
Copy Markdown
Owner

DemchaAV commented May 9, 2026

Consolidated into #14. Thank you.

@DemchaAV DemchaAV mentioned this pull request May 9, 2026
@DemchaAV DemchaAV closed this May 9, 2026
DemchaAV added a commit that referenced this pull request May 9, 2026
* Updates module dependencies

* Main focus is on bytebuddy, BUT
* Also updates other dependencies to update CVE issues

* Updating benchmarks dependencies for CI/CD

* Updates code to conform to java 17

* Does not fully address submodules, as they're out of scope and should be backwards compatible anyway
* Migrates pattern-matched switch to alternatives
* Migrates new List access methods to alternatives
* Changes PreviewCompiler in src/test/java to use Maven-specified compiler version

* v1.7 prep: complete the Java 17 baseline migration

Builds on @jottinger's three commits cherry-picked above:

- 5b7d6d4 Updates module dependencies        (=#11 base)
- 2e32eb5 Updating benchmarks dependencies   (=#11 follow-up)
- af1f82a Updates code to conform to java 17 (=#12)

Follow-up gaps closed by this commit:

- examples/pom.xml — flip <maven.compiler.release> 21 → 17 (only
  the root and benchmarks POMs were flipped upstream).
- examples/.../WeeklyScheduleRenderer.java — rewrite the two
  switch-with-deconstruction blocks (DayShift / Half) plus the
  StatusFill instanceof-with-record-pattern check as Java
  17–compatible instanceof if-else chains. Records still expose
  status() / shift() / lunch() / dinner() so call sites are a
  one-line refactor each.
- benchmarks/.../BenchmarkMedianTool.java — 9× List.getFirst() →
  .get(0) (List.getFirst is JEP 431, Java 21).
- benchmarks/.../ComparativeBenchmark.java — Thread.threadId() →
  Thread.getId() (threadId is Java 19+).
- .github/workflows/ci.yml — actions/setup-java java-version
  '21' → '17' across all five jobs.
- README.md — Java-21 badge → Java-17+.
- CHANGELOG.md — new "## v1.7.0 — Planned" entry detailing the
  Java 17 baseline, dependency refresh, and Maven Central
  distribution plan; credits @jottinger upfront.

Local verification on this worktree:

- ./mvnw -B -ntp clean verify -pl . → 785 / 0 / 0 / 0
- ./mvnw -B -ntp -f examples/pom.xml clean compile → BUILD SUCCESS
- ./mvnw -B -ntp -f benchmarks/pom.xml clean compile → BUILD SUCCESS

Note: local Maven runs on Java 21 with -release 17, which catches
compile-time API drift but not runtime-only Java 21 hooks. The
draft PR's CI run on Temurin JDK 17 is the truth source for that.

Co-Authored-By: Joseph Ottinger <joeo@enigmastation.com>

* ci: run on PRs to any base + on develop pushes (so v1.7 prep PR triggers)

* docs(changelog): reframe Java 17 baseline as v1.6.1 (patch), not v1.7

SemVer-correct framing: this PR adds no public API, doesn't break
v1.6.0 callers, and is mostly compatibility expansion + dependency
hygiene. That's a patch, not a minor. Maven Central distribution
(the only true minor-level change in the prior framing) stays
queued for the actual v1.7.0 alongside the JMH migration.

* ci: matrix Build+test across JDK 17 / 21 / 25

Each push and PR now runs the full canonical test suite (~819
tests, mvnw verify) against three JDKs in parallel:

  - JDK 17 — the baseline target (compiler.release=17). Lowest
    common denominator; what enterprise stacks deploy on.
  - JDK 21 — the previous baseline. Validates that bytecode-17
    still runs cleanly on the post-Java-21 sequenced-collections
    JVM after the getFirst()/getLast()/threadId() rewrites.
  - JDK 25 — the current LTS. Validates the byte-buddy 1.18.7 +
    mockito 5.23 dependency bumps that #10 motivated; without
    those, mockito breaks on the JDK 25 access rules.

fail-fast: false so a regression on one JDK doesn't mask the
status of the others. Javadoc generation runs only on JDK 17 —
its output is JVM-independent, no need to triple-bake.

The other jobs (Architecture+Documentation Guards, Examples
Generation Smoke Test, Performance Smoke Check, Weekly Benchmark
Diff) stay single-JDK on 17 — they're fast enough that adding
parallel JDKs adds no signal, and benchmark numbers across JVMs
would be noisy comparisons rather than a regression gate.

---------

Co-authored-by: Joseph Ottinger <joeo@enigmastation.com>
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.

2 participants