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

[ATO-1652] Add unit tests for GRPCActionServerWebhook #1112

Merged
merged 1 commit into from
Jun 26, 2024

Conversation

radovanZRasa
Copy link
Contributor

Add unit tests for GRPCActionServerWebhook

Status (please check what you already did):

  • made PR ready for code review
  • added some tests for the functionality
  • updated the documentation in the rasaHQ/rasa
  • updated the changelog (please check changelog for instructions)
  • reformat files using black (please check Readme for instructions)

@radovanZRasa radovanZRasa force-pushed the tests/ATO-1652-add-grpc-unit-tests branch from 3698521 to d99f6fe Compare June 24, 2024 13:11
Copy link
Member

@ancalita ancalita 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 💯 The only suggestion I have is to move some of the fixtures either in the same module as test_grpc_server, or in other cases even prepend them with expected_response etc and not have them as fixtures since they're not being reused, it was a bit difficult to understand the test constantly switching between the two files.

@radovanZRasa radovanZRasa force-pushed the tests/ATO-1652-add-grpc-unit-tests branch from d99f6fe to 41513c5 Compare June 25, 2024 10:33
Copy link
Member

@ancalita ancalita left a comment

Choose a reason for hiding this comment

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

🎉

@radovanZRasa radovanZRasa merged commit f8b2d70 into main Jun 26, 2024
16 checks passed
@radovanZRasa radovanZRasa deleted the tests/ATO-1652-add-grpc-unit-tests branch June 26, 2024 11:59
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.

2 participants