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

Test coverage: addTarget add to main project as dependency #76

Merged
merged 2 commits into from
Oct 21, 2019

Conversation

l3ender
Copy link
Contributor

@l3ender l3ender commented Oct 21, 2019

Resolves #73.

@l3ender l3ender marked this pull request as ready for review October 21, 2019 02:25
Copy link

@brodycj brodycj left a comment

Choose a reason for hiding this comment

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

I just manually verified that the added test covers the mutation that I reported in #73 and would cover the last surviving mutant that I discovered in PR #56, merging now. I do have a couple of low-priority comments that we may want to address someday:

I noticed we needed a workaround to pass on Node.js 6. I hope we can drop Node.js 6 support in the near future, just raised #77. I think it would be ideal if we can remove the workaround once we do drop Node.js 6.

I have a feeling (and just a feeling) that it should be possible to make the UUID search process a little more intuitive. I hope to look into this someday.

@brodycj brodycj merged commit a80e27b into apache:master Oct 21, 2019
@brodycj brodycj mentioned this pull request Oct 21, 2019
2 tasks
@l3ender l3ender deleted the add-target-dependency-coverage branch October 21, 2019 23:58
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.

Internal pbxProject.prototype.addTargetDependency call not properly tested
2 participants