fix(test-utils): resolve circular dependency#16731
Conversation
Summary of ChangesHello @sehoon38, 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 a critical architectural issue by eliminating a circular dependency that existed between the Highlights
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
|
|
Hi @sehoon38, thank you so much for your contribution to Gemini CLI! We really appreciate the time and effort you've put into this. We're making some updates to our contribution process to improve how we track and review changes. Please take a moment to review our recent discussion post: Improving Our Contribution Process & Introducing New Guidelines. Key Update: Starting January 26, 2026, the Gemini CLI project will require all pull requests to be associated with an existing issue. Any pull requests not linked to an issue by that date will be automatically closed. Thank you for your understanding and for being a part of our community! |
|
Size Change: -2 B (0%) Total Size: 23.1 MB ℹ️ View Unchanged
|
There was a problem hiding this comment.
Code Review
This pull request resolves a circular dependency between test-utils and core by hardcoding two constants, GEMINI_DIR and DEFAULT_GEMINI_MODEL, within test-utils. While this effectively breaks the cycle, it introduces code duplication. This can create maintenance challenges, as changes to these constants in the core package will require manual updates in test-utils to prevent inconsistencies. My review includes a suggestion to consider alternative, more robust solutions like dependency injection or creating a shared constants package to mitigate this risk.
| const GEMINI_DIR = '.gemini'; | ||
| const DEFAULT_GEMINI_MODEL = 'gemini-2.5-pro'; |
There was a problem hiding this comment.
While hardcoding these constants breaks the circular dependency, it introduces duplication. If these values change in @google/gemini-cli-core, they will also need to be updated here manually. This could lead to tests becoming out of sync with the application's behavior, especially for DEFAULT_GEMINI_MODEL which might change over time.
To create a more maintainable solution, consider one of the following alternatives:
-
Dependency Injection: Modify
TestRigto accept these values through its constructor orsetupmethod. The test files that useTestRigcan then import the constants from@google/gemini-cli-coreand pass them in. This keepstest-utilsdecoupled while avoiding duplication. -
Shared Constants Package: For a more robust long-term solution, you could extract these and other shared constants into a new, lower-level package (e.g.,
@google/gemini-cli-constants). Bothcoreandtest-utilscould then depend on this new package without creating a circular dependency.
|
using this to resolve #16730 |
Summary
Resolves a circular dependency between
test-utilsandcoreby hardcodingGEMINI_DIRandDEFAULT_GEMINI_MODELintest-utilsinstead of importing them from@google/gemini-cli-core.Details
Previously,
packages/test-utils/src/test-rig.tsimported constants from@google/gemini-cli-core. This created a circular dependency chain. This PR breaks that chain by defining the necessary constants locally within the test utility.Related Issues
Fixes #16732
How to Validate
Run the tests to ensure that the test rig still functions correctly:
Pre-Merge Checklist