Fix code coverage not gathered for project with enabled code coverage when running tuist test

[adellibovi]

Resolves #2182
Request for comments document (if applies):

Short description 📝

Hi, as mentioned in #2182, currently running tuist test when having code coverage enabled for the project do not correctly gather code coverage.
To try it out:

• Enable code coverage to fixture: ios_workspace_with_microfeature_architecture
• tuist generate
• tuist test
• Open xcresult in DerivedData\ProjectHash\Logs\Test

Checklist ✅

• The code architecture and patterns are consistent with the rest of the codebase.
• The changes have been tested following the documented guidelines.
• The CHANGELOG.md has been updated to reflect the changes. In case of a breaking change, it's been flagged as such.
• In case the PR introduces changes that affect users, the documentation has been updated.

[fortmarek]

Hi @adellibovi!

could you specify what tuist version you are using?

For the current release tuist test does not actually work at all 😞 - the fixes are here.

Under the hood, tuist test now runs Project scheme - for which the code coverage you have actually fixed already (thanks!)

So, from what I understand this change is not necessary.

You have also stated that this PR will fix #2186 - is that the case, though? This does not solve the fact that the test results are saved into a derived data path. We should indeed provide a way to specify where the results will be saved which is related to this.

[adellibovi]

@fortmarek oops, wrong mention of issue, I meant #2182, updated in the description too 😄

Regarding the version, I believe I tried with a branch up-to-date from main, and was still getting the tests run targets one by one 🤔
Will try again just to be sure. About the change it may still make sense for users who run wants to run tuist test FrameworkTests or in general for other targets which now limits the code coverage to just their self and not the affected targets (the description of the issue would need to be changed though).

Update: I was using the branch only to generate the project, then, since this PR does not modify the test command, I was running tuist test installed locally, which was using an older version.

[fortmarek]

ah, ok, I see

Following up on the discussion from the issue:

Shouldn't the default behaviour be that if we enable code coverage we gather coverage for all targets?

I don't have a strong opinion on this but if I run XXXTests, I'd expect to get coverage only for XXX - as the situation currently is.

A more delicate fix of the original issue would be something like this:

let codeCoverageTargets = codeCoverage ? testableTargets + [targetReference] : []

Because the issue is that we add code coverage only for the target itself but we should actually collect code coverage for the targets we want to test.

[adellibovi]

Adding a comment for other reviewers, while having a chat offline with @fortmarek we also noticed that is worth mentioning that the default behavior of Xcode when enabling Code Coverage is to enable for all targets (as this PR)

[fortmarek]

@kwridan do you have any thoughts on this? I think we can follow the Xcode's behavior and keep the PR as-is but it would be great to get the third opinion in here

[kwridan]

Both approaches have merit. Perhaps keeping the Xcode behaviour makes sense.

I can imagine when running UI tests or integration tests you may want to see the coverage for all the dependent modules too.

I guess if that isn't desired running the tests on the individual modules to see their coverage is still available.

For example running AppUITests vs NetworkingTests

AppUITests may include coverage or App, Networking, ...

Vs NetworkingTests will include coverage of Networking (and its dependencies if has any)

As a final measure, worth remembering users can create custom schemes and tailor it to their specific needs, so here it's a matter of picking a default behaviour we think applies best to most cases.

Hope this helps 🙂

[adellibovi]

@kwridan and @fortmarek not sure what we decided 😆
I understand we are more towards going with Xcode approach and therefor keeping the PR as is?

In any case, I rebased this PR and changed the changelog, as the fix for tuist test is already fixed, this PR now mention that, when code coverage is enabled, targets such as TestMyFramework gather coverage for all affected targets instead of TestMyFramework only.

If I misunderstood, please clarify it for me so I can make the appropriate changes 😃 🙏

[fortmarek]

I'm ok with keeping Xcode default behavior, approved ✅