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

Evolution Discussion
1

Hello Swift community,

A 2nd review of “Swift Testing” for the Swift Package Manager begins now and runs through Tuesday, January 19th. The proposal is available here:

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

This is the 2nd review of the proposal, after revisions based on the first review. A summary of issues raised in the first review can be found at:

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

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 the Swift Package Manager?
  * Does this proposal fit well with the feel and direction of the Swift Package Manager?
  * If you have you used other package managers 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

        * What is your evaluation of the proposal?

I think this is an excellent proposal, and I am excited to see testing
support added to swift-package-manager.

My only reservation is in regards to the following:

Executing a test from the terminal will produce user-readable output. This should incorporate colorization and other formatting similar to other testing tools to indicate the success and failure of different tests. [...] An additional option may be passed to the testing command to output JUnit-style XML or other formats that can be integrated with continuous integration (CI) and other systems.

swift-corelibs-xctest is currently responsible for its own output--it
prints test results to stdout. How do the authors of the proposal plan
to implement colorization and formatting?

Personally, I believe that the testing framework itself--in this case
swift-corelibs-xctest--should be responsible for colorization and
output formatting. I believe swift-corelibs-xctest will soon provide
an observation API similar to Apple XCTest (adding it has been
discussed several times; see:
[SR-460] Provide XCTestCase a class to allow overriding setUp() and tearDown() by briancroom · Pull Request #40 · apple/swift-corelibs-xctest · GitHub,
https://lists.swift.org/pipermail/swift-corelibs-dev/2015-December/000034.html\).
Once it does, any user will be able to register whichever output
formatter they wish.

So I would recommend that swift-package-manager use this API. Ideally,
swift-package-manager would provide default formatters, and those
could be customized via command-line arguments to `swift test`.

Another potential solution would be to borrow a page from third-party
utilities like xcpretty (https://github.com/supermarin/xcpretty\),
which parses textual output from XCTest and formats it. This is
suboptimal, however, because it (1) assumes XCTest output is not
formatted via custom observers, and (2) establishes a dependency upon
a very particular output from XCTest, which is then hard to change.

I am concerned to find this proposal for swift-package-manager mention
output formatting, since I it doesn't seem like output formatters
belong in swift-package-manager itself, and the API for
swift-corelibs-xctest hasn't been implemented yet. I suggest deferring
the discussion on output formatting to a future proposal--it seems
like something that would be merely nice to have, whereas this
proposal states "we would like to get testing up to speed as soon as
possible."

···

On Fri, Jan 15, 2016 at 2:34 PM, Rick Ballard via swift-build-dev <swift-build-dev@swift.org> wrote

On Fri, Jan 15, 2016 at 2:34 PM, Rick Ballard via swift-build-dev <swift-build-dev@swift.org> wrote

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

I have read the proposal thoroughly.

- Brian Gesiak

3

This proposal is a nice improvement over the previous iteration, and I support it.

The new command line makes much more sense: it’s worth the trouble to make “swift test” work right of the gate, and the hierarchical Module.TestGroup.individualTest structure makes sense and addresses several concerns.

It’s unclear to me how this plays out:

    swift test --kind=performance

…but it doesn’t seem to constrain what’s actually proposed here, so I’ll wait for the future proposal on that one.

I continue to think this is a bad decision:

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).

I expounded on my this thoroughly (perhaps too) in my previous review, so I won’t repeat those concerns; I’ll just note that they still stand.

This is important:

This proposal also does not cover the need for utility code, ie. a module that is built for tests to consume that is provided as part of a package and is not desired to be an external package. This is something we would like to add as part of a future proposal.

…and it makes me wonder whether multiple test files can share helper code within a single test module (crucial!), or whether every test file is compiled in isolation.

I’ll be interested in the future proposals that work out how test module options are specified in Package.swift.

I assume that all test modules are run by default with “swift test”? Again, I won’t repeat the more details concerns about that from the previous review, but just note that it should be possible for Package.swift to specify that some test modules don’t get run by default. I imagine this might be part of the proposal that establishes what “--kind=performance” means.

Looking forward to seeing this in action!

Cheers,

Paul

···

On Jan 15, 2016, at 4:34 PM, Rick Ballard via swift-evolution <swift-evolution@swift.org> wrote:

Hello Swift community,

A 2nd review of “Swift Testing” for the Swift Package Manager begins now and runs through Tuesday, January 19th. The proposal is available here:

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

This is the 2nd review of the proposal, after revisions based on the first review. A summary of issues raised in the first review can be found at:

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

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 the Swift Package Manager?
  * Does this proposal fit well with the feel and direction of the Swift Package Manager?
  * If you have you used other package managers 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

My only reservation is in regards to the following:

Executing a test from the terminal will produce user-readable output. This should incorporate colorization and other formatting similar to other testing tools to indicate the success and failure of different tests. [...] An additional option may be passed to the testing command to output JUnit-style XML or other formats that can be integrated with continuous integration (CI) and other systems.

swift-corelibs-xctest is currently responsible for its own output--it
prints test results to stdout. How do the authors of the proposal plan
to implement colorization and formatting?

This is a good point.

Personally, I believe that the testing framework itself--in this case
swift-corelibs-xctest--should be responsible for colorization and
output formatting. I believe swift-corelibs-xctest will soon provide
an observation API similar to Apple XCTest (adding it has been
discussed several times; see:
https://github.com/apple/swift-corelibs-xctest/pull/40,
https://lists.swift.org/pipermail/swift-corelibs-dev/2015-December/000034.html\).
Once it does, any user will be able to register whichever output
formatter they wish.

The proposal is not specific about who is providing the output. I personally
expected the testing-framework to provide the default Terminal output

I am concerned to find this proposal for swift-package-manager mention
output formatting, since I it doesn't seem like output formatters
belong in swift-package-manager itself, and the API for
swift-corelibs-xctest hasn't been implemented yet. I suggest deferring
the discussion on output formatting to a future proposal--it seems
like something that would be merely nice to have, whereas this
proposal states "we would like to get testing up to speed as soon as
possible.”

I think the proposal is OK. We’re not clear about who provides the output,
so nothing is fixed.

When we work on the alternative testing framework proposal we will go into
more detail on this matter.

So TL;DR: for now XCTest will provide the output, colorization can be added
immediately with some output parsing but is probably better left for the next
proposal.

One part of the proposal cannot be met immediately:

An additional option may be passed to the testing command to output JUnit-style XML or other formats that can be integrated with continuous integration (CI) and other systems.

It is essential that `swift test` itself can provide some kind of standardized
output so other tools can use `swift test` as part of larger CI systems.

However what is sufficient? Is it mandatory for us to output JUnit-esque XML
for all possible test frameworks? Would that restrict what innovations are
possible by testing frameworks?

Is it enough to simply report success or failure for the invocation? CI services
would then know if the tests ran to success or not and could simply provide
the output from the test framework as logs.

If we are to provide XML/JSON output as part of this initial implementation
then we must parse XCTest output or figure out some other way to integrate
with XCTest so that the output format can be modified.

Really the proposal should be modified and this part removed pending the
next proposal, but it would be a shame to delay implementation by a further
week.

Max

5

Hi Paul,

This is important:

This proposal also does not cover the need for utility code, ie. a module that is built for tests to consume that is provided as part of a package and is not desired to be an external package. This is something we would like to add as part of a future proposal.

…and it makes me wonder whether multiple test files can share helper code within a single test module (crucial!), or whether every test file is compiled in isolation.

Test modules are like any other swift module and are not restricted to compiling a single file. So sharing helper code within a test module should be fine; the part we don't support right now is sharing one module of utility code among multiple separate test modules, unless you make that utility module a separate package.

I’ll be interested in the future proposals that work out how test module options are specified in Package.swift.

I assume that all test modules are run by default with “swift test”? Again, I won’t repeat the more details concerns about that from the previous review, but just note that it should be possible for Package.swift to specify that some test modules don’t get run by default. I imagine this might be part of the proposal that establishes what “--kind=performance” means.

Our plan is that 'swift test' by default will run all the test modules in the top-level package, but will not by default run all test modules in depended-upon packages (though that will be possible). Adding the ability to remove test modules from the default set that we run would be a good future enhancement.

Thanks,

  - Rick

···

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

6

However what is sufficient? Is it mandatory for us to output JUnit-esque XML
for all possible test frameworks? Would that restrict what innovations are
possible by testing frameworks?

"for all possible test frameworks"? I don't think you can do something like
that unless the build-tool provides an entire output shell that testing
frameworks can hook into for reporting at runtime... and in that case, yes,
I personally think it would be overly restrictive on testing frameworks, at
least from the perspective of a *build tool*. The build
tool's responsibility is that it has to compile and run the tests, and then
allow said tests to be configured to provide feedback in whatever way I can
configure them to. The opinionated output proposed in this document would
seem to overreach those responsibilities unless, as Max and Brian
mentioned, it lived in XCTest or similar.

Is it enough to simply report success or failure for the invocation? CI
services
would then know if the tests ran to success or not and could simply provide
the output from the test framework as logs.

I'd say yes: Most CI tools I know of will just check the exit status of the
commands they're provided, in the simplest cases... So, if the test
framework provides a zero/non-zero status for pass/fail, that's enough for
most CI services I've used (Travis, Wercker, Jenkins, to name a few). Yes,
you can get a lot more detailed, but the implementations would probably
differ for each service. Exit statuses are something they all understand,
and something the build tool understands already too.

If we are to provide XML/JSON output as part of this initial implementation
then we must parse XCTest output or figure out some other way to integrate
with XCTest so that the output format can be modified.

Parsing the output feels like a fragile integration, and one that could
handcuff what XCTest can change in the future. While it'd be possible to
integrate with XCTest more directly (via a reporter registration system
that I'm sure Brian can explain better than I can), I don't think the value
of that has been defended very well from the perspective of the build tool:
The win of getting uniform test output from a build tool is not worth the
loss of simple testing integration, whether it be provided by a runtime
library defining a set of output protocols, or by intercepting other tools'
output and parsing them to transform it, or anything else I've seen
mentioned.

We would like to get testing up to speed as soon as possible.

If this is true, then we should make it clear our intention to push those
details out to XCTest and allow it to innovate and report on its own terms.
Anything else is going to slow things down. Right now, given that the
output part of the proposal is getting pushback both from developers
working on XCTest and developers who have worked on third-party testing
frameworks, I'd say that's just too much scope for a build tool to be
taking on.

Beyond that, there's a very low-effort way that's already included in the
tool to get testing up-to-speed, and this would do that: let me define a
target (or a list of targets) in my Package.swift that are for tests.
Compile them, link them with my testDependencies, and run them, checking
exit statuses for results. This achieves:
- the ability to build and run tests from the build-tool
- it works with XCTest. Today. Not in the future when they have
registration for reporters or when the build-tool writes some test wrapper
- a simple integration point for third-party frameworks
- support for almost every single CI system in existence (if it can run
Swift)
- a quick turnaround to put this in the hands of developers (who want it)

More notably, it leaves the concerns of actual testing output to the
testing frameworks, where, as so far all parties who have commented on this
review agree, those concerns actually belong. This would even fit in the
confines of this proposal and could "kick the can down the road" for
third-party testing protocols for quite some time as the build tool
evolves. Anything else is going to:
- limit the flexibility of test framework developers (including those who
work on XCTest)
- require more development (in the case of creating an XCTest
formatter/reporter, or a parser/transformer, or a protocol that XCTest
itself will then have to implement)
- be more complex

All of those things are net losses, all for something that we can't even
agree is *even philosophically a responsibility of the build tool*? Let's
just drop the output idea, even if just for now, and start delivering
first-class testability with the tools that already exist, please.

···

On Tue, Jan 19, 2016 at 1:50 PM, Max Howell via swift-evolution < swift-evolution@swift.org> wrote:

My only reservation is in regards to the following:

Executing a test from the terminal will produce user-readable output. This
should incorporate colorization and other formatting similar to other
testing tools to indicate the success and failure of different tests. [...]
An additional option may be passed to the testing command to output
JUnit-style XML or other formats that can be integrated with continuous
integration (CI) and other systems.

swift-corelibs-xctest is currently responsible for its own output--it
prints test results to stdout. How do the authors of the proposal plan
to implement colorization and formatting?

This is a good point.

Personally, I believe that the testing framework itself--in this case
swift-corelibs-xctest--should be responsible for colorization and
output formatting. I believe swift-corelibs-xctest will soon provide
an observation API similar to Apple XCTest (adding it has been
discussed several times; see:
[SR-460] Provide XCTestCase a class to allow overriding setUp() and tearDown() by briancroom · Pull Request #40 · apple/swift-corelibs-xctest · GitHub,

[swift-corelibs-dev] [xctest] Who tests the tests?
).
Once it does, any user will be able to register whichever output
formatter they wish.

The proposal is not specific about who is providing the output. I
personally
expected the testing-framework to provide the default Terminal output

I am concerned to find this proposal for swift-package-manager mention
output formatting, since I it doesn't seem like output formatters
belong in swift-package-manager itself, and the API for
swift-corelibs-xctest hasn't been implemented yet. I suggest deferring
the discussion on output formatting to a future proposal--it seems
like something that would be merely nice to have, whereas this
proposal states "we would like to get testing up to speed as soon as
possible.”

I think the proposal is OK. We’re not clear about who provides the output,
so nothing is fixed.

When we work on the alternative testing framework proposal we will go into
more detail on this matter.

So TL;DR: for now XCTest will provide the output, colorization can be added
immediately with some output parsing but is probably better left for the
next
proposal.

One part of the proposal cannot be met immediately:

An additional option may be passed to the testing command to output
JUnit-style XML or other formats that can be integrated with continuous
integration (CI) and other systems.

It is essential that `swift test` itself can provide some kind of
standardized
output so other tools can use `swift test` as part of larger CI systems.

However what is sufficient? Is it mandatory for us to output JUnit-esque
XML
for all possible test frameworks? Would that restrict what innovations are
possible by testing frameworks?

Is it enough to simply report success or failure for the invocation? CI
services
would then know if the tests ran to success or not and could simply provide
the output from the test framework as logs.

If we are to provide XML/JSON output as part of this initial implementation
then we must parse XCTest output or figure out some other way to integrate
with XCTest so that the output format can be modified.

Really the proposal should be modified and this part removed pending the
next proposal, but it would be a shame to delay implementation by a further
week.

Max

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

7

Test modules are like any other swift module and are not restricted to compiling a single file. So sharing helper code within a test module should be fine; the part we don't support right now is sharing one module of utility code among multiple separate test modules, unless you make that utility module a separate package.

…

Our plan is that 'swift test' by default will run all the test modules in the top-level package, but will not by default run all test modules in depended-upon packages (though that will be possible). Adding the ability to remove test modules from the default set that we run would be a good future enhancement.

Good, that all seems to me like the right 80% case (or maybe more like 95% case) for the first round.

Some projects will eventually want code sharing across test modules and control over which test modules are run by default, but I think that can all wait, and the proposal as it stands does leave room for those future features.

Cheers, P

···

On Jan 21, 2016, at 2:54 PM, Rick Ballard <rballard@apple.com> wrote:

8

Hi Brian,

As Max noted, this part of the proposal really ought to be removed and pended for the next proposal, for third party testing framework support. There is a good discussion to be had about the merits of requiring a testing adaptor that conforms to a protocol for structured test results vs just expecting other test frameworks to report an overall success/fail, but that's not the focus of this proposal and shouldn't block implementing anything here. Please do raise your concerns here when we do discuss supporting other test frameworks, though!

  - Rick

···

On Jan 19, 2016, at 1:02 PM, Brian Pratt via swift-evolution <swift-evolution@swift.org> wrote:

However what is sufficient? Is it mandatory for us to output JUnit-esque XML
for all possible test frameworks? Would that restrict what innovations are
possible by testing frameworks?

"for all possible test frameworks"? I don't think you can do something like that unless the build-tool provides an entire output shell that testing frameworks can hook into for reporting at runtime... and in that case, yes, I personally think it would be overly restrictive on testing frameworks, at least from the perspective of a build tool. The build tool's responsibility is that it has to compile and run the tests, and then allow said tests to be configured to provide feedback in whatever way I can configure them to. The opinionated output proposed in this document would seem to overreach those responsibilities unless, as Max and Brian mentioned, it lived in XCTest or similar.

Is it enough to simply report success or failure for the invocation? CI services
would then know if the tests ran to success or not and could simply provide
the output from the test framework as logs.

I'd say yes: Most CI tools I know of will just check the exit status of the commands they're provided, in the simplest cases... So, if the test framework provides a zero/non-zero status for pass/fail, that's enough for most CI services I've used (Travis, Wercker, Jenkins, to name a few). Yes, you can get a lot more detailed, but the implementations would probably differ for each service. Exit statuses are something they all understand, and something the build tool understands already too.

If we are to provide XML/JSON output as part of this initial implementation
then we must parse XCTest output or figure out some other way to integrate
with XCTest so that the output format can be modified.

Parsing the output feels like a fragile integration, and one that could handcuff what XCTest can change in the future. While it'd be possible to integrate with XCTest more directly (via a reporter registration system that I'm sure Brian can explain better than I can), I don't think the value of that has been defended very well from the perspective of the build tool: The win of getting uniform test output from a build tool is not worth the loss of simple testing integration, whether it be provided by a runtime library defining a set of output protocols, or by intercepting other tools' output and parsing them to transform it, or anything else I've seen mentioned.

We would like to get testing up to speed as soon as possible.

If this is true, then we should make it clear our intention to push those details out to XCTest and allow it to innovate and report on its own terms. Anything else is going to slow things down. Right now, given that the output part of the proposal is getting pushback both from developers working on XCTest and developers who have worked on third-party testing frameworks, I'd say that's just too much scope for a build tool to be taking on.

Beyond that, there's a very low-effort way that's already included in the tool to get testing up-to-speed, and this would do that: let me define a target (or a list of targets) in my Package.swift that are for tests. Compile them, link them with my testDependencies, and run them, checking exit statuses for results. This achieves:
- the ability to build and run tests from the build-tool
- it works with XCTest. Today. Not in the future when they have registration for reporters or when the build-tool writes some test wrapper
- a simple integration point for third-party frameworks
- support for almost every single CI system in existence (if it can run Swift)
- a quick turnaround to put this in the hands of developers (who want it)

More notably, it leaves the concerns of actual testing output to the testing frameworks, where, as so far all parties who have commented on this review agree, those concerns actually belong. This would even fit in the confines of this proposal and could "kick the can down the road" for third-party testing protocols for quite some time as the build tool evolves. Anything else is going to:
- limit the flexibility of test framework developers (including those who work on XCTest)
- require more development (in the case of creating an XCTest formatter/reporter, or a parser/transformer, or a protocol that XCTest itself will then have to implement)
- be more complex

All of those things are net losses, all for something that we can't even agree is even philosophically a responsibility of the build tool? Let's just drop the output idea, even if just for now, and start delivering first-class testability with the tools that already exist, please.

On Tue, Jan 19, 2016 at 1:50 PM, Max Howell via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

My only reservation is in regards to the following:

Executing a test from the terminal will produce user-readable output. This should incorporate colorization and other formatting similar to other testing tools to indicate the success and failure of different tests. [...] An additional option may be passed to the testing command to output JUnit-style XML or other formats that can be integrated with continuous integration (CI) and other systems.

swift-corelibs-xctest is currently responsible for its own output--it
prints test results to stdout. How do the authors of the proposal plan
to implement colorization and formatting?

This is a good point.

Personally, I believe that the testing framework itself--in this case
swift-corelibs-xctest--should be responsible for colorization and
output formatting. I believe swift-corelibs-xctest will soon provide
an observation API similar to Apple XCTest (adding it has been
discussed several times; see:
[SR-460] Provide XCTestCase a class to allow overriding setUp() and tearDown() by briancroom · Pull Request #40 · apple/swift-corelibs-xctest · GitHub,
https://lists.swift.org/pipermail/swift-corelibs-dev/2015-December/000034.html\).
Once it does, any user will be able to register whichever output
formatter they wish.

The proposal is not specific about who is providing the output. I personally
expected the testing-framework to provide the default Terminal output

I am concerned to find this proposal for swift-package-manager mention
output formatting, since I it doesn't seem like output formatters
belong in swift-package-manager itself, and the API for
swift-corelibs-xctest hasn't been implemented yet. I suggest deferring
the discussion on output formatting to a future proposal--it seems
like something that would be merely nice to have, whereas this
proposal states "we would like to get testing up to speed as soon as
possible.”

I think the proposal is OK. We’re not clear about who provides the output,
so nothing is fixed.

When we work on the alternative testing framework proposal we will go into
more detail on this matter.

So TL;DR: for now XCTest will provide the output, colorization can be added
immediately with some output parsing but is probably better left for the next
proposal.

One part of the proposal cannot be met immediately:

An additional option may be passed to the testing command to output JUnit-style XML or other formats that can be integrated with continuous integration (CI) and other systems.

It is essential that `swift test` itself can provide some kind of standardized
output so other tools can use `swift test` as part of larger CI systems.

However what is sufficient? Is it mandatory for us to output JUnit-esque XML
for all possible test frameworks? Would that restrict what innovations are
possible by testing frameworks?

Is it enough to simply report success or failure for the invocation? CI services
would then know if the tests ran to success or not and could simply provide
the output from the test framework as logs.

If we are to provide XML/JSON output as part of this initial implementation
then we must parse XCTest output or figure out some other way to integrate
with XCTest so that the output format can be modified.

Really the proposal should be modified and this part removed pending the
next proposal, but it would be a shame to delay implementation by a further
week.

Max

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

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