Repository navigation
Test critter's pre-generated models in pull-request CI - #4370
Merged
Merged
Conversation
The pull-request workflow's build runs a plain install, so critter has no pre-generated models there and every model is generated at runtime. Pushes to this repo also run build.yml, which tests against generated models, but pull requests from forks only get this workflow. Add a job that builds, runs critter-maven's generate-test-models and then runs the tests without recompiling, as build.yml does.
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The job follows the established AOT sequence, preserves woven classes, and has no identified blocking issues.
Review effort: Balanced
Findings: None
What changed in this PR
Adds pull-request CI coverage for Critter’s pre-generated models, closing the AOT testing gap for fork contributions.
Changes:
- Adds a dedicated AOT job that builds, generates models, and tests without recompiling.
- Preserves existing runtime-generation coverage and collects logs on failure.
| File | Description |
|---|---|
.github/workflows/pull-request.yml |
Adds the Critter AOT test job and failure-log artifacts. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a Critter (AOT) job to the Pull Requests workflow, so pull requests exercise critter's pre-generated models and not only runtime generation.
Why
Buildruns a plaininstall -Dmorphia.mapper=critter. Core's build doesn't run critter-maven, so there are no pre-generated models and critter generates every model at runtime. That's how Test record entities with both mappers #4366's AOT record tests failed in PR CI.build.yml. That workflow generates the models and tests against them, so same-repo PRs did get AOT coverage there.What the job does
It mirrors
build.yml:install -DskipTestsgenerate-test-modelsincoresurefire:test -Dmorphia.mapper=critter, which doesn't recompile. Recompiling would replace the woven classes and leave stale models (NoSuchMethodError: __readXxx).The existing
Buildjob is unchanged. It keeps running critter with runtime generation for every model, which, together withCoverage Report (critter)inbuild.yml, is the only full-suite run of the runtime tier. The two jobs cover the two critter tiers.Verified locally
BUILD SUCCESSacross the reactor.Aot*record fixtures got their models, and their AOT-tier checks passed.This PR's own CI runs the new job, since
pull_requestuses the workflow from the PR.The new job isn't a required check. If it should block merges, add
Critter (AOT)to master's required status checks.Fixes #4369