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 bug in SelectionSetNode.AddSelections() and update test snap #1142

Merged
merged 2 commits into from
Oct 17, 2019
Merged

Fix bug in SelectionSetNode.AddSelections() and update test snap #1142

merged 2 commits into from
Oct 17, 2019

Conversation

Avilad
Copy link
Contributor

@Avilad Avilad commented Oct 17, 2019

3 character bugfix 😮

@michaelstaib
Copy link
Member

Can you add another test that ads a selection to a field that already has a selection?

@michaelstaib
Copy link
Member

And thanks for your help

@michaelstaib michaelstaib added this to the 10.2.0 milestone Oct 17, 2019
@Avilad
Copy link
Contributor Author

Avilad commented Oct 17, 2019

Can you add another test that ads a selection to a field that already has a selection?

Unless I am misunderstanding you, there is already an existing test for this, which was succeeding because the snapshot file contained output from the buggy method. That was fixed in this PR. But let me know if you meant a different test.

@michaelstaib
Copy link
Member

I see ... misread the snapshot... you are right.

@michaelstaib michaelstaib merged commit d395542 into ChilliCream:version_10_0_0_master Oct 17, 2019
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