-
Notifications
You must be signed in to change notification settings - Fork 13
Error message improvement #1013
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
People who forget to give any tests do not understand that 'subset candidates' are expected, so better to rework the message.
WalkthroughA new boolean attribute, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Optimize
User->>Optimize: test_path() / scan()
Optimize->>Optimize: Set input_given = True
User->>Optimize: run()
alt No tests found
alt input_given == True
Optimize->>User: Error: Arguments did not match any tests
else input_given == False
Optimize->>User: Error: No tests provided (with guidance)
end
else Tests found
Optimize->>User: Proceed with subset
end
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
⏰ Context from checks skipped due to timeout of 90000ms (13)
🔇 Additional comments (5)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR improves the error message shown when no subset candidates (tests) are provided. The changes include:
- Updating the test for asserting the error message.
- Introducing conditional error messages in the subset command based on whether any test input was provided.
- Adding a link to the documentation for better guidance.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tests/commands/test_subset.py | Updated test assertion to check for improved error message. |
| launchable/commands/subset.py | Added conditional error message logic and documentation link. |
Comments suppressed due to low confidence (2)
tests/commands/test_subset.py:292
- Ensure that the test consistently triggers the branch expected to output a message containing this substring; if conditions change (e.g., self.input_given being true), the test may need to account for the alternate error message.
self.assertIn("use the `--get-tests-from-previous-sessions` option", result.stdout)
launchable/commands/subset.py:518
- [nitpick] Consider aligning the error message details between the 'input given' and 'no input' cases so that users receive consistent guidance, even when inputs are provided but unmatched.
if self.input_given:
|



People who forget to give any tests do not understand that 'subset candidates' are expected, so better to rework the message.
Summary by CodeRabbit