Fix data validator ignore_warnings logic #11844
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fix failing tests logic Add test_verify_utterances_does_not_error_when_no_utterance_template_provided
|
|
tmbo
approved these changes
Dec 7, 2022
auto-merge was automatically disabled
December 8, 2022 08:18
Head branch was pushed to by a user without write access
My pleasure. I fixed one linter issue but I see some other validation errors in CI which I am not aware of... |
vitordiniz25
approved these changes
Dec 15, 2022
|
@tmbo What needs to happen for this PR to get merged? |
|
I've just approved the CI runs. Once they go green the PR should be automatically merged. Thanks for your contribution 🙏🏻 |
|
Could not update branch. Most likely this is due to a merge conflict. Please update the branch manually and fix any issues. Error: HttpError: user doesn't have permission to update head repository |
auto-merge was automatically disabled
February 18, 2023 00:47
Head branch was pushed to by a user without write access
|
@m-vdb I think I've fixed the issue in the integration test. |
|
it seems that the
|
This is strange, if I run the tests locally they pass: |
|
@m-vdb One change I observed is that I am using Python 3.10 and the failing test uses 3.8. Not sure if that is intended, but different tests in the set use different Python versions (integration tests are using 3.10, so I installed that locally). |
|
but it seems these tests are failing on Ubuntu for all the python versions we support: 3.7, 3.8, 3.9 and 3.10 😬 |
Right, then I don't know what is happening. The tests pass on my local Ubuntu system. Furthermore, this seems to be unrelated to my PR since I only changed validation logic. |
|
@m-vdb I've re-tested on Ubuntu 22.04.1 LTS / Python 3.10 and the tests still succeed: |
|
@m-vdb I just found similarly failing tests in PR #12074, for example: https://github.com/RasaHQ/rasa/actions/runs/4232026432/jobs/7351360693 |
|
@timidri I'm going to go ahead and merge this PR, thanks for putting up with our flaky CI checks. We'll make sure to address these pains in the near future |
Thank you! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proposed changes:
ignore_warningsparameter insidevalidator.pyignore_warnings=Falseintest_validator.pytest_verify_utterances_does_not_error_when_no_utterance_template_providedto make sure that utterances specified without a template are not considered a validation errorStatus (please check what you already did):
black(please check Readme for instructions)This PR resolves 11843