-
-
Notifications
You must be signed in to change notification settings - Fork 29
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 scenario outlines' examples insertions inside the DocString's content. #146
Add scenario outlines' examples insertions inside the DocString's content. #146
Conversation
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.
To preserve this work, I decided to merge it and leave the other changes I mentioned for future releases.
Please address the comments first.
source/Xunit.Gherkin.Quick/CoreModel/Scenario/GherkinScenarioOutlineExtensions.cs
Outdated
Show resolved
Hide resolved
source/Xunit.Gherkin.Quick.ProjectConsumer/Placeholders/Placeholders.cs
Outdated
Show resolved
Hide resolved
source/Xunit.Gherkin.Quick.ProjectConsumer/Placeholders/Placeholders.cs
Outdated
Show resolved
Hide resolved
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.
Fixed naming based on the comments, lgtm.
source/Xunit.Gherkin.Quick.ProjectConsumer/Placeholders/Placeholders.cs
Outdated
Show resolved
Hide resolved
source/Xunit.Gherkin.Quick.ProjectConsumer/Placeholders/Placeholders.cs
Outdated
Show resolved
Hide resolved
@ttutisani The comments have been addressed! |
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.
LGTM
Can't merge, your tests fail due to line endings. I'm assuming you are on Linux/Unix, and I'm on windows. You need to use Environment.NewLine in your tests instead of "Enter" to separate lines. This is from Placeholders.cs. All errors come from there. You have a string in the assertions that has new-lines. This approach is generally a good practice to write cross-platform tests. |
Thanks! My bad, I'm too used to writing and running all the tests (even production ones) on Unix-based servers. I'll fix it tomorrow. |
Fixed + tested on Windows. Gonna run it on Mac tomorrow just in case. |
Tests pass on Mac as well! |
This pull request fixes this issue by updating DocStrings' contents while running Scenario Outline tests based on the Examples table.
Following this similar issue's solution, both a unit test and a behaviour test are added.