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

Add print to confirm that the failed_test_stories.yml has been saved #6793

Merged
merged 14 commits into from
Dec 1, 2020

Conversation

Henrike100
Copy link
Contributor

@Henrike100 Henrike100 commented Sep 25, 2020

Proposed changes:

Default path (results)
img1

User specified path
img2

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)

@CLAassistant
Copy link

CLAassistant commented Sep 25, 2020

CLA assistant check
All committers have signed the CLA.

@sara-tagger
Copy link
Collaborator

Thanks for submitting a pull request 🚀 @erohmensing will take a look at it as soon as possible ✨

Copy link
Contributor

@erohmensing erohmensing left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution! Works great. I provided some suggestions based on our internal engineering guidelines.

rasa/cli/test.py Outdated
@@ -17,6 +17,8 @@
)
import rasa.shared.utils.validation as validation_utils
import rasa.cli.utils
from rasa.shared.utils.cli import print_info
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
from rasa.shared.utils.cli import print_info
import rasa.shared.utils.cli

We don't import methods directly. Instead we import the module they came from and call them explicitly.

Copy link
Contributor

Choose a reason for hiding this comment

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

And move to line 9, after the other import rasa statements :)

rasa/cli/test.py Outdated
@@ -101,6 +103,8 @@ def run_core_test(args: argparse.Namespace) -> None:
else:
test_core_models(args.model, stories, output)

print_info("Failed stories written to {}/{}".format(output, FAILED_STORIES_FILE))
Copy link
Contributor

@erohmensing erohmensing Sep 25, 2020

Choose a reason for hiding this comment

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

Suggested change
print_info("Failed stories written to {}/{}".format(output, FAILED_STORIES_FILE))
rasa.shared.utils.cli.print_info(f"Failed stories written to '{output}/{FAILED_STORIES_FILE}'")

@Henrike100
Copy link
Contributor Author

@erohmensing thank you for reviewing the PR. I made the suggested changes. When you can, review the PR again, please.

rasa/cli/test.py Outdated
@@ -109,6 +110,10 @@ def run_core_test(args: argparse.Namespace) -> None:
else:
test_core_models(args.model, stories, output)

rasa.shared.utils.cli.print_info(
f"Failed stories written to '{output}/{FAILED_STORIES_FILE}'"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
f"Failed stories written to '{output}/{FAILED_STORIES_FILE}'"
f"Failed stories written to '{os.path.join(output, FAILED_STORIES_FILE)}'"

(otherwise, this will create wrong paths on windows)

Copy link
Member

@tmbo tmbo left a comment

Choose a reason for hiding this comment

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

thanks a lot for this improvement 🚀 (and sorry for the delay in approving it)

@tmbo tmbo merged commit ed509eb into RasaHQ:master Dec 1, 2020
eanile added a commit to UTkzhang/rasa that referenced this pull request Dec 2, 2020
* backport fix

* backport pika fix

* pika fix backport

* fix tests

* test fixes

* remove logger statement

* Updated version of PikaMessageProcessor from the source branch

* Small bugfix

* changelog

* prepared release of version 1.10.16

* Update CHANGELOG.rst

Co-authored-by: Ella Rohm-Ensing <[email protected]>

* add robots.txt and redirection headers

* bump rasabaster version

* Fix Pika multiprocessing for 1.10.x

* Reformatting

* Changelog entry

* Updated tests

* is_ready method is back

* uninstall cached pydocstyle

* also uninstall pydocstyle

* uninstall in poetry env

* Removing f-string

* NoReturn -> None

* prepared release of version 1.10.17

* Fix incorrect attribute for key prefix in RedisTrackerStore

Fix RasaHQ#7306

* Add changelog entry

* Remove the set-env command - 1.10.x (RasaHQ#7367)

* Fix DynamoDB TrackerStore (RasaHQ#7340)

* attempting to fix dynamo

* fixed

* fixed import

* applied some more fixes

* added deprecation warning for DynamoDB tracker store attribute

* added changelog description for PR 7340

* fix formatting

Co-authored-by: Tom Bocklisch <[email protected]>

* prepared release of version 1.10.18 (RasaHQ#7381)

* Update docstrings

* Update CHANGELOG.mdx

* line length fix

* Add print to confirm that the failed_test_stories.yml has been saved (RasaHQ#6793)

* Add print to confirm that the failed_test_stories.yml has been saved (RasaHQ#3998)

Co-authored-by: Henrike100 <[email protected]>

* Change the way to show the file name

* files formatted with black

* add changelog

* Update test core

* Formatted with black

* Update print

Co-authored-by: mbslet <[email protected]>

* Amend response schema to support missing top-level keys (RasaHQ#7232)

* amend response schema to support missing top level keys

* changelog entry

* download mitie from github instead of s3 (RasaHQ#7434)

Co-authored-by: melindaloubser1 <[email protected]>
Co-authored-by: alwx <[email protected]>
Co-authored-by: Roberto <[email protected]>
Co-authored-by: Melinda Loubser <[email protected]>
Co-authored-by: Ella Rohm-Ensing <[email protected]>
Co-authored-by: m-vdb <[email protected]>
Co-authored-by: Tobias Wochinger <[email protected]>
Co-authored-by: Duc-Minh Phan <[email protected]>
Co-authored-by: Federico Tedin <[email protected]>
Co-authored-by: Tomasz Czekajlo <[email protected]>
Co-authored-by: Tom Herold <[email protected]>
Co-authored-by: Tom Bocklisch <[email protected]>
Co-authored-by: Henrique Martins <[email protected]>
Co-authored-by: mbslet <[email protected]>
Co-authored-by: pheel <[email protected]>
@iurisevero iurisevero deleted the failed-storie-message branch February 21, 2021 03:43
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.

Print out "Failed stories written to results/failed_stories.md" at the end of running rasa test core
6 participants