fix(core): adjust the jsonCreator Annotation for full argument constructor #756
fix(core): adjust the jsonCreator Annotation for full argument constructor #756Willam2004 wants to merge 3 commits intoagentscope-ai:mainfrom
Conversation
- Move `@JsonCreator` to the second constructor - Add unit tests for ToolUseBlock JSON serialization and deserialization
Summary of ChangesHello @Willam2004, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses an issue with the JSON deserialization of Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
|
|
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly adjusts the @JsonCreator annotation in ToolUseBlock to point to the constructor with the most arguments, ensuring robust JSON deserialization for all fields. The change is well-supported by a comprehensive new test suite in ToolUseBlockTest.java, which covers various serialization and deserialization scenarios. I've added a couple of suggestions to improve the test code by using more idiomatic JUnit 5 assertions for exception testing, which will enhance readability and maintainability.
| try { | ||
| toolUseBlock.getInput().put("key3", "value3"); | ||
| assertFalse(true, "Expected UnsupportedOperationException"); | ||
| } catch (UnsupportedOperationException e) { | ||
| assertTrue(true); | ||
| } |
There was a problem hiding this comment.
For better readability and to follow modern JUnit 5 practices, it's recommended to use assertThrows to verify that an exception is thrown. This makes the test's intent clearer and more concise than a try-catch block.
assertThrows(UnsupportedOperationException.class, () -> toolUseBlock.getInput().put("key3", "value3"));| try { | ||
| toolUseBlock.getMetadata().put("meta3", "data3"); | ||
| assertFalse(true, "Expected UnsupportedOperationException"); | ||
| } catch (UnsupportedOperationException e) { | ||
| assertTrue(true); | ||
| } |
There was a problem hiding this comment.
Similar to the previous comment, using assertThrows is the preferred way to test for exceptions in JUnit 5. It improves the clarity and conciseness of the test.
assertThrows(UnsupportedOperationException.class, () -> toolUseBlock.getMetadata().put("meta3", "data3"));
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
1 participant
AgentScope-Java Version
[The version of AgentScope-Java you are working on, e.g. 1.0.8, check your pom.xml dependency version or run
mvn dependency:tree | grep agentscope-parent:pom(only mac/linux)]Description
[Please describe the background, purpose, changes made, and how to test this PR]
Checklist
Please check the following items before code is ready to be reviewed.
mvn spotless:applymvn test)