Unit tests bundle for an app target does not compile

[marciniwanicki]

Describe the bug

While playing with cucumber acceptance tests I discovered that the generated unit test bundle for an app bundle does not work out of the box (while @testable import App is used).
Current integration fixtures/app_with_tests tests seem to pass as no symbol from the App target is actually used, once I added a dummy method to the AppDelegate:

    func hello() -> String {
        return "Hello"
    }

and used that method in the AppTests:

    func testHello() {
        let sut = AppDelegate()

        XCTAssertEqual("Hello", sut.hello())
    }

the acceptance test Then I should be able to test the scheme AppTests failed.

As the generated unit test bundle configuration does not have "Host Application" and "Allow testing Host Application APIs" set, the linker fails:

Undefined symbols for architecture x86_64:
  "type metadata accessor for App.AppDelegate", referenced from:
      AppTests.AppTests.testHello() -> () in AppTests.o
      @objc AppTests.AppTests.testHello() -> () in AppTests.o
  "App.AppDelegate.init() -> App.AppDelegate", referenced from:
      AppTests.AppTests.testHello() -> () in AppTests.o
      @objc AppTests.AppTests.testHello() -> () in AppTests.o
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)

Expected behavior

Tests are completing without any issues.

Additional context

I am using:

db6812a244fea60005f4670ee788367f1e402ebe (upstream/master, origin/master, master)

Proposal

Some ways to solve this:

1. We can set TEST_HOST and infer it based on the following rule:

   The test target has a target dependency of product type .app

2. We can set TEST_HOST and infer it based on a name convention,

   ie. set it only if the test target is named same as .app target with Tests suffix (to push convention over configuration to its limits 😉)

3. We can beef up the Product enum type to contain associated types:

i.e.

public enum Product: Codable {
    // ...
    case unitTests(hostApp: String?)
    static let unitTests = Product.unitTests(hostApp: nil) // This will maintain compatibility with existing manifests
}

4. We can think about moving from simple product property towards a little target type hierarchy UnitTestTarget extending Target so we have more flexibility to set up target specific properties.

Would love to hear about some other ideas how to approach it.

We can address this after we tackle the project generator library.

[ollieatkinson]

Good catch,

I would advise against relying on inference from the system and configuration (names, etc) as they are prone to breaking and it's better that we don't force people to use particular naming schemes.

We can think about moving from simple product property towards a little target type hierarchy UnitTestTarget extending Target so we have more flexibility to set up target specific properties.

I am intrigued by this, since it might open up the opportunity for future improvements. Have you got any other thoughts about target specific properties which might be added, other than this one?

[pepicrft]

I would advise against relying on inference from the system and configuration (names, etc) as they are prone to breaking and it's better that we don't force people to use particular naming schemes.

Couldn't agree more with this one. Rails has a bunch of features that rely on naming conventions, and they are horrible because when things break it's hard to spot which convention was not followed.

I am intrigued by this, since it might open up the opportunity for future improvements. Have you got any other thoughts about target specific properties which might be added, other than this one?

This is something worth considering for an iteration of the manifest format. The same idea could be applied to extensions that depend on apps. Although the idea excited me at the beginning, I found it inconsistent with how we are currently modelling dependencies between targets. So basically there'd be hierarchy to define the dependencies from extensions and test targets, but a flat structure to define the dependencies with other sort of targets. What do you think @ollieatkinson / @marciniwanicki ?

From your proposals I'm more in favour of the third one. It's not a breaking change, and we don't need to infer anything ourselves. Moreover, we can set up the dependency with the app automatically without the developer having to explicitly define the dependency in the dependencies section.

[marciniwanicki]

Sorry guys for the lack of response, I'd like to first focus on the #205 PR (should be ready next week). Promise to help with this ticket once we have the PR sorted.

[pepicrft]

No worries @marciniwanicki. Once thing at a time 😛

[kubajakowski]

Should it work for app extensions tests? Because I’m unable to compile them with errors similar to OP