search

muter-mutation-testing / muter

🔎 Automated mutation testing for Swift 🕳️
MIT License
496 stars 39 forks source link

Let XCTestRun handle XCode TestPlan configuration #268

Closed gsl-anthonymerle closed 8 months ago

gsl-anthonymerle commented 8 months ago

What it adds

Make XCTestRun.updateEnvironmentVariables aware of the new XCode TestPlan's configuration structure

Why ?

Before TestPlan, .xctestrun configuration was limited to a simple structure, but TestPlan has a new structure that XCTestRun has to handle to be able to enable the various Mutation Schematas. To see the difference, check Tests/fixtures/BuildForTesting/projectWithTestPlan.xctestrun file in this Pull Request.

More informations

I'm no expert on the .xctestrun subject, and neither am I regarding being good at unit testing. So please, don't hesitate to let me know if I did anything wrong in this PR, although I did my best to match your coding style and testing skills 🙏

Side note: I think I see that the XCTestRun may want to be split between two separate implementation with the same interface (i.e. protocol), with one being specialised for what I call the "legacy test configuration" and the other implementation specialised for the new TestPlan configuration. But that would have made me change the way the XCTestRun is created and I didn't want the change to leak to every place where a XCTestRun is being created. I don't feel confident enough to make such a big change in a project I don't own 🙏 But I would still be glad to know what you think of the idea, even if you don't make it happen

rakaramos commented 8 months ago

hi @gsl-anthonymerle, this is great, thanks for adding it. 100% agree with what you think about XCTestRun It is far from perfect, but it works as is :P This can be cleaned up later!

gsl-anthonymerle commented 8 months ago

Thanks @rakaramos for your answer and having merged my PR :)

XCTestRun It is far from perfect, but it works as is :P

Oh! I didn't intend to criticise how XCTestRun was designed in the first place, and I'm sorry if I suggested that were the case 🙏 I really wanted to criticise the "big if" I added to handle the TestPlan case, and as you said this can be cleaned up later :)