fix: Emit deprecation if filter string does not contain @ prefixes #417
Open
acoulton wants to merge 5 commits intoBehat:masterfrom
Open
fix: Emit deprecation if filter string does not contain @ prefixes #417acoulton wants to merge 5 commits intoBehat:masterfrom
acoulton wants to merge 5 commits intoBehat:masterfrom
Conversation
The verbosity of the existing tests make it slightly hard to see exactly what syntax is and is not covered by tests. Particularly because there is some duplication between the examples for features and scenarios. It will become even more difficult when we add more examples for syntax that is not currently covered. Therefore, refactor to use a single test method with data providers for the expected matching / not matching cases. This commit only refactors the cases that were already tested.
These cases were not fully covered by existing tests, and help to clarify the expected behaviour of the filter.
Refactor the existing test to cover more cases, in particular the interaction between deprecation and matching and the interaction with the separate `testTagFilterThatIsAllWhitespaceIsIgnored` which was not actually testing any strings with whitespace.
Whitespace is allowed within a tag filter expression either side of the `&&` or `,` operators. This reinstates historical behaviour where these strings were allowed. The deprecation on filters containing whitespace is now only triggered if the tags themselves contain whitespace, after trimming any space around operators.
When we added the BC layer to reintroduce support for filter strings without `@` prefixes, we didn't add a runtime deprecation because Behat didn't have a way to capture and report these. However, since Behat does now have a deprecation collector / reporter (which does not fail the build by default) we can now safely emit the deprecation.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #417 +/- ##
============================================
+ Coverage 95.71% 95.79% +0.07%
- Complexity 685 689 +4
============================================
Files 45 45
Lines 2031 2117 +86
============================================
+ Hits 1944 2028 +84
- Misses 87 89 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
This PR builds on #416 - only e70deb9 is new in this branch.
When we added the BC layer to reintroduce support for filter strings without
@prefixes, we didn't add a runtime deprecation because Behat didn't have a way to capture and report these.However, since Behat does now have a deprecation collector / reporter (which does not fail the build by default) we can now safely emit the deprecation.
Fixes #408