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

Fix/issue54 multiple example tables #59

Conversation

Wayne-Morgan
Copy link
Contributor

@Wayne-Morgan Wayne-Morgan commented Apr 10, 2017

This branch addresses issue #54. There were three changes required to fix the issue:

  • Fix invocationForScenarioOutline so it uses the appropriate examples table. example is passed in to the method rather than just taking the first.

  • Add tags to the CCIExample class. Populate the tags from the dictionary provided by the parser.

  • Change the filtering in CCIFeaturesManager to respect the examples tags: For any scenario outline which is not excluded, remove any examples which are excluded, and then remove the scenario outline if all examples have been removed. For any scenario outline which has not been included, remove any examples which have not been included, and then remove the scenario outline if all examples have been removed.

@brentleyjones
Copy link
Collaborator

I have a vacation this week. I'll try to review this early next week. Thank you @Wayne-Morgan for your contribution.

@brentleyjones brentleyjones changed the base branch from master to develop April 24, 2017 20:38
@brentleyjones brentleyjones self-requested a review April 24, 2017 20:39
@brentleyjones
Copy link
Collaborator

@Wayne-Morgan In the meantime though, if you want to resolve the merge conflict that would be 💯.

@Wayne-Morgan
Copy link
Contributor Author

Thanks @brentleyjones. I'll look forward to your feedback.

Copy link
Collaborator

@brentleyjones brentleyjones left a comment

Choose a reason for hiding this comment

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

The logic checks out 👍.

Preferably there wouldn't be all the changes to the whitespace, since it made it harder to review, and it's technically mixing empty and non-empty lines together, but it wasn't enough for me to reject this.

@brentleyjones brentleyjones merged commit c13beca into Ahmed-Ali:develop May 4, 2017
@Wayne-Morgan
Copy link
Contributor Author

Yeah. Sorry about that. My editor is set to remove trailing whitespace, and I didn't realise until after the commit. Thanks.

@Wayne-Morgan Wayne-Morgan deleted the fix/issue54_multiple_example_tables branch May 8, 2017 10:51
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