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.

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

Allow tests to be located anywhere, not just in tests directory #8151

Merged
merged 8 commits into from
Mar 11, 2021
Merged

Conversation

alwx
Copy link
Contributor

@alwx alwx commented Mar 9, 2021

Proposed changes:

Status (please check what you already did):

  • added some tests for the functionality
  • updated the documentation
  • updated the changelog (please check changelog for instructions)
  • reformat files using black (please check Readme for instructions)

@alwx alwx changed the title Allow e2e tests to be located anywhere, not just in tests directory Allow tests to be located anywhere, not just in tests directory Mar 9, 2021
@alwx alwx marked this pull request as ready for review March 9, 2021 13:32
@alwx alwx requested review from a team, Tawakalt and wochinge and removed request for a team and Tawakalt March 9, 2021 13:32
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.

💯

tests/shared/test_data.py Outdated Show resolved Hide resolved
rasa/shared/constants.py Show resolved Hide resolved
return (
DEFAULT_E2E_TESTS_PATH in dirname
MarkdownStoryReader._is_tests_file(file_path)
Copy link
Contributor

Choose a reason for hiding this comment

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

optional: I'm a bit torn here. In my opinion this is rather animprovement than a bug and as markdown is deprecated I wouldn't change any of its behavior unless it's a bug. So in my opinion file also to just keep the old behavior here. This might also reduce the effort in case we have to do a follow-up PR for some reason in these parts.

Comment on lines 354 to 356
return Path(file_path).name.startswith(TEST_STORIES_FILE_PREFIX) or Path(
file_path
).name.endswith("_tests.md")
Copy link
Contributor

Choose a reason for hiding this comment

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

mhm, I'd keep the old logic here (check if tests is in dirname`).

@alwx alwx requested a review from wochinge March 11, 2021 09:25
@alwx alwx enabled auto-merge March 11, 2021 13:29
@alwx alwx merged commit 5af7175 into main Mar 11, 2021
@alwx alwx deleted the bug/7763 branch March 11, 2021 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rasa test should search for test files recursively in all sub-directories, not just under tests/
2 participants