Skip to content
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.

Sign up for GitHub

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

Jump to bottom

rasa init should include tests by default #5416

Merged
merged 7 commits into from
Mar 19, 2020
Merged

rasa init should include tests by default #5416

merged 7 commits into from
Mar 19, 2020

Conversation

alwx
Copy link
Contributor

@alwx alwx commented Mar 12, 2020

Proposed changes:

  • include default e2e tests to an initial project created with rasa init command

Status (please check what you already did):

  • updated the changelog (please check changelog for instructions)

@wochinge
Copy link
Contributor

@alwx I guess I'm the reviewer not the assignee right?

@alwx alwx requested a review from wochinge March 16, 2020 09:11
@alwx
Copy link
Contributor Author

alwx commented Mar 16, 2020

@wochinge sorry! yes :)

wochinge
wochinge reviewed Mar 17, 2020
View reviewed changes
docs/user-guide/evaluating-models.rst Outdated Show resolved Hide resolved
docs/user-guide/evaluating-models.rst

.. code-block:: bash

$ rasa test
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@erohmensing do we need the dollar sign or can we remove it?

rasa/cli/initial_project/tests/conversation_tests.md Show resolved Hide resolved
@alwx alwx requested a review from wochinge March 17, 2020 14:30
@alwx alwx force-pushed the rasa-init-tests branch from 38028d7 to e418583 Compare March 18, 2020 10:07
@alwx alwx requested a review from wochinge March 18, 2020 11:41
@alwx alwx force-pushed the rasa-init-tests branch 2 times, most recently from fa3cad9 to f0fd0b0 Compare March 18, 2020 11:50
wochinge
wochinge approved these changes Mar 18, 2020
View reviewed changes
Copy link
Contributor

@wochinge wochinge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great. I'd add a comment + docs link to the testing file as we said in the comment, but then feel free to merge 🚀

@alwx alwx force-pushed the rasa-init-tests branch 2 times, most recently from ecaf1b7 to 81e1882 Compare March 18, 2020 15:27
@tttthomasssss tttthomasssss added this to the Rasa 1.9 milestone Mar 18, 2020
@tttthomasssss
Copy link
Contributor

@alwx @wochinge is this ready to go into 1.9.0?

@alwx
Copy link
Contributor Author

alwx commented Mar 18, 2020

@tttthomasssss for sure. I am only waiting for #5438 to be merged, and then this one will be merged, too.

@alwx alwx force-pushed the rasa-init-tests branch from 217e3ca to 94f0e6e Compare March 19, 2020 09:44
@alwx alwx force-pushed the rasa-init-tests branch from 94f0e6e to e3cfad6 Compare March 19, 2020 09:53
@alwx alwx merged commit 82e5298 into master Mar 19, 2020
@tmbo tmbo deleted the rasa-init-tests branch May 1, 2020 10:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wochinge wochinge wochinge approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Rasa 1.9
Development

Successfully merging this pull request may close these issues.
3 participants
@alwx @wochinge @tttthomasssss