Add MessageAdapter content parts serialization test coverage#11800
Conversation
|
Hi! 👋 Thanks for your pull request! 🎉 To help us review it, please make sure to:
If you need help, please check our contributing guidelines. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b845795757
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
| @@ -861,6 +861,25 @@ class AIGuardInternalTests extends DDSpecification { | |||
| } | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
nit: the adapter is re-created on every where: row. Consider moving it to a @Shared field.
b845795 to
09457bf
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 09457bfe70
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| } | ||
|
|
||
| void 'test adapter serializes content parts'() { |
There was a problem hiding this comment.
Move this new coverage to JUnit 5
The root AGENTS.md test-framework guideline says to always use JUnit 5 and not write new Groovy/Spock tests, migrating existing Groovy tests when touched. Adding this new Spock feature method extends the Groovy suite instead of moving the coverage to JUnit 5, so this should be rewritten as a JUnit 5 test rather than adding another Spock case.
Useful? React with 👍 / 👎.
eaad3c1
into
master
What Does This Do
Motivation
Additional Notes
Contributor Checklist
type:and (comp:orinst:) labels in addition to any other useful labelsclose,fix, or any linking keywords when referencing an issueUse
solvesinstead, and assign the PR milestone to the issue/merge. You can also:/merge --commit-message "..."/merge -c/merge -f --reason "reason"; please use this judiciously, as some checks do not run at the PR-level (note: the PR still needs to be mergeable, this will only skip the pre-merge build)Jira ticket: [PROJ-IDENT]