Skip to content

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

Merged
brody4hire merged 2 commits intoapache:masterfrom
l3ender:add-target-dependency-coverage
Oct 21, 2019
Merged

Test coverage: addTarget add to main project as dependency#76
brody4hire merged 2 commits intoapache:masterfrom
l3ender:add-target-dependency-coverage

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

@brody4hire brody4hire 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.

@brody4hire brody4hire merged commit a80e27b into apache:master Oct 21, 2019
@brody4hire brody4hire 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