Re: [swift-evolution] [Review] SE-0019 Swift Testing (Package Manager)

[Paul Cantrell via swift-evolution]

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 
>  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:
> 
>   https://github.com/apple/swift-evolution/pull/51
> 
> 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

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

Re: [swift-evolution] [Review] SE-0019 Swift Testing (Package Manager)

[John Joyce via swift-evolution]

> On Jan 6, 2016, at 4:06 AM, Rick Ballard via swift-evolution 
>  wrote:
> 
>   * 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?___
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

Re: [swift-evolution] [Review] SE-0019 Swift Testing (Package Manager)

[Max Howell via swift-evolution]

>> 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
___
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

Re: [swift-evolution] [Review] SE-0019 Swift Testing (Package Manager)

[Max Howell via swift-evolution]

> On Jan 5, 2016, at 12:21 PM, Paul Cantrell  wrote:
> 
> Thanks for the clarifications. This helps me understand much better how the 
> proposal plays out.
> 
>> On Jan 5, 2016, at 2:14 PM, Max Howell  wrote:
>> 
>>> 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: 
>> https://github.com/apple/swift-package-manager/pull/74
> 
> 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.

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

Re: [swift-evolution] [Review] SE-0019 Swift Testing (Package Manager)

[Paul Cantrell via swift-evolution]

> On Jan 5, 2016, at 2:32 PM, Max Howell  wrote:
> 
>> On Jan 5, 2016, at 12:21 PM, Paul Cantrell  wrote:
>> 
>>> On Jan 5, 2016, at 2:14 PM, Max Howell  wrote:
>>> 
 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: 
>>> https://github.com/apple/swift-package-manager/pull/74
>> 
>> 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

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

Re: [swift-evolution] [Review] SE-0019 Swift Testing (Package Manager)

[David Owens II via swift-evolution]

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 
>  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:
> 
>   https://github.com/apple/swift-evolution/pull/51
> 
> 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 

Re: [swift-evolution] [Review] SE-0019 Swift Testing (Package Manager)

[Paul Cantrell via swift-evolution]

> On Jan 5, 2016, at 4:48 PM, Max Howell  wrote:
> 
>>> 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

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