Conversation
This is meant to be a conversation starter, rather than the final implementation.
Summary of ChangesHello @hcarreras, 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 introduces an initial draft of Abtion's comprehensive testing strategy, intended to serve as a conversation starter for defining company-wide testing expectations. It replaces a more specific document on Test-Driven Development with a broader framework that covers general testing principles, CI requirements, and specific guidelines for both backend and frontend development. 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
|
There was a problem hiding this comment.
Code Review
This pull request introduces a new, comprehensive testing strategy document, replacing the previous one focused on TDD. The new guidelines are clear and set solid expectations for testing across projects, including rules for critical flows, bug fixes, CI/CD, and test coverage for both backend and frontend. My review focuses on improving the clarity and conciseness of the new documentation to make the rules even more effective and less open to misinterpretation. The suggestions aim to reduce redundancy and refine some of the phrasing.
| **CI cannot merge if red** — this is non-negotiable. | ||
|
|
||
| All tests must pass before code can be merged. No exceptions. |
There was a problem hiding this comment.
These two statements are slightly redundant. The first statement, 'CI cannot merge if red — this is non-negotiable', already implies that all tests must pass. You could combine them for conciseness and impact.
| **CI cannot merge if red** — this is non-negotiable. | |
| All tests must pass before code can be merged. No exceptions. | |
| **CI pipelines must pass before merging.** A failing (red) pipeline is a hard block, and this rule is non-negotiable. |
| - **80% test coverage** is enforced as a CI rule | ||
| - Coverage gates are mandatory and will block merges if not met |
There was a problem hiding this comment.
These two points are redundant. 'Enforced as a CI rule' already implies that mandatory coverage gates will block merges. You can combine them into a single, more concise point.
| - **80% test coverage** is enforced as a CI rule | |
| - Coverage gates are mandatory and will block merges if not met | |
| - A minimum of **80% test coverage** is enforced through mandatory CI gates that block merges if the threshold is not met. |
| - **Don't unit test the UI** — avoid testing implementation details of visual components | ||
| - **Test component logic** — focus on behavior, state management, and business logic within components |
There was a problem hiding this comment.
The statement 'Don't unit test the UI' is very absolute and could be misinterpreted. While the intent to avoid testing implementation details is correct, this phrasing might discourage valuable component-level testing. Consider rephrasing to emphasize testing behavior over implementation, which seems to be the core message of these two points.
| - **Don't unit test the UI** — avoid testing implementation details of visual components | |
| - **Test component logic** — focus on behavior, state management, and business logic within components | |
| - **Test behavior, not implementation details.** UI tests should focus on what the component does from a user's perspective (behavior, state changes, business logic) rather than its internal structure. This creates more robust and less brittle tests. |
This is meant to be a conversation starter, rather than the final implementation.