[Review] SE-0019 Swift Testing (Package Manager)

Evolution Discussion
1

Hello Swift community,

The review of “Swift Testing” for the Package Manager begins now and runs through Thursday, January 7th. The proposal is available here:

  swift-evolution/0019-package-manager-testing.md at master · apple/swift-evolution · GitHub

For this particular review, please note that a significant amount of discussion history is available in the original pull request for the proposal:

  Proposal for testing support in SwiftPM by mxcl · Pull Request #51 · apple/swift-evolution · GitHub

Reviews are an important part of the Swift evolution process. All reviews should be sent to the swift-evolution mailing list at

  https://lists.swift.org/mailman/listinfo/swift-evolution

or, if you would like to keep your feedback private, directly to the review manager.

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift. When writing your review, here are some questions you might want to answer in your review:

  * What is your evaluation of the proposal?
  * Is the problem being addressed significant enough to warrant a change to Swift?
  * Does this proposal fit well with the feel and direction of Swift?
  * If you have you used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
  * How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

More information about the Swift evolution process is available at

  swift-evolution/process.md at master · apple/swift-evolution · GitHub

Thank you,

  - Rick
    Review Manager

2

A few requests for clarification from the proposal authors:

1. The proposal talks in several places about “test modules” (for example, “test-module is created per subdirectory of Tests”). How do these test modules interact with Package.swift? Does each of them have a separate target? If so, how is the test module name specified in the Target(…) entry?

2. Perhaps answered by #1, how does Package.swift specify test-only dependencies (e.g. Quick) that are necessary to build tests, but should not be exported to downstream projects?

3. You propose that “building a module also builds that module's corresponding tests.” Does this apply only to the top-level package, or to all of its dependencies? For example, if my FooApp depends on BarLib, and BarLib’s tests depend on HugeFancyTestFramework, do I have to download and compile HugeFancyTestFramework in order to build FooApp? (Hopefully not!)

I apologize if these questions are already answered in the proposal. I’m not sure I caught every subtlety of the writeup.

Cheers,

Paul

···

On Jan 5, 2016, at 1:06 PM, Rick Ballard via swift-evolution <swift-evolution@swift.org> wrote:

Hello Swift community,

The review of “Swift Testing” for the Package Manager begins now and runs through Thursday, January 7th. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0019-package-manager-testing.md

For this particular review, please note that a significant amount of discussion history is available in the original pull request for the proposal:

  Proposal for testing support in SwiftPM by mxcl · Pull Request #51 · apple/swift-evolution · GitHub

Reviews are an important part of the Swift evolution process. All reviews should be sent to the swift-evolution mailing list at

  https://lists.swift.org/mailman/listinfo/swift-evolution

or, if you would like to keep your feedback private, directly to the review manager.

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift. When writing your review, here are some questions you might want to answer in your review:

  * What is your evaluation of the proposal?
  * Is the problem being addressed significant enough to warrant a change to Swift?
  * Does this proposal fit well with the feel and direction of Swift?
  * If you have you used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
  * How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

More information about the Swift evolution process is available at

  https://github.com/apple/swift-evolution/blob/master/process.md

Thank you,

  - Rick
    Review Manager
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

3

Overall, I think the feature is important to have, but I don’t understand some of the aspects of the proposal. I also don’t think there is a real focus for clarity on the types of testing that are being supported here. The implication is that unit tests are what this is being targeted, but is this proposal specifically limiting to those particular types of tests? If so, why? If not, some of the aspects really don’t make much sense to be defaulted into.

Additionally we will support directories called FooTests. This layout style is prevalent in existing open source projects and supporting it will minimize vexation for their authors. However in the interest of consistency and the corresponding reduction of cognitive-load when examining new Swift packages we will not recommend this layout. For example:

    Package
    └── Sources
    │ └── Foo.swift
    └── FooTests
        └── Test.swift

Why support something that that is not going to be recommended? Prevalence of something seems like a poor choice, especially when you are going to specifically not recommend to use it. Also, the proposal already mentioned an override mechanism to allow these to be specified. This seems like something that could easily be cut.

Additionally, we propose that building a module also builds that module's corresponding tests. Although this would result in slightly increased build times, we believe that tests are important enough to justify this (one might even consider slow building tests to be a code smell). We would prefer to go even further by executing the tests each time a module is built as well, but we understand that this would impede debug cycles.

Re-building tests all of the time is a huge time waste. Executing those tests even more so (also see original question on the types of tests being supported). Not only that, in production software, it’s very often the case that there are levels of tests that get run because of the shear amount of them that exist and the time involved to run them. In addition to levels, there are classifications of tests (perf, robustness, memory, stress, fuzzing, etc…).

This is something that should be an opt-in. The most basic example of this is refactoring a code base (which is later briefly mentioned at the end o the proposal). The first step is getting the code compiling for the project. It’s not true that the very next step you take is fix up the tests, especially in the cases that the refactoring/changes are exploratory.

In the future, we may choose to promote the --test option to be a subcommand of the swift command itself:

$ swift test

However, any such decision would warrant extensive design consideration, so as to avoid polluting or crowding the command-line interface. Should there be sufficient demand and justification for it, though, it would be straightforward to add this functionality.

This doesn’t make sense to me. It’s either straightforward, or it requires extensive design consideration. I personally strongly dislike coupling the notion of building with test execution, so I would much rather see “swift test” used, especially with a look into the future where it’s going to be asked for the ability to run categories of tests and filter to a particular set of tests to be run.

Another real problem with this type of design, is that is makes more advanced build systems very complicated to make. For example, distributed builds that bring together all of the compiled bits to run tests on get blocked because the build command starts to expect certain intermediate output. This is a real problem my previous team still has to this day with xcodebuild and actively prevents us from doing this exact thing with standard tools from Apple, so we have to basically roll our own. I see this design following in the exact same footsteps.

I think a lot of the design would be clarified by changing all of “by convention” items into realized default values in the Package.swift file. That would clearly demonstrate how the work and how we can change them.

-David

···

On Jan 5, 2016, at 11:06 AM, Rick Ballard via swift-evolution <swift-evolution@swift.org> wrote:

Hello Swift community,

The review of “Swift Testing” for the Package Manager begins now and runs through Thursday, January 7th. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0019-package-manager-testing.md

For this particular review, please note that a significant amount of discussion history is available in the original pull request for the proposal:

  Proposal for testing support in SwiftPM by mxcl · Pull Request #51 · apple/swift-evolution · GitHub

Reviews are an important part of the Swift evolution process. All reviews should be sent to the swift-evolution mailing list at

  https://lists.swift.org/mailman/listinfo/swift-evolution

or, if you would like to keep your feedback private, directly to the review manager.

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift. When writing your review, here are some questions you might want to answer in your review:

  * What is your evaluation of the proposal?
  * Is the problem being addressed significant enough to warrant a change to Swift?
  * Does this proposal fit well with the feel and direction of Swift?
  * If you have you used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
  * How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

More information about the Swift evolution process is available at

  https://github.com/apple/swift-evolution/blob/master/process.md

Thank you,

  - Rick
    Review Manager
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

4

  * What is your evaluation of the proposal?

Overall this is great direction, but needs more deliberation and sifting about the details. This kind of infrastructure will live a while, or if it ends up stinky, will go unused.

  * Is the problem being addressed significant enough to warrant a change to Swift?

Not really a language change as far as I can tell.

  * Does this proposal fit well with the feel and direction of Swift?

Definitely.

  * If you have you used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

This is an excellent addition that keeps the whole life cycle view in.

  * How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

On Build Configuration/Testability
This seems to be a fairly narrow view, but maybe it's enough.
I think release and debug are not the only configurations, though likely the most common set.
Perhaps some sensible defaults should be implicit (written automatically in a PM file) but customizable?
(apologies if this exists and I missed it.)

Additionally, this takes a narrow view on the nature of testing.
Unit tests are not the only game in town and I would like to see this left open to other kinds of automated testing.
You might have UI testing among others. XCUITest is not the only idea here.
You may need to test interactions and flows between systems.

For the Command Line Interface section
I would suggest the subcommand test makes a lot of sense.

Is there a proper place here somewhere for ensuring a home for test resources?
(a standardized test/ subdirectory for static/standard test data )

Lovely open ended callouts on the test reporting section.
Important for that to be malleable and replaceable. Might be JSON would be an obvious addition there...?

Lastly, should there be any built-in facility for bug/issue tracking relationships?
Perhaps too much minutia for this proposal, but seems a good thing to include.
In particular for reporting, any ticket/tracking numbers referring to regression tests are always good callouts in manager friendly reporting.

Lastly as an addition, would it make sense to include some kind of test/code-coverage element here?

···

On Jan 6, 2016, at 4:06 AM, Rick Ballard via swift-evolution <swift-evolution@swift.org> wrote:

5

What is your evaluation of the proposal?

I recommend accepting it as a vision and first draft, though not as a releasable product (yet).

This is a difficult proposal to review because of the way it mixes the general and the specific. On the one hand, it feels like this document’s primary intent is to lay out a large-scale vision, a philosophy of how the Package Manager should treat tests — and more importantly, a vision of the habits package authors should have around writing tests. On the other hand, the proposal is full of very specific details (such as directory structure) — too many such details to evaluate it as a large-scale vision only, yet not enough to evaluate it as a completely specified solution. I’ll therefore take the vision and the details separately.

I like the proposal’s broad vision very much. It advocates three important cultural norms about testing in Swift, all somewhat contrary to other package management systems, but all ones I approve of:

The package manager not only allows but proactively encourages package authors to write and maintain tests.
Tests are a uniform feature of Swift packages, with a standard invocation mechanism and standard result reporting (as opposed to being an ad hoc build task that differs from project to project).
Tests should run reproducibly, for everyone, on checkout with no manual setup. It’s just “run tests;” there is no step 2.

While the proposal doesn’t state all of this quite so specifically, it’s there in the thinking — and I approve. Thinking about this proposal in terms of Chris Lattner’s notion of the “programmer model,” I see this proposal tending toward a world where a package’s tests are run regularly not only but its contributors, but also its clients. That has cultural implications: users will hold package owners accountable if tests are missing, don’t work, or fail to catch problems. “Tests pass for everyone everywhere” is a significantly higher bar than “tests passed for one person somewhere,” and I like a Swift culture built around that higher standard of reproducibility.

The specific details of the proposal are a mixed bag. These seem good to me:

the proposed directory structure,
the notion of multiple test modules,
automatic dependency determination between test modules and library targets,
the long-term goal of “swift test” being the command line invocation mechanism,
the ability to target individual tests from the command line,
starting with XCTest as the initial mechanism with the goal of shifting to a more abstract protocol-based approach in the future, and
providing machine-readable test results.

These details I have questions or reservations about:

lack of clarity about how to run individual test modules from the command line,
lack of clarity about which test modules are run by default with “swift test” (“all of them” is probably the wrong answer),
lack of clarity about how test modules interact with Package.swift, and related to that, how a project should specify build configuration and dependencies specific to a particular test module,
the “swift build --test” syntax (although I realize that this initial approach is due to larger constraints), and
a seeming assumption that extra compilation is essentially free.

This last one deserves special comment. The notion that the build command will also build tests but will not run them seems peculiar, and poorly justified in the document. Why build if you’re not going to use the build output? Developers spend hours shaving fractions of seconds off of their build cycles, and for good reason: development is highly iterative. And not all those iterations involve running tests!

The proposal’s authors’ answer to this seems to run something along the lines of “incremental builds are fast, and slow test builds are a code smell anyway.” That’s a weak justification. Incremental builds are not fast when iterating on changes to a core type that forces recompilation of large swaths of the tests, and even in ideal cases, Swift’s incremental builds are not always as fast in practice as they ought to be in theory. If users’ tests are building slowly, then either (1) that smell is there for a good reason, and you’re punishing users already in pain, or (2) the additional disincentive of slow builds is unlikely to alter behavior for which there are already the much larger disincentives of high maintenance cost and difficulty bringing in new contributors.

The “--without-tests” option does little to allay this concern. Because it is the right default, it will become another hoop to jump through — and Swift developers will forever be swapping shell aliases that make it the default. (Check the number of upvotes on this question: ruby - How to make --no-ri --no-rdoc the default for gem install? - Stack Overflow)

Carthage made similar mistakes about unnecessary builds being approximately free, and has paid for it in a larger stream of user complaints (some written by this author, as you can probably tell).

These concerns aside, this proposal is the right direction. My recommendation is thus to accept it as a vision, direction for development, and first draft, with the expectation that subsequent proposals will address the concerns above.

Is the problem being addressed significant enough to warrant a change to Swift … er, the Swift PM?

Yes. As noted above, making testing a first-class citizen of the Swift ecosystem has large cultural benefits. The potential to be able to build dependency tests against a particular set of resolved version is highly appealing. Setting testing standards at the package manager’s outset will help prevent test approach fragmentation, and thus help realize these benefits.

Does this proposal fit well with the feel and direction of the Swift PM?

It does fit with the Swift PM’s general direction of being prescriptive about top-level project structure and standardized build process, while leaving intra-module structure fairly open.

If you have you used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

I’ve used bundler extensively, which does not address testing at all. Though Ruby has a strong testing culture, individual gem tests are of highly varying quality, and are sometimes difficult to run. Environment-related gem bugs — new Ruby version, incompatible dependency version, etc. — are sometimes a problem, and would presumably be easier to catch if it were easy to run gem tests en masse.

How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

I read the proposal carefully and asked some questions, but have not followed the whole discussion thread.

Cheers,

Paul

···

On Jan 5, 2016, at 1:06 PM, Rick Ballard via swift-evolution <swift-evolution@swift.org> wrote:

Hello Swift community,

The review of “Swift Testing” for the Package Manager begins now and runs through Thursday, January 7th. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0019-package-manager-testing.md

For this particular review, please note that a significant amount of discussion history is available in the original pull request for the proposal:

  Proposal for testing support in SwiftPM by mxcl · Pull Request #51 · apple/swift-evolution · GitHub

Reviews are an important part of the Swift evolution process. All reviews should be sent to the swift-evolution mailing list at

  https://lists.swift.org/mailman/listinfo/swift-evolution

or, if you would like to keep your feedback private, directly to the review manager.

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift. When writing your review, here are some questions you might want to answer in your review:

  * What is your evaluation of the proposal?
  * Is the problem being addressed significant enough to warrant a change to Swift?
  * Does this proposal fit well with the feel and direction of Swift?
  * If you have you used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
  * How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

More information about the Swift evolution process is available at

  https://github.com/apple/swift-evolution/blob/master/process.md

Thank you,

  - Rick
    Review Manager
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

6

This review has completed and the proposal is now "Under Revision". A second review will be scheduled once revisions are complete. Thank you all for your feedback.

  - Rick
    Review Manager

···

On Jan 5, 2016, at 11:06 AM, Rick Ballard via swift-evolution <swift-evolution@swift.org> wrote:

Hello Swift community,

The review of “Swift Testing” for the Package Manager begins now and runs through Thursday, January 7th. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0019-package-manager-testing.md

For this particular review, please note that a significant amount of discussion history is available in the original pull request for the proposal:

  Proposal for testing support in SwiftPM by mxcl · Pull Request #51 · apple/swift-evolution · GitHub

Reviews are an important part of the Swift evolution process. All reviews should be sent to the swift-evolution mailing list at

  https://lists.swift.org/mailman/listinfo/swift-evolution

or, if you would like to keep your feedback private, directly to the review manager.

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift. When writing your review, here are some questions you might want to answer in your review:

  * What is your evaluation of the proposal?
  * Is the problem being addressed significant enough to warrant a change to Swift?
  * Does this proposal fit well with the feel and direction of Swift?
  * If you have you used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
  * How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

More information about the Swift evolution process is available at

  https://github.com/apple/swift-evolution/blob/master/process.md

Thank you,

  - Rick
    Review Manager
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

7

A few requests for clarification from the proposal authors:

1. The proposal talks in several places about “test modules” (for example, “test-module is created per subdirectory of Tests”). How do these test modules interact with Package.swift? Does each of them have a separate target? If so, how is the test module name specified in the Target(…) entry?

There is no support for referring to test targets in the Package.swift as part of this proposal.

2. Perhaps answered by #1, how does Package.swift specify test-only dependencies (e.g. Quick) that are necessary to build tests, but should not be exported to downstream projects?

This is already a part of the package manager: Add Private dependency to Manifest by kostiakoval · Pull Request #74 · apple/swift-package-manager · GitHub

3. You propose that “building a module also builds that module's corresponding tests.” Does this apply only to the top-level package, or to all of its dependencies? For example, if my FooApp depends on BarLib, and BarLib’s tests depend on HugeFancyTestFramework, do I have to download and compile HugeFancyTestFramework in order to build FooApp? (Hopefully not!)

HugeFancyTestFramework will not be downloaded or built.

8

On Build Configuration/Testability
This seems to be a fairly narrow view, but maybe it's enough.
I think release and debug are not the only configurations, though likely the most common set.

As yet SwiftPM doesn’t support other configurations, that would have to be another proposal.

Perhaps some sensible defaults should be implicit (written automatically in a PM file) but customizable?
(apologies if this exists and I missed it.)

Sorry, I don’t understand this.

Additionally, this takes a narrow view on the nature of testing.
Unit tests are not the only game in town and I would like to see this left open to other kinds of automated testing.
You might have UI testing among others. XCUITest is not the only idea here.
You may need to test interactions and flows between systems.

The proposal details future intention to support other test frameworks and styles.

Is there a proper place here somewhere for ensuring a home for test resources?
(a standardized test/ subdirectory for static/standard test data )

Not as part of the proposal. Constructing paths with __FILE__ works for SwiftPM itself currently and is enough for now IMO.

Lovely open ended callouts on the test reporting section.
Important for that to be malleable and replaceable. Might be JSON would be an obvious addition there…?

We can certainly build it so it’s the output engine is a protocol underneath.

JSON makes sense, but I don’t think it needs to hold up review though, a PR to add JSON after the proposal is implemented is adequate IMO.

Lastly, should there be any built-in facility for bug/issue tracking relationships?
Perhaps too much minutia for this proposal, but seems a good thing to include.
In particular for reporting, any ticket/tracking numbers referring to regression tests are always good callouts in manager friendly reporting.

Sounds like an interesting and valuable addition. But again I think it can be a PR. The proposal is mostly to ensure a solid foundation that we can build on in future. We must ensure there is nothing missing or of the wrong direction at the start.

Lastly as an addition, would it make sense to include some kind of test/code-coverage element here?

Certainly, but I think this is outside the scope of this proposal.

Thanks!

Max

9

Thanks for the clarifications. This helps me understand much better how the proposal plays out.

1. The proposal talks in several places about “test modules” (for example, “test-module is created per subdirectory of Tests”). How do these test modules interact with Package.swift? Does each of them have a separate target? If so, how is the test module name specified in the Target(…) entry?

There is no support for referring to test targets in the Package.swift as part of this proposal.

2. Perhaps answered by #1, how does Package.swift specify test-only dependencies (e.g. Quick) that are necessary to build tests, but should not be exported to downstream projects?

This is already a part of the package manager: Add Private dependency to Manifest by kostiakoval · Pull Request #74 · apple/swift-package-manager · GitHub

Cool. Hadn’t seen the private dependencies yet.

Taking 1 and 2 together, does that mean that all dependencies necessary for _all_ the test modules must be downloaded & compiled in order to run _any_ or them? Not ideal, but not a deal-killer either.

3. You propose that “building a module also builds that module's corresponding tests.” Does this apply only to the top-level package, or to all of its dependencies? For example, if my FooApp depends on BarLib, and BarLib’s tests depend on HugeFancyTestFramework, do I have to download and compile HugeFancyTestFramework in order to build FooApp? (Hopefully not!)

HugeFancyTestFramework will not be downloaded or built.

That’s a relief!

Cheers,

Paul

···

On Jan 5, 2016, at 2:14 PM, Max Howell <max.howell@apple.com> wrote:

10

Overall, I think the feature is important to have, but I don’t understand some of the aspects of the proposal. I also don’t think there is a real focus for clarity on the types of testing that are being supported here. The implication is that unit tests are what this is being targeted, but is this proposal specifically limiting to those particular types of tests? If so, why? If not, some of the aspects really don’t make much sense to be defaulted into.

The proposal does not advocate any particular testing methodology. Certainly to start with we only support XCTest, but that is just a practical decision. Fundamentally the proposal is only advocating building modules that are *for* testing, and then executing these tests with a runner, which at first will be an XCTest runner.

Additionally we will support directories called FooTests. This layout style is prevalent in existing open source projects and supporting it will minimize vexation for their authors. However in the interest of consistency and the corresponding reduction of cognitive-load when examining new Swift packages we will not recommend this layout. For example:

    Package
    └── Sources
    │ └── Foo.swift
    └── FooTests
        └── Test.swift

Why support something that that is not going to be recommended? Prevalence of something seems like a poor choice, especially when you are going to specifically not recommend to use it. Also, the proposal already mentioned an override mechanism to allow these to be specified. This seems like something that could easily be cut.

If we don’t cater to the thousands of projects already out there we potentially will hinder adoption.

I’d agree with you if supporting this was going to hurt, but it won’t.

Additionally, we propose that building a module also builds that module's corresponding tests. Although this would result in slightly increased build times, we believe that tests are important enough to justify this (one might even consider slow building tests to be a code smell). We would prefer to go even further by executing the tests each time a module is built as well, but we understand that this would impede debug cycles.

Re-building tests all of the time is a huge time waste. Executing those tests even more so (also see original question on the types of tests being supported). Not only that, in production software, it’s very often the case that there are levels of tests that get run because of the shear amount of them that exist and the time involved to run them. In addition to levels, there are classifications of tests (perf, robustness, memory, stress, fuzzing, etc…).

I would like to wait and see. If you are right and it is a huge time waste then we can turn this off.

This is something that should be an opt-in. The most basic example of this is refactoring a code base (which is later briefly mentioned at the end o the proposal). The first step is getting the code compiling for the project. It’s not true that the very next step you take is fix up the tests, especially in the cases that the refactoring/changes are exploratory.

Indeed, I agree. But I’d like to wait and see.

In the future, we may choose to promote the --test option to be a subcommand of the swift command itself:

$ swift test

However, any such decision would warrant extensive design consideration, so as to avoid polluting or crowding the command-line interface. Should there be sufficient demand and justification for it, though, it would be straightforward to add this functionality.

This doesn’t make sense to me. It’s either straightforward, or it requires extensive design consideration. I personally strongly dislike coupling the notion of building with test execution, so I would much rather see “swift test” used, especially with a look into the future where it’s going to be asked for the ability to run categories of tests and filter to a particular set of tests to be run.

I agree, but my issue with `swift test` is that we are beginning to tightly couple `swift build` with swift itself. I’m not sure we should run-and-gun into this decision. SwiftPM is strictly alpha/beta and any decisions we make on these sorts of issues can change.

Another real problem with this type of design, is that is makes more advanced build systems very complicated to make. For example, distributed builds that bring together all of the compiled bits to run tests on get blocked because the build command starts to expect certain intermediate output. This is a real problem my previous team still has to this day with xcodebuild and actively prevents us from doing this exact thing with standard tools from Apple, so we have to basically roll our own. I see this design following in the exact same footsteps.

I’m not sure I understand.

I think a lot of the design would be clarified by changing all of “by convention” items into realized default values in the Package.swift file. That would clearly demonstrate how the work and how we can change them.

Can you explain what you mean?

11

Thank you, Paul. Your detailed feedback is both appreciated and just the sort of feedback we need to make this process useful.

The concerns that you raise here are the sorts of things that I think we'll be able to address outside this proposal process. The purpose of this process is to validate our high-level direction, not to vet every last technical detail, so I think we can do things like change the default behavior of `swift build` with respect to test code without going through this process. As such, I'd encourage you to follow up on the swift-build-dev mailing list about your specific reservations and help us get the right specifics in place here as we implement this.

Cheers,

  - Rick

···

On Jan 7, 2016, at 9:26 PM, Paul Cantrell <cantrell@pobox.com> wrote:

What is your evaluation of the proposal?

I recommend accepting it as a vision and first draft, though not as a releasable product (yet).

This is a difficult proposal to review because of the way it mixes the general and the specific. On the one hand, it feels like this document’s primary intent is to lay out a large-scale vision, a philosophy of how the Package Manager should treat tests — and more importantly, a vision of the habits package authors should have around writing tests. On the other hand, the proposal is full of very specific details (such as directory structure) — too many such details to evaluate it as a large-scale vision only, yet not enough to evaluate it as a completely specified solution. I’ll therefore take the vision and the details separately.

I like the proposal’s broad vision very much. It advocates three important cultural norms about testing in Swift, all somewhat contrary to other package management systems, but all ones I approve of:

The package manager not only allows but proactively encourages package authors to write and maintain tests.
Tests are a uniform feature of Swift packages, with a standard invocation mechanism and standard result reporting (as opposed to being an ad hoc build task that differs from project to project).
Tests should run reproducibly, for everyone, on checkout with no manual setup. It’s just “run tests;” there is no step 2.

While the proposal doesn’t state all of this quite so specifically, it’s there in the thinking — and I approve. Thinking about this proposal in terms of Chris Lattner’s notion of the “programmer model,” I see this proposal tending toward a world where a package’s tests are run regularly not only but its contributors, but also its clients. That has cultural implications: users will hold package owners accountable if tests are missing, don’t work, or fail to catch problems. “Tests pass for everyone everywhere” is a significantly higher bar than “tests passed for one person somewhere,” and I like a Swift culture built around that higher standard of reproducibility.

The specific details of the proposal are a mixed bag. These seem good to me:

the proposed directory structure,
the notion of multiple test modules,
automatic dependency determination between test modules and library targets,
the long-term goal of “swift test” being the command line invocation mechanism,
the ability to target individual tests from the command line,
starting with XCTest as the initial mechanism with the goal of shifting to a more abstract protocol-based approach in the future, and
providing machine-readable test results.

These details I have questions or reservations about:

lack of clarity about how to run individual test modules from the command line,
lack of clarity about which test modules are run by default with “swift test” (“all of them” is probably the wrong answer),
lack of clarity about how test modules interact with Package.swift, and related to that, how a project should specify build configuration and dependencies specific to a particular test module,
the “swift build --test” syntax (although I realize that this initial approach is due to larger constraints), and
a seeming assumption that extra compilation is essentially free.

This last one deserves special comment. The notion that the build command will also build tests but will not run them seems peculiar, and poorly justified in the document. Why build if you’re not going to use the build output? Developers spend hours shaving fractions of seconds off of their build cycles, and for good reason: development is highly iterative. And not all those iterations involve running tests!

The proposal’s authors’ answer to this seems to run something along the lines of “incremental builds are fast, and slow test builds are a code smell anyway.” That’s a weak justification. Incremental builds are not fast when iterating on changes to a core type that forces recompilation of large swaths of the tests, and even in ideal cases, Swift’s incremental builds are not always as fast in practice as they ought to be in theory. If users’ tests are building slowly, then either (1) that smell is there for a good reason, and you’re punishing users already in pain, or (2) the additional disincentive of slow builds is unlikely to alter behavior for which there are already the much larger disincentives of high maintenance cost and difficulty bringing in new contributors.

The “--without-tests” option does little to allay this concern. Because it is the right default, it will become another hoop to jump through — and Swift developers will forever be swapping shell aliases that make it the default. (Check the number of upvotes on this question: ruby - How to make --no-ri --no-rdoc the default for gem install? - Stack Overflow)

Carthage made similar mistakes about unnecessary builds being approximately free, and has paid for it in a larger stream of user complaints (some written by this author, as you can probably tell).

These concerns aside, this proposal is the right direction. My recommendation is thus to accept it as a vision, direction for development, and first draft, with the expectation that subsequent proposals will address the concerns above.

Is the problem being addressed significant enough to warrant a change to Swift … er, the Swift PM?

Yes. As noted above, making testing a first-class citizen of the Swift ecosystem has large cultural benefits. The potential to be able to build dependency tests against a particular set of resolved version is highly appealing. Setting testing standards at the package manager’s outset will help prevent test approach fragmentation, and thus help realize these benefits.

Does this proposal fit well with the feel and direction of the Swift PM?

It does fit with the Swift PM’s general direction of being prescriptive about top-level project structure and standardized build process, while leaving intra-module structure fairly open.

If you have you used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

I’ve used bundler extensively, which does not address testing at all. Though Ruby has a strong testing culture, individual gem tests are of highly varying quality, and are sometimes difficult to run. Environment-related gem bugs — new Ruby version, incompatible dependency version, etc. — are sometimes a problem, and would presumably be easier to catch if it were easy to run gem tests en masse.

How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

I read the proposal carefully and asked some questions, but have not followed the whole discussion thread.

Cheers,

Paul

On Jan 5, 2016, at 1:06 PM, Rick Ballard via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

Hello Swift community,

The review of “Swift Testing” for the Package Manager begins now and runs through Thursday, January 7th. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0019-package-manager-testing.md

For this particular review, please note that a significant amount of discussion history is available in the original pull request for the proposal:

  Proposal for testing support in SwiftPM by mxcl · Pull Request #51 · apple/swift-evolution · GitHub

Reviews are an important part of the Swift evolution process. All reviews should be sent to the swift-evolution mailing list at

  https://lists.swift.org/mailman/listinfo/swift-evolution

or, if you would like to keep your feedback private, directly to the review manager.

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift. When writing your review, here are some questions you might want to answer in your review:

  * What is your evaluation of the proposal?
  * Is the problem being addressed significant enough to warrant a change to Swift?
  * Does this proposal fit well with the feel and direction of Swift?
  * If you have you used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
  * How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

More information about the Swift evolution process is available at

  https://github.com/apple/swift-evolution/blob/master/process.md

Thank you,

  - Rick
    Review Manager
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org <mailto:swift-evolution@swift.org>
https://lists.swift.org/mailman/listinfo/swift-evolution

12

There is certainly a difference between Swift (the language) evolution proposals and SwiftPM evolution proposals.

Language features should be considered unchangeable (you might have to change them one day, but to do so is painful to the ecosystem)

SwiftPM is a tool, we can change it, especially since it is pre-1.0, we should be careful still, but our proposals are submitted here with the understanding that they are foundations, and not complete solutions. We will iterate

We can reverse decisions. For example, building tests by default is controversial, and perhaps it is unwise. We’ll find out in due course, I for one have no problem with reversing a decision provided it is proved incorrect.

Honestly I am leaning towards thinking it is idealist and not practical already, but I’d still like to try it out and see.

lack of clarity about how to run individual test modules from the command line,

I imagine it would be `swift test foo` where foo is the test module name.

lack of clarity about which test modules are run by default with “swift test” (“all of them” is probably the wrong answer),

Why is all of them the wrong answer? It seems to be the only sensible default to me. Clearly there should be a way to configure that in Package.swift, but that is for another proposal.

lack of clarity about how test modules interact with Package.swift, and related to that, how a project should specify build configuration and dependencies specific to a particular test module,

We already have a testDependencies option in Package.swift, hence the lack of clarity here. However Package.swift needs a rethink IMO. I plan to propose changes in a few weeks after seeing it in the wild for longer.

Thanks for your input Paul,

Max

13

Hello,

only as an additional data-point and without me having read the proposal very deeply

[snip]

These details I have questions or reservations about:

  • lack of clarity about how to run individual test modules from the command line,
  • lack of clarity about which test modules are run by default with “swift test” (“all of them” is probably the wrong answer),
  • lack of clarity about how test modules interact with Package.swift, and related to that, how a project should specify build configuration and dependencies specific to a particular test module,
  • the “swift build --test” syntax (although I realize that this initial approach is due to larger constraints), and
  • a seeming assumption that extra compilation is essentially free.

I have next to no experience with Swift, but for a (heavily template-based… :/) C++ code, the compilation time of the unit tests is an order of magnitude longer than the compilation time of the actual code / library itself.
I would advise only compiling the tests when they are actually used (run).

For said C++ code we use waf (https://waf.io) as build-tool and our default build (triggered by `./waf`) only builds the library itself, while `./waf test` performs the normal build and in addition compiles and runs the tests (including correct dependency checking so that only those tests that need to be recompiled or rerun due to changes in the library itself do so). This later command then has options for forcing running of all or no tests.
This seems to work quite well for us in that context.

  Daniel.

···

On 8 Jan 2016, at 06:26, Paul Cantrell via swift-evolution <swift-evolution@swift.org> wrote:

14

Thanks for the clarifications. This helps me understand much better how the proposal plays out.

1. The proposal talks in several places about “test modules” (for example, “test-module is created per subdirectory of Tests”). How do these test modules interact with Package.swift? Does each of them have a separate target? If so, how is the test module name specified in the Target(…) entry?

There is no support for referring to test targets in the Package.swift as part of this proposal.

2. Perhaps answered by #1, how does Package.swift specify test-only dependencies (e.g. Quick) that are necessary to build tests, but should not be exported to downstream projects?

This is already a part of the package manager: Add Private dependency to Manifest by kostiakoval · Pull Request #74 · apple/swift-package-manager · GitHub

Cool. Hadn’t seen the private dependencies yet.

Taking 1 and 2 together, does that mean that all dependencies necessary for _all_ the test modules must be downloaded & compiled in order to run _any_ or them? Not ideal, but not a deal-killer either

Yes, and indeed, this isn’t really acceptable, but I think any changes to how this work would involve a discussion on the swift-build-dev mailing list. Really how targets depend on each other and external dependencies in the Package.swift manifest needs some work in general.

3. You propose that “building a module also builds that module's corresponding tests.” Does this apply only to the top-level package, or to all of its dependencies? For example, if my FooApp depends on BarLib, and BarLib’s tests depend on HugeFancyTestFramework, do I have to download and compile HugeFancyTestFramework in order to build FooApp? (Hopefully not!)

HugeFancyTestFramework will not be downloaded or built.

That’s a relief!

Probably people would want an option to do this though. You should be running and checking the tests of your dependencies somewhat regularly.

But when developing your own software, this would be a waste of engineering time.

···

On Jan 5, 2016, at 12:21 PM, Paul Cantrell <cantrell@pobox.com> wrote:

On Jan 5, 2016, at 2:14 PM, Max Howell <max.howell@apple.com> wrote:

15

1. The proposal talks in several places about “test modules” (for example, “test-module is created per subdirectory of Tests”). How do these test modules interact with Package.swift? Does each of them have a separate target? If so, how is the test module name specified in the Target(…) entry?

There is no support for referring to test targets in the Package.swift as part of this proposal.

2. Perhaps answered by #1, how does Package.swift specify test-only dependencies (e.g. Quick) that are necessary to build tests, but should not be exported to downstream projects?

This is already a part of the package manager: Add Private dependency to Manifest by kostiakoval · Pull Request #74 · apple/swift-package-manager · GitHub

Cool. Hadn’t seen the private dependencies yet.

Taking 1 and 2 together, does that mean that all dependencies necessary for _all_ the test modules must be downloaded & compiled in order to run _any_ or them? Not ideal, but not a deal-killer either

Yes, and indeed, this isn’t really acceptable, but I think any changes to how this work would involve a discussion on the swift-build-dev mailing list. Really how targets depend on each other and external dependencies in the Package.swift manifest needs some work in general.

Given that you’ve already recognized the future need for more flexible test-module-specific build config, and now there’s also this concern about per-test-module dependencies, is it worth considering giving each test module its own (possibly implicit) target in Package.swift? That might kill several birds with one stone.

(Poor sweet little birdies. Always getting the short end of that idiom.)

3. You propose that “building a module also builds that module's corresponding tests.” Does this apply only to the top-level package, or to all of its dependencies? For example, if my FooApp depends on BarLib, and BarLib’s tests depend on HugeFancyTestFramework, do I have to download and compile HugeFancyTestFramework in order to build FooApp? (Hopefully not!)

HugeFancyTestFramework will not be downloaded or built.

That’s a relief!

Probably people would want an option to do this though. You should be running and checking the tests of your dependencies somewhat regularly.

But when developing your own software, this would be a waste of engineering time.

Indeed, clearly that should not be the default. In general, assuming unnecessary building to be “almost free” is a mistake. Carthage’s authors hit this: they built for all platforms by default, stubbornly resisted fine-tuning of the build process at the command line, and then were deluged with complaints as soon as popular libs like Alamofire started building for iOS, OS X, and watchOS even for apps that only needed one of the platforms.

However, having the option to run dependency tests seems very nice. Something I tentatively like about this proposal is that it nudges library authors to make their tests work immediately on checkout with no further futzing. Running dependency tests would push further in that direction.

A particular advantage of running all the dependencies’ tests would be to test them against the specific versions of all indirect dependencies used in production. In other words, if MyApp uses FooLib, FooLib depends on BarLib, and FooLib’s author tested against BarLib 1.0 but 1.1 inadvertently introduced a change that breaks FooLib, then running all of MyApp’s dependency tests might catch the breakage.

I realize I’ve wandered from the task at hand. I’ll get a proper review together after digesting. Thanks for the clarifications.

Cheers,

Paul

···

On Jan 5, 2016, at 2:32 PM, Max Howell <max.howell@apple.com> wrote:

On Jan 5, 2016, at 12:21 PM, Paul Cantrell <cantrell@pobox.com> wrote:

On Jan 5, 2016, at 2:14 PM, Max Howell <max.howell@apple.com> wrote:

16

Overall, I think the feature is important to have, but I don’t understand some of the aspects of the proposal. I also don’t think there is a real focus for clarity on the types of testing that are being supported here. The implication is that unit tests are what this is being targeted, but is this proposal specifically limiting to those particular types of tests? If so, why? If not, some of the aspects really don’t make much sense to be defaulted into.

The proposal does not advocate any particular testing methodology. Certainly to start with we only support XCTest, but that is just a practical decision. Fundamentally the proposal is only advocating building modules that are *for* testing, and then executing these tests with a runner, which at first will be an XCTest runner.

XCTest is just the runner with an additional style of how to validate the test. I’m talking about the distinction between unit, integration, performance, etc. All of these types of tests can be written with XCTest, though some harder than others. There are choices being made with the implicit assumption that tests in your test modules are unit tests. Running tests by default and only running tests by default on debug builds are examples of those implicit assumptions about the classification of test within your modules.

Additionally we will support directories called FooTests. This layout style is prevalent in existing open source projects and supporting it will minimize vexation for their authors. However in the interest of consistency and the corresponding reduction of cognitive-load when examining new Swift packages we will not recommend this layout. For example:

    Package
    └── Sources
    │ └── Foo.swift
    └── FooTests
        └── Test.swift

Why support something that that is not going to be recommended? Prevalence of something seems like a poor choice, especially when you are going to specifically not recommend to use it. Also, the proposal already mentioned an override mechanism to allow these to be specified. This seems like something that could easily be cut.

If we don’t cater to the thousands of projects already out there we potentially will hinder adoption.

I’d agree with you if supporting this was going to hurt, but it won’t.

It “hurts” by increasing cost, maintenance, and cognitive overload for understanding all of the “out-of-the-box” ways that test modules get implicitly created.

Additionally, we propose that building a module also builds that module's corresponding tests. Although this would result in slightly increased build times, we believe that tests are important enough to justify this (one might even consider slow building tests to be a code smell). We would prefer to go even further by executing the tests each time a module is built as well, but we understand that this would impede debug cycles.

Re-building tests all of the time is a huge time waste. Executing those tests even more so (also see original question on the types of tests being supported). Not only that, in production software, it’s very often the case that there are levels of tests that get run because of the shear amount of them that exist and the time involved to run them. In addition to levels, there are classifications of tests (perf, robustness, memory, stress, fuzzing, etc…).

I would like to wait and see. If you are right and it is a huge time waste then we can turn this off.

Then please provide and specify how this can be opt-ed out of clearly. Is it the --without-tests flag? Can we specific default options in the Package.swift so that this always occurs?

As a simple query, go around Apple and ask the compiler team if they want this feature for their code. Ask those developing in the corelibs. Ask any of the app teams. Please also do performance testing on this for various sized projects. It will absolutely have an impact, especially with regards to the current state of Swift compile times.

This is something that should be an opt-in. The most basic example of this is refactoring a code base (which is later briefly mentioned at the end o the proposal). The first step is getting the code compiling for the project. It’s not true that the very next step you take is fix up the tests, especially in the cases that the refactoring/changes are exploratory.

Indeed, I agree. But I’d like to wait and see.

I really would like to understand what your metrics of acceptability are here. Is twice the build cost ok? Three times?

In the future, we may choose to promote the --test option to be a subcommand of the swift command itself:

$ swift test

However, any such decision would warrant extensive design consideration, so as to avoid polluting or crowding the command-line interface. Should there be sufficient demand and justification for it, though, it would be straightforward to add this functionality.

This doesn’t make sense to me. It’s either straightforward, or it requires extensive design consideration. I personally strongly dislike coupling the notion of building with test execution, so I would much rather see “swift test” used, especially with a look into the future where it’s going to be asked for the ability to run categories of tests and filter to a particular set of tests to be run.

I agree, but my issue with `swift test` is that we are beginning to tightly couple `swift build` with swift itself. I’m not sure we should run-and-gun into this decision. SwiftPM is strictly alpha/beta and any decisions we make on these sorts of issues can change.

The alternatives are:

1. Factor out to a new binary, like spm, or
2. Continue down the path of tightly coupling build code with test execution.

I’d actually like to see the Swift Package Manager factor out to it’s own entity sooner rather than later and instead shell out and use the swift compiler. This would also help with other proposals, such as the “Getting C code compiling” one. But that’s a different proposal...

Another real problem with this type of design, is that is makes more advanced build systems very complicated to make. For example, distributed builds that bring together all of the compiled bits to run tests on get blocked because the build command starts to expect certain intermediate output. This is a real problem my previous team still has to this day with xcodebuild and actively prevents us from doing this exact thing with standard tools from Apple, so we have to basically roll our own. I see this design following in the exact same footsteps.

I’m not sure I understand.

Every time you run “swift build” it’s going to trigger a build. This is should never be required to run tests. Even if you say, “we’ll only re-build on incremental builds”, you are assuming that you are building and running tests within the same environment and that the environment is a build-capable environment. This is not a valid assumption. This is what xcodebuild test does today, it tries to smartly only trigger builds when necessary, but since we build on one set of boxes and run tests on other machines, this simply doesn’t work.

We have the opportunity to build a proper toolset that can be correctly used within many different workflows. However, I already see a lot of the coupling starting to happen today that has happened within xcodebuild already, and that really concerns me. Basically, it means that these tools aren’t going to be usable outside of the smaller-scale apps, and teams like mine are going to have to continue investing in more robust and flexible tools to enable our scenarios.

I think a lot of the design would be clarified by changing all of “by convention” items into realized default values in the Package.swift file. That would clearly demonstrate how the work and how we can change them.

Can you explain what you mean?

Show what the values for Package.swift are for this structure:

Package
├── Sources
│ └── Foo
│ └──Foo.swift
└── Tests
    └── Foo
        └── Test.swift
    └── Bar
        └── Test.swift

Instead of it being “by convention”, show what actual layout of a Package.swift file looks like to achieve the same thing. We should be able to define a Package.swift file that does exactly the same thing.

-David

···

On Jan 5, 2016, at 2:58 PM, Max Howell <max.howell@apple.com> wrote:

17

We will iterate.
…
We can reverse decisions. For example, building tests by default is controversial, and perhaps it is unwise. We’ll find out in due course, I for one have no problem with reversing a decision provided it is proved incorrect.

That’s good to know.

Really, my reaction to this whole proposal is that I won’t really know until I try it, so let’s try it!

lack of clarity about how to run individual test modules from the command line,

I imagine it would be `swift test foo` where foo is the test module name.

Disambiguating between test module names and test names would need some thought then, but probably that’s easily solved.

lack of clarity about which test modules are run by default with “swift test” (“all of them” is probably the wrong answer),

Why is all of them the wrong answer? It seems to be the only sensible default to me. Clearly there should be a way to configure that in Package.swift, but that is for another proposal.

It depends on what all those different test modules are.

One possibility is that they’re arbitrary functional groupings: Tests/Validation, Tests/Computation, Tests/Formatting. In that case, “all of them” is right. But I don’t know anybody who organizes test modules at the top level like that.

Another possibility is that they correspond to targets/submodules under Sources: Tests/Core, Tests/FancyAddOn, Tests/UIKitExtensions, Tests/WatchKitExtensions. In that case, the default would be to build & run the tests for whatever source module(s) you’re building.

A third possibility is that they correspond to different _kinds_ of testing: Tests/Regression, Tests/Integration, Tests/Performance. In this example, the integration tests might require some kind of environment setup, the performance tests might take 60 seconds to run, and so you’d only want the regression tests to run by default.

OK, so maybe “all of them” is the default and it’s overridable … but I tend to think that there should be a default of Sources/Foo running only Tests/Foo by default. Something along those lines.

Cheers, P

···

On Jan 8, 2016, at 1:57 PM, Max Howell <max.howell@apple.com> wrote:

18

Yes, and indeed, this isn’t really acceptable, but I think any changes to how this work would involve a discussion on the swift-build-dev mailing list. Really how targets depend on each other and external dependencies in the Package.swift manifest needs some work in general.

Given that you’ve already recognized the future need for more flexible test-module-specific build config, and now there’s also this concern about per-test-module dependencies, is it worth considering giving each test module its own (possibly implicit) target in Package.swift? That might kill several birds with one stone.

I’m not sure it’s worth it yet. You can’t really do anything with targets in Package.swift yet (just specify internal inter-module dependencies). I’d rather wait until the manifest format had some more functionality and had settled more, rather than slap things in without more understanding about how the manifest is going to actually be used.

3. You propose that “building a module also builds that module's corresponding tests.” Does this apply only to the top-level package, or to all of its dependencies? For example, if my FooApp depends on BarLib, and BarLib’s tests depend on HugeFancyTestFramework, do I have to download and compile HugeFancyTestFramework in order to build FooApp? (Hopefully not!)

HugeFancyTestFramework will not be downloaded or built.

That’s a relief!

Probably people would want an option to do this though. You should be running and checking the tests of your dependencies somewhat regularly.

But when developing your own software, this would be a waste of engineering time.

Indeed, clearly that should not be the default. In general, assuming unnecessary building to be “almost free” is a mistake. Carthage’s authors hit this: they built for all platforms by default, stubbornly resisted fine-tuning of the build process at the command line, and then were deluged with complaints as soon as popular libs like Alamofire started building for iOS, OS X, and watchOS even for apps that only needed one of the platforms.

However, having the option to run dependency tests seems very nice. Something I tentatively like about this proposal is that it nudges library authors to make their tests work immediately on checkout with no further futzing. Running dependency tests would push further in that direction.

A particular advantage of running all the dependencies’ tests would be to test them against the specific versions of all indirect dependencies used in production. In other words, if MyApp uses FooLib, FooLib depends on BarLib, and FooLib’s author tested against BarLib 1.0 but 1.1 inadvertently introduced a change that breaks FooLib, then running all of MyApp’s dependency tests might catch the breakage.

I agree and I wish we could force all tests to be run. But this is just idealism. Practically all of us would just start building with the “don’t do that flag”.

I think at the least when we have a “publish” command we will run all the tests. As well as lint. In day to day development we cannot expect developers to wait, but we have a duty to ensure the packaging ecosystem we help create is as high quality as possible.

Max

19

Overall, I think the feature is important to have, but I don’t understand some of the aspects of the proposal. I also don’t think there is a real focus for clarity on the types of testing that are being supported here. The implication is that unit tests are what this is being targeted, but is this proposal specifically limiting to those particular types of tests? If so, why? If not, some of the aspects really don’t make much sense to be defaulted into.

The proposal does not advocate any particular testing methodology. Certainly to start with we only support XCTest, but that is just a practical decision. Fundamentally the proposal is only advocating building modules that are *for* testing, and then executing these tests with a runner, which at first will be an XCTest runner.

XCTest is just the runner with an additional style of how to validate the test. I’m talking about the distinction between unit, integration, performance, etc. All of these types of tests can be written with XCTest, though some harder than others. There are choices being made with the implicit assumption that tests in your test modules are unit tests. Running tests by default and only running tests by default on debug builds are examples of those implicit assumptions about the classification of test within your modules.

We won’t be running tests by default, just building them by default. We have to pick a build-configuration as a default.

I’m sorry I must be missing something, I don’t see how we are only supporting unit tests. At first XCTest is the only framework, but part of the proposal explains how in the future we will support any framework that can conform to a protocol. That protocol is the topic for another proposal.

Additionally we will support directories called FooTests. This layout style is prevalent in existing open source projects and supporting it will minimize vexation for their authors. However in the interest of consistency and the corresponding reduction of cognitive-load when examining new Swift packages we will not recommend this layout. For example:

    Package
    └── Sources
    │ └── Foo.swift
    └── FooTests
        └── Test.swift

Why support something that that is not going to be recommended? Prevalence of something seems like a poor choice, especially when you are going to specifically not recommend to use it. Also, the proposal already mentioned an override mechanism to allow these to be specified. This seems like something that could easily be cut.

If we don’t cater to the thousands of projects already out there we potentially will hinder adoption.

I’d agree with you if supporting this was going to hurt, but it won’t.

It “hurts” by increasing cost, maintenance, and cognitive overload for understanding all of the “out-of-the-box” ways that test modules get implicitly created.

True, but I don’t feel that the cost here is so great that is not a reasonable compromise relative to the other goals this project has.

Additionally, we propose that building a module also builds that module's corresponding tests. Although this would result in slightly increased build times, we believe that tests are important enough to justify this (one might even consider slow building tests to be a code smell). We would prefer to go even further by executing the tests each time a module is built as well, but we understand that this would impede debug cycles.

Re-building tests all of the time is a huge time waste. Executing those tests even more so (also see original question on the types of tests being supported). Not only that, in production software, it’s very often the case that there are levels of tests that get run because of the shear amount of them that exist and the time involved to run them. In addition to levels, there are classifications of tests (perf, robustness, memory, stress, fuzzing, etc…).

I would like to wait and see. If you are right and it is a huge time waste then we can turn this off.

Then please provide and specify how this can be opt-ed out of clearly. Is it the --without-tests flag? Can we specific default options in the Package.swift so that this always occurs?

As a simple query, go around Apple and ask the compiler team if they want this feature for their code. Ask those developing in the corelibs. Ask any of the app teams. Please also do performance testing on this for various sized projects. It will absolutely have an impact, especially with regards to the current state of Swift compile times.

The proposal specifies there will be a flag, the flag is TBD. Probably exactly what you specified.

This is something that should be an opt-in. The most basic example of this is refactoring a code base (which is later briefly mentioned at the end o the proposal). The first step is getting the code compiling for the project. It’s not true that the very next step you take is fix up the tests, especially in the cases that the refactoring/changes are exploratory.

Indeed, I agree. But I’d like to wait and see.

I really would like to understand what your metrics of acceptability are here. Is twice the build cost ok? Three times?

I’d say twice is too much.

In the future, we may choose to promote the --test option to be a subcommand of the swift command itself:

$ swift test

However, any such decision would warrant extensive design consideration, so as to avoid polluting or crowding the command-line interface. Should there be sufficient demand and justification for it, though, it would be straightforward to add this functionality.

This doesn’t make sense to me. It’s either straightforward, or it requires extensive design consideration. I personally strongly dislike coupling the notion of building with test execution, so I would much rather see “swift test” used, especially with a look into the future where it’s going to be asked for the ability to run categories of tests and filter to a particular set of tests to be run.

I agree, but my issue with `swift test` is that we are beginning to tightly couple `swift build` with swift itself. I’m not sure we should run-and-gun into this decision. SwiftPM is strictly alpha/beta and any decisions we make on these sorts of issues can change.

The alternatives are:

1. Factor out to a new binary, like spm, or
2. Continue down the path of tightly coupling build code with test execution.

I’d actually like to see the Swift Package Manager factor out to it’s own entity sooner rather than later and instead shell out and use the swift compiler.

This is already how it is.

This would also help with other proposals, such as the “Getting C code compiling” one. But that’s a different proposal...

Another real problem with this type of design, is that is makes more advanced build systems very complicated to make. For example, distributed builds that bring together all of the compiled bits to run tests on get blocked because the build command starts to expect certain intermediate output. This is a real problem my previous team still has to this day with xcodebuild and actively prevents us from doing this exact thing with standard tools from Apple, so we have to basically roll our own. I see this design following in the exact same footsteps.

I’m not sure I understand.

Every time you run “swift build” it’s going to trigger a build. This is should never be required to run tests. Even if you say, “we’ll only re-build on incremental builds”, you are assuming that you are building and running tests within the same environment and that the environment is a build-capable environment. This is not a valid assumption. This is what xcodebuild test does today, it tries to smartly only trigger builds when necessary, but since we build on one set of boxes and run tests on other machines, this simply doesn’t work.

We have the opportunity to build a proper toolset that can be correctly used within many different workflows. However, I already see a lot of the coupling starting to happen today that has happened within xcodebuild already, and that really concerns me. Basically, it means that these tools aren’t going to be usable outside of the smaller-scale apps, and teams like mine are going to have to continue investing in more robust and flexible tools to enable our scenarios.

Well, it is not too late to uncouple these two pieces. Can you provide justification as to why we should decouple them? The engineering is not terrible to decouple them, but it would make things more complicated. Currently a good deal of the knowledge about where built products are and how they relate to targets does not need to be communicated outside of a single process, and simpler systems have less bugs, so I’d prefer not to complicate things without good rationale.

I think a lot of the design would be clarified by changing all of “by convention” items into realized default values in the Package.swift file. That would clearly demonstrate how the work and how we can change them.

Can you explain what you mean?

Show what the values for Package.swift are for this structure:

Package
├── Sources
│ └── Foo
│ └──Foo.swift
└── Tests
    └── Foo
        └── Test.swift
    └── Bar
        └── Test.swift

Instead of it being “by convention”, show what actual layout of a Package.swift file looks like to achieve the same thing. We should be able to define a Package.swift file that does exactly the same thing.

We agree. This is not part of this proposal however. But it is planned to make it possible to ignore the conventions if that is desired. The conventions are intended to speed development, but not hinder it, so when it may hinder it we intend to provide alternatives.

20

Yes, and indeed, this isn’t really acceptable, but I think any changes to how this work would involve a discussion on the swift-build-dev mailing list. Really how targets depend on each other and external dependencies in the Package.swift manifest needs some work in general.

Given that you’ve already recognized the future need for more flexible test-module-specific build config, and now there’s also this concern about per-test-module dependencies, is it worth considering giving each test module its own (possibly implicit) target in Package.swift? That might kill several birds with one stone.

I’m not sure it’s worth it yet. You can’t really do anything with targets in Package.swift yet (just specify internal inter-module dependencies). I’d rather wait until the manifest format had some more functionality and had settled more, rather than slap things in without more understanding about how the manifest is going to actually be used.

Fair enough. I’d at least make sure that this proposal doesn’t interfere with doing that in the future.

(My understanding of the package manager is too shallow to make that assessment myself, so I’ll just leave the thought sitting there….)

You should be running and checking the tests of your dependencies somewhat regularly.

But when developing your own software, this would be a waste of engineering time.

Indeed, clearly that should not be the default. In general, assuming unnecessary building to be “almost free” is a mistake.
…
However, having the option to run dependency tests seems very nice.

I agree and I wish we could force all tests to be run. But this is just idealism. Practically all of us would just start building with the “don’t do that flag”.

I think at the least when we have a “publish” command we will run all the tests. As well as lint. In day to day development we cannot expect developers to wait, but we have a duty to ensure the packaging ecosystem we help create is as high quality as possible.

Yes, anything that slows or complicates the dev cycle will send everyone straight to “torches and pitchforks” mode, and will be the target of hacks and workarounds galore. The normal build cycle needs to be as lean as possible.

Running dependency tests by default for any sort of “publish” command seems right.

Now lint … lint I’m not sure about. It’s problematic as soon as you try to establish a standard, as we’ve already established on this list in the “mandatory self” discussion. The truth is that I don’t care about a third party lib’s weird formatting decisions nearly as much as I care about its tests passing.

Cheers,

Paul

···

On Jan 5, 2016, at 4:48 PM, Max Howell <max.howell@apple.com> wrote: