[for testing purposes] : Run latest mypy on fragments#2578
[for testing purposes] : Run latest mypy on fragments#2578
Conversation
0d99262 to
e49ad70
Compare
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 primarily focuses on enhancing the testing infrastructure by integrating 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
|
There was a problem hiding this comment.
Code Review
This pull request adds mypy checking for generated code fragments by modifying the noxfile.py to run pytest and then mypy. The implementation is functionally correct, but it introduces an inefficiency by generating code from fragments twice. My review includes a suggestion to refactor the test execution flow to avoid this redundant work and improve performance.
noxfile.py
Outdated
|
|
||
| def run_tests(mypy_only=False): | ||
| """Helper to handle the parallel vs sequential toggle.""" | ||
| tester = FragTester(session, use_ads_templates, mypy_only=mypy_only) | ||
|
|
||
| if is_parallel: | ||
| with ThreadPoolExecutor() as p: | ||
| results = p.map(tester, frag_files) | ||
| session.log("".join(results)) | ||
| else: | ||
| for frag in frag_files: | ||
| session.log(tester(frag)) | ||
|
|
||
| output = "".join(all_outs) | ||
| session.log(output) | ||
| else: | ||
| tester = FragTester(session, use_ads_templates) | ||
| for frag in frag_files: | ||
| session.log(tester(frag)) | ||
| run_tests(mypy_only=False) | ||
| session.install("mypy", "types-protobuf", "types-requests") |
There was a problem hiding this comment.
The current implementation generates code from fragments twice: once for running pytest and a second time for mypy. This happens because run_tests is called twice, and code generation is part of the FragTester callable it uses.
This is inefficient. Consider refactoring to generate code only once per fragment. You could generate all fragment code first, then run pytest on all of them, and finally run mypy on all of them. This would avoid the redundant generation step and speed up the tests.
fae9bd7 to
d8131f2
Compare
d8131f2 to
6410633
Compare
No description provided.