[swift-build-dev] [swift-evolution] [Review] SE-0019 Swift Testing (Package Manager)

[Rick Ballard]

Hi all,

I'd like to thank everyone for their participation in this discussion. I originally scheduled this review for Jan 5th-7th, but left the review running as active discussion was still ongoing. At this point I am going to mark this review as "Under revision" and have Max incorporate your feedback before bringing it back for a subsequent review.

Specifically, here are the outstanding points I'm aware of and what I'll expect we'll do with each:

– There is currently no way to conditionally compile code for testing (a la "-D TESTING").

This is not going to be addressed by this initial proposal, but we should revise the proposal to mention that this needs to be solved in the future.

– The current proposal does not say that we'll support a test-without-build action.

We should revise the proposal to call out that this should be supported. This is important for workflows that involve building products on a machine other than the one which will run the tests.

– The current proposal is for a --test subcommand to 'swift build' and leaves 'swift test' for the future, without good justification.

We should revise the proposal to say that we'll implement this using 'swift test' now instead of a --test flag to 'swift build'.

– The current proposal says that we will automatically build test code when building your package code, even when you're not explicitly running tests. This has raised concerns about the performance implications of always building test code and whether this is the right default.

We still think that for most packages, it will not be overly costly to automatically build the tests as well, and that doing so makes it easier to avoid accidentally breaking your test code when you change your package's code. As such, we still plan to leave this as the default, but we are prepared to be proven wrong, and to reverse the default behavior here in the future, if community feedback indicates that we've gotten this wrong once SwiftPM is in wider use.

We do think that it would be nice to be able to turn this off on a per-package basis for all users of a package, so that packages with slow-to-compile tests can be built without needing to explicitly pass "--without-tests", but we don't want to clutter up Package.swift with that sort of configuration and don't have another great place to put it right now. We should mention this in the proposal and also explicitly address that this behavior is controversial. We should also explicitly call out that it is only the top-level package's tests that will be built automatically, not the tests of depended-upon packages.

– The current proposal does not mention test-only dependencies.

This is already implemented with via testDependencies specified in Package.swift, but this proposal should be revised to mention this mechanism.

– The current proposal does not provide a mechanism for supplying shared utility code that multiple test modules in a package wish to share. The only way to do this currently is to put that utility code in a separate package which is specified as a testDependency of this package.

This initial proposal is not going to address this need, but it should be revised to mention the problem and call out that we need to fix this in the future.

– The current proposal does not specify how test modules can be explicitly specified in the Package.swift or how those test modules can establish dependencies.

The proposal does already say that it will be possible to specify test modules in Package.swift in the future and just not in this initial implementation. The proposal should be revised to mention that this will also include the ability to specify dependencies of a test module. This should probably not just be listed as a "Backwards Compatibility" feature as that is not the only purpose of needing to explicitly specify test modules.

– The current proposal does not specify whether we will automatically run all tests or only specific tests, and whether it would be all tests in the top-level package or all tests in the entire package tree.

We think the right behavior is for 'swift test' to run all tests in the top-level package only, and for there to be a flag that runs all tests in the recursive package tree when desired; the proposal should be revised to reflect this. A concern was raised about how this works when there are different types of tests, e.g. unit tests vs performance tests; since this initial implementation of the feature doesn't have a notion of different types of tests, we think that this isn't an issue yet, and when we do support different test types we may want to revisit this behavior.

– The current proposal does not specify how to run specific tests.

The proposal should be revised to call out that it is possible to invoke specific test modules (and maybe specific tests?) by name.

– The evolution process document says that proposal review managers should be a member of the swift core team, but the review manager for this proposal is not listed as a member of the core team.

The evolution process documentation is geared towards proposals for the swift language; we're using the same process for SwiftPM, but there are a few discrepancies. For SwiftPM, the review manager for each proposal will likely be either myself, Max Howell, or Daniel Dunbar for the foreseeable future; this is essentially the "SwiftPM core team", though that's not currently formally defined. We should update the swift evolution process document to mention how SwiftPM differs here.

– The proposal says no alternatives were considered, but there are alternatives to explicitly mention.

The alternatives Drew mentioned were:

    1.  Do testing from a completely separate tool that isn't SwiftPM (probably frontended through swift-multitool)
    1a.  Or building the test frontend inside the XCTest project instead
    2.  Start with defining interface for the testing framework first, instead of potentially having one codepath for XCTest and another codepath for third party
    3.  UNIX process model instead of protocol, JSON format instead of JUnit XML, compiling without testability for debug builds by default, etc.
    4.  Doing nothing, until we have a "final draft" proposal that reviewers can endorse with fewer reservations.

We should revise the proposal to explain why we are taking this approach instead of those.

– Should we be doing this at all, or should testing support be completely decoupled from SwiftPM?

To some degree testing _is_ decoupled; SwiftPM will know how to invoke the XCTest test runner, and it will define the protocol a test framework / runner needs to implement to work with SwiftPM, but it doesn't know any details beyond that for how test execution works; that's left up to the test framework. That said, we believe that this level of knowledge of testing is appropriate. SwiftPM provides the build system, and the build system needs to know how to build testing targets if we're to support testing, so some level of knowledge is necessary. The advantage of separating out the remaining knowledge (how to invoke the test runner) is minimal, and there's significant cost to requiring a completely separate tool which would need to integrate with SwiftPM just to invoke the test runner. Knowing how to build tests and how to run them are related tasks, and we don't see a real benefit to completely isolating that knowledge.

– Should other test frameworks be supported via a basic function with a simple success or failure return code which can call out to whatever test system the package wants, instead of needing to define a real protocol for test result reporting and requiring someone to write a SwiftPM adaptor for that protocol for each test framework someone wants to use with SwiftPM?

This is something we should discuss in the separate third-party testing support proposal, not in this proposal. That said, I believe there is a strong advantage to defining a real protocol, as it allows all packages to have uniform test result reporting, regardless of what test framework the package uses. This will be important if we ever want to have a package index that can understand the health of submitted packages (or do things like let you test the effect of changes in a widely-depended upon library on clients of that library that the index knows about). It's also important for clients of a package that want to be able to hook up the tests of their dependencies to a CI system in a standard way.

– Should SwiftPM have special knowledge of XCTest baked in at all, or should test support be completely generic?

Most of the details in this proposal (such as how to define test modules, how to determine dependencies, and how to run the tests) are actually completely independent of XCTest. That said, we do believe that there is a benefit to supplying a zero-configuration-required (given convention-based layout), out-of-the-box ecosystem default testing solution, and that XCTest is the best candidate to be that solution. Having a standard makes it easy for new package authors to get off the ground and running quickly and easily. It also lets us go ahead and implement this test support now with something real before we've defined the generic protocol for other testing solutions. When we do review the proposal for third party testing frameworks, we should make sure that it requires minimal configuration to use such frameworks – that XCTest's "default" nature doesn't make other solutions cumbersome to use.

Thank you all for your feedback!

	- Rick
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.swift.org/pipermail/swift-build-dev/attachments/20160112/e56b7509/attachment.html>