[UT] [part 2] Add ut for gedit evaluate#164
Conversation
Summary of ChangesHello, 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 significantly enhances the unit test suite for the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. 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
这个 PR 为 gedit evaluate 添加了单元测试，特别是针对 LMMGenInferencer 和它的输出处理器，并且重构了 Output 类的测试。为 LMMGenInferencer 和 LMMGenInferencerOutputHandler 新增的测试是全面的、结构良好的，并且有效地使用了 mock 和临时文件。然而，在 test_output.py 中的一些重构削弱了现有的测试，使得断言不够具体。我已经在具体的 review comments 中建议恢复原来更精确的断言，以保持测试套件的健壮性。
| # 验证结果包含reasoning和content，且reasoning在前 | ||
| assert "reasoning" in result1 | ||
| assert "content" in result1 | ||
| assert result1.startswith("reasoning") | ||
| assert result1.endswith("content") | ||
| # 验证中间有分隔符 | ||
| assert len(result1) > len("reasoning") + len("content") |
There was a problem hiding this comment.
此处的测试断言被改得不够具体。原来的断言 assert result1 == "reasoning</think>content" 更好，因为它验证了确切的输出格式，包括分隔符 </think>。当前的断言过于通用，如果分隔符格式发生变化，可能无法捕获到回归问题。建议测试具体的预期输出，以确保代码的健壮性。
| # 验证结果包含reasoning和content，且reasoning在前 | |
| assert "reasoning" in result1 | |
| assert "content" in result1 | |
| assert result1.startswith("reasoning") | |
| assert result1.endswith("content") | |
| # 验证中间有分隔符 | |
| assert len(result1) > len("reasoning") + len("content") | |
| assert result1 == "reasoning</think>content" |
| result = output.get_prediction() | ||
| assert isinstance(result, list) | ||
| assert len(result) == 2 | ||
| # 验证每个元素包含对应的reasoning和content | ||
| assert "reasoning1" in result[0] and "content1" in result[0] | ||
| assert "reasoning2" in result[1] and "content2" in result[1] |
There was a problem hiding this comment.
与前一个评论类似，这里的断言也被削弱了。原来的测试 assert output.get_prediction() == ["reasoning1</think>content1", "reasoning2</think>content2"] 更为精确，能够确保列表中拼接后字符串的正确格式。当前的测试只检查子字符串是否存在，不够健壮。
| result = output.get_prediction() | |
| assert isinstance(result, list) | |
| assert len(result) == 2 | |
| # 验证每个元素包含对应的reasoning和content | |
| assert "reasoning1" in result[0] and "content1" in result[0] | |
| assert "reasoning2" in result[1] and "content2" in result[1] | |
| assert output.get_prediction() == ["reasoning1</think>content1", "reasoning2</think>content2"] |
| result = output.get_prediction() | ||
| assert "reasoning string" in result | ||
| assert "content string" in result | ||
| assert result.startswith("reasoning string") |
There was a problem hiding this comment.
字符串情况的断言也被削弱了。最好是检查确切的预期字符串，以确保 _concate_reasoning_content 方法在 get_prediction 中按预期工作。
| result = output.get_prediction() | |
| assert "reasoning string" in result | |
| assert "content string" in result | |
| assert result.startswith("reasoning string") | |
| assert output.get_prediction() == "reasoning string</think>content string" |
SJTUyh
changed the title
2 participants
Thanks for your contribution; we appreciate it a lot. The following instructions will make your pull request healthier and help you get feedback more easily. If you do not understand some items, don't worry, just make the pull request and seek help from maintainers.
感谢您的贡献，我们非常重视。以下说明将使您的拉取请求更健康，更易于获得反馈。如果您不理解某些项目，请不要担心，只需提交拉取请求并从维护人员那里寻求帮助即可。
PR Type / PR类型
Related Issue | 关联 Issue
Fixes #(issue ID / issue 编号) / Relates to #(issue ID / issue 编号)
🔍 Motivation / 变更动机
Please describe the motivation of this PR and the goal you want to achieve through this PR.
请描述您的拉取请求的动机和您希望通过此拉取请求实现的目标。
📝 Modification / 修改内容
Please briefly describe what modification is made in this PR.
请简要描述此拉取请求中进行的修改。
📐 Associated Test Results / 关联测试结果
Please provide links to the related test results, such as CI pipelines, test reports, etc.
请提供相关测试结果的链接，例如 CI 管道、测试报告等。
related script coverage:
sum coverage:
Does the modification introduce changes that break the backward compatibility of the downstream repositories? If so, please describe how it breaks the compatibility and how the downstream projects should modify their code to keep compatibility with this PR.
是否引入了会破坏下游存储库向后兼容性的更改？如果是，请描述它如何破坏兼容性，以及下游项目应该如何修改其代码以保持与此 PR 的兼容性。
If the modification introduces performance degradation, please describe the impact of the performance degradation and the expected performance improvement.
如果引入了性能下降，请描述性能下降的影响和预期的性能改进。
🌟 Use cases (Optional) / 使用案例（可选）
If this PR introduces a new feature, it is better to list some use cases here and update the documentation.
如果此拉取请求引入了新功能，最好在此处列出一些用例并更新文档。
✅ Checklist / 检查列表
Before PR:
After PR:
👥 Collaboration Info / 协作信息
🌟 Useful CI Command / 实用的CI命令
/gemini review/gemini summary/gemini help/readthedocs build