Allow schemes to add external targets specially unit tests

[shahzadmajeed]

Hello,

Based on the thread here https://tuistapp.slack.com/archives/C018QG7U7SN/p1664558358448169 it seems like following would be a good addition to Tuist

Proposal:

1. Allow adding external dependency targets to schemes by name. Currently you can add a TargetReference representing external target Target by specifying project folder path.
2. Allow adding test targets from external dependencies in schemes so that we can create "AllTests" scheme. This is very important aspect as it allows us to unit test all relevant parts of the code base together. Currently local swift packages or non-tuist projects in same repository cannot be unit tested together with tuist projects.

Implementation:

1. Something like .external(name: String) on TargetReference will be useful. This is non-breaking change and a nice addition which shouldn't have any implication on current use cases. Users can manually opt-in if they want to use this feature
2. Adding unit tests will require us to stop filtering testing targets from external dependencies. This will require us to add some opt-in parameter in Dependencies.swift which can be used to enable unit testing of external dependencies. As @danyf90 suggested below, we can use testableTargetsFromPackages: [package_name] array or testableTargets: [package_name: [target_name]] for users to opt-in. Another option is a filtertestableTargetsOptInFilter: ((packageName: String, targetName: String) -> String)? = nil) which defaults to nil which means user is not opting for testing dependencies.

Opening up this thread for discussion and suggestion before I start implementation. Thank you!

-Shahzad

[danieleformichelli]

Regarding 2, I think that you rarely want test to import test targets of your dependencies, unless they are internal dependencies distributed as SPM.
Hence, to avoid impacting fetch and generation time computing useless targets, I would make it opt in, with some kind of configuration in Dependencies.swift to specify for which packages you want to include also the test targets, defaulting it to empty

[shahzadmajeed]

Yes, you correctly identified the use case. It is mostly useful for testing internal dependencies which are actually part of the same code base. So, as a developer you would like to test those tests targets in generated projects.

I think package level opt-in is enough and there is probably no need to specify individual targets because that puts maintenance burden on developer i.e, add every new test target to the list. What do you think?

[kwridan]

Thanks for proposing this @shahzadmajeed

Does Tuist need to run the tests for external dependencies? if it's for CI purposes, perhaps have the CI separately run swift test on the local packages as needed.

I fear this may unnecessarily complicate the dependency feature of Tuist which was intended for external dependencies which as @danyf90 points wouldn't be typically tested as part of the project.

[shahzadmajeed]

Hi @kwridan

Thanks for the input. Basically, this feature request is only for internal/local swift packages in same repository. This is why the implementation will be Opt-In per package.

I think it does make sense to not test external dependencies but local swift packages aren't really external dependencies. Currently, there is no way to test your changes in common modules available in local swift packages while you are building and testing your Tuist targets. For us, all the modules (whether a local swift package or a Tuist target) in this repository are part of the same codebase/project and it is valuable to include all of your related targets into a single scheme to build and test together. Otherwise it complicates the process and adds manual checks which are subject to developer error.

After looking into the Dependencies.swift implementation (PackageInfoMapper more specifically), it seems like this addition mayn't be too bad in terms of complexity. This code path will basically be only triggered if you have opted-into a swift package for including test targets otherwise it will continue working as normal.