-
Notifications
You must be signed in to change notification settings - Fork 478
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
Add test-level context #2142
base: main
Are you sure you want to change the base?
Add test-level context #2142
Conversation
@balloob @synesthesiam i know the tests will fail, I've only fixed the EN ones for now, but before undertaking the massive task of correcting all tests in all languages, I'd like to get your feedback on the approach here. |
8f5f980
to
59190a0
Compare
WalkthroughWalkthroughThe changes introduce a new optional "context" field to the validation function and adjust how context is handled in various test files. This includes moving context specifications to higher levels or test-specific contexts and removing some context requirements. These modifications enhance flexibility and organization in specifying test contexts. Changes
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (11)
Additional context usedPath-based instructions (1)
Additional comments not posted (28)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
It should be fine to have 2 identical test sentences if their context is different (e.g. "turn off the lights" should mean "turn off all lights" without context and "turn off the lights in this area" when the satellite's area is provided as context).
Although technically you could have passed such a set of sentences to HA, it was impossible to add them to the intents repo until now due to tests not passing.
Part of the cause was the double use of
intent.context
, both as input context to the test and as expected outcome after the recognition. This PR splits the 2 concerns (input context and expected output context) and allows several identical sentences to be tested, as long as their input contexts differ.Summary by CodeRabbit
New Features
Improvements
Bug Fixes
requires_context
field from the binary sensor intent related to vibration detection, simplifying the configuration.Refactor
Tests