fix(test): strip inline tests from remote presets on load#227
Conversation
Preset inline tests were evaluated against the full merged config, causing them to fail when local rules overrode the same patterns with stricter actions (e.g., preset `ask` overridden by local `deny`). Inline tests should verify that their rule matches the given command, not how the entire config evaluates it after downstream overrides. Resolve extends twice: once without local rules to capture the preset-only config, once normally for the merged config. Inline tests whose rule_index falls within the preset range receive the preset config as their evaluation scope via a new `scope_config` field on `TestCase`. Local inline tests and top-level tests continue to use the full merged config.
This reverts commit 46c728b.
Remote preset inline tests are authored for the preset's own rules and should not be evaluated by downstream consumers. Local overrides can change the evaluation result (e.g., preset `ask` overridden by local `deny`), causing preset tests to fail with "expected ask, got deny". Strip `RuleEntry.tests` from remote presets in `load_preset_with` so they are never collected by `parse_test_cases`. Local preset inline tests remain unaffected.
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! The 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 implements the stripping of inline tests from remote presets to prevent them from running in consumer environments. The logic is sound and is added to load_preset_with to handle remote presets. New tests have been added to verify that tests are stripped for remote presets but preserved for local ones. I have one suggestion to improve the assertions in the new test for better maintainability, which is included in the comments.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #227 +/- ##
==========================================
+ Coverage 90.09% 90.14% +0.05%
==========================================
Files 50 50
Lines 10230 10243 +13
==========================================
+ Hits 9217 9234 +17
+ Misses 1013 1009 -4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
The previous commit strips inline tests from remote presets on load, but the docs did not reflect this behavior change.
The previous commit only stripped inline tests (RuleEntry.tests) but missed the top-level config.tests section. A remote preset with a top-level tests block would have its test cases leak into the merged config via Config::merge, and tests.extends paths would resolve against the consumer's base_dir instead of the preset's cache dir.
1 participant
Why
runok testevaluates inline tests from remote presets under local overrides, causing them to fail against the preset author's intentask: 'gh api * --paginate *'inline test fails with "expected ask, got deny" due to a localdenyoverrideWhat
RuleEntry.tests) from remote presets on load, excluding them fromrunok testtest case collection