-
-
Notifications
You must be signed in to change notification settings - Fork 616
test: Update expected output to reflect new gherkin translations #1635
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
Conversation
|
Urgh, of course the output of the I'm not sure of the best solution for this, particularly because of course now that we're updating gherkin regularly it could easily recur in future (and perhaps on a gherkin release that also contains more significant changes).
I think the wildcard approach is the least likely to unexpectedly break in future, with a feature like: Then the output should contain the following with russian translations:
"""
# language: ru
[%GHERKIN_FEATURES_KEYWORDS%]: Internal operations
In order to stay secret
As a secret organization
We need to be able to erase past agents' memory
[%GHERKIN_SCENARIO_KEYWORDS%]:
[%GHERKIN_GIVEN_KEYWORDS%] there is agent A
... etc ...
"""However, this means we'd be asking gherkin at runtime for both the implementation and the expected test result. Which means the feature provides less confidence about the actual output than it does now... @carlos-granados what do you think - any better ideas? |
|
@acoulton Mmm, it is a difficult problem. Of the solutions that you mentioned I feel that "We could introduce a behat profile with tags to allow us to skip specific scenarios on the composer --lowest build" is the one that sounded better as it would allow us to confirm the behaviour with both versions. Can you maybe check about the feasibility of this option? |
|
Yeah I think that might be best, but (because we don't pin the upper gherkin version) would still mean the "not-lowest" build will start failing on all branches if/when a future Gherkin release adds more russian translations until we update the features and merge/rebase them into all Behat branches. Given that, I'm wondering this evening if we should just swap that example from Russian to a language we know is the same in all current Gherkin versions, and hope that it doesn't change in future... One of the latin-based languages that's been in gherkin for a long time would probably be fairly stable, ideally one with at least a couple of utf8 diacritics to prove the character set works as expected... That would definitely be the quickest solution, WDYT? |
|
Ah, yes, that would work too, let's go with that |
f5e9c20 to
2c26385
Compare
Behat/Gherkin@4.13.0 introduced a number of new translations. These are BC for end-users (when parsing the features, any of the past-or-present translations will be recognised). However, our `--story-syntax` command outputs all known translations when displaying the example feature. This means the expected output in our tests has to exactly match the Behat/Gherkin translations across all supported versions. This caused CI to break due to extra Russian translations between 4.12 and 4.13. I have switched the test to Greek because it appears to be stable (has not changed in cucumber/gherkin in the last 8 years), still contains UTF-8 to prove character sets work as expected, and has a variety of keywords with/without synonyms.
2c26385 to
4fc1d69
Compare
|
@carlos-granados I went for Greek as it has extended characters and according to the |
carlos-granados
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good, I hope that the Greek is ok and we don't get any Greek users saying "what the heck is that?" 🤣
|
I hope not - if so at least we can direct them to cucumber/gherkin so it won't be our fault 😆 |
Behat/Gherkin@4.13.0 introduced a number of new translations. Although these are BC for end-users, the new translation options appear in our
--story-syntaxoutput so the expectation for this needs to be updated.