-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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 tests using yaml #6457
Conversation
60d9e9e
to
ab208e5
Compare
@erohmensing I've updated the documentation to reflect the changes to the story testing parts. Also updated the chitchat-faq's page along the way. (and some other smaller things I noticed in the docs). do you have time to review the docs changes? |
@degiz this is ready for a code review, still need to add some tests but I think giving the code a review first helps speed this up. |
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.
Awesome that you were able to pull out that interpreter
from so much stuff 😄
Oops, I didn't see that you just wanted my review on docs changes, sorry 😅 |
No worries, always also happy about any code comments 🙂 |
Co-authored-by: Ella Rohm-Ensing <[email protected]>
Resolved issues in the following files via DeepSource Autofix: 1. rasa/cli/utils.py
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.
Great job with refactoring! 🚀
One general note:
I thought we've using the test stories
term now? Slack link.
So, for example, the name of the files would be failed_test_stories.yml
, test_stories.yml
, successful_stories.yml
etc.
Co-authored-by: Alexander Khizov <[email protected]>
good point about the naming, I'll make sure it is test_stories everywhere instead of test conversations. I'll keep some occurrences in the docs page though for discoverability |
@degiz addressed all the comments - still need an accept though 😉 |
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.
Let's go 🚀 👍
@@ -135,8 +142,8 @@ def test_get_label_set(targets, exclude_label, expected): | |||
assert set(expected) == set(actual) | |||
|
|||
|
|||
async def test_interpreter_passed_to_agent( |
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.
@tmbo Why did you remove this test?
""" | ||
raise NotImplementedError() | ||
|
||
async def get_stories( | ||
self, | ||
interpreter: "NaturalLanguageInterpreter" = RegexInterpreter(), |
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.
We are changing an interface here which might be re-implemented by a community member. Can we please add a removal changelog?
* Create conversation_tests.yml * implement conversation tests using the new yml format * fixed linter issues * renamed test_conversatiosn to stories * fixed story reader detection * fixed remaining tests * fixed type error * fixed some smaller style issues * added documentation * style improvements * Update rasa/core/training/story_reader/markdown_story_reader.py Co-authored-by: Ella Rohm-Ensing <[email protected]> * adressed review comments (and linter error) * added changelog item * fixed failing tests * added tests * fixed linting * fixed classification test * fixed some more style errors deepsource found * fixed some more deepsource issues * fixed some more deepsource issues... * Autofix issues in 1 files Resolved issues in the following files via DeepSource Autofix: 1. rasa/cli/utils.py * added comments for public funcionts * trying to fix tests * fixed server tests * Apply suggestions from code review Co-authored-by: Alexander Khizov <[email protected]> * adressed review comments * renamed files to stay in test stories naming convention * removed trailing whitespace * fixed shitty tests * added default argument * Update test_multi_project.py * fixed windows errors * trying to fix escaping issues * fixed linter Co-authored-by: Ella Rohm-Ensing <[email protected]> Co-authored-by: deepsource-autofix[bot] <62050782+deepsource-autofix[bot]@users.noreply.github.com> Co-authored-by: Alexander Khizov <[email protected]>
Proposed changes:
UserUttered
events contain the fullparse_data
conversation_tests.md
totest_conversations.yml
(as well as remainingnlu.md
andstory.md
ones)rasa init
in yaml formatStatus (please check what you already did):
black
(please check Readme for instructions)