<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class="">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.</div><div class=""><br class=""></div><div class=""></div><blockquote type="cite" class=""><div class="">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 <span class="Apple-tab-span" style="white-space:pre"> </span>recommend this layout. For example:</div></blockquote><blockquote type="cite" class=""><br class=""></blockquote><div class=""></div><blockquote type="cite" class=""><div class=""><font face="Menlo" class=""> Package</font></div><div class=""><font face="Menlo" class=""> └── Sources</font></div><div class=""><font face="Menlo" class=""> │ └── Foo.swift</font></div><div class=""><font face="Menlo" class=""> └── FooTests</font></div><div class=""><font face="Menlo" class=""> └── Test.swift</font></div></blockquote><div class=""><br class=""></div><div class="">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 <i class="">not </i>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.</div><div class=""><br class=""></div><div class=""></div><blockquote type="cite" class=""><div class="">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.</div></blockquote><div class=""><br class=""></div><div class="">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…).</div><div class=""><br class=""></div><div class="">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.</div><div class=""><br class=""></div><div class=""><blockquote type="cite" class="">In the future, we may choose to promote the --test option to be a subcommand of the swift command itself:<br class=""><br class=""><div class=""><font face="Menlo" class="">$ swift test</font></div><br class="Apple-interchange-newline">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.</blockquote></div><div class=""><br class=""></div><div class="">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.</div><div class=""><br class=""></div><div class="">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 <i class="">real</i> 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.</div><div class=""><br class=""></div><div class="">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.</div><div class=""><br class=""></div><div class="">-David</div><div class=""></div><div class=""><br class=""></div><br class=""><div><blockquote type="cite" class=""><div class="">On Jan 5, 2016, at 11:06 AM, Rick Ballard via swift-evolution <<a href="mailto:swift-evolution@swift.org" class="">swift-evolution@swift.org</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div class="">Hello Swift community,<br class=""><br class="">The review of “Swift Testing” for the Package Manager begins now and runs through Thursday, January 7th. The proposal is available here:<br class=""><br class=""><span class="Apple-tab-span" style="white-space:pre"> </span><a href="https://github.com/apple/swift-evolution/blob/master/proposals/0019-package-manager-testing.md" class="">https://github.com/apple/swift-evolution/blob/master/proposals/0019-package-manager-testing.md</a><br class=""><br class="">For this particular review, please note that a significant amount of discussion history is available in the original pull request for the proposal:<br class=""><br class=""><span class="Apple-tab-span" style="white-space:pre"> </span>https://github.com/apple/swift-evolution/pull/51<br class=""><br class="">Reviews are an important part of the Swift evolution process. All reviews should be sent to the swift-evolution mailing list at<br class=""><br class=""><span class="Apple-tab-span" style="white-space:pre"> </span>https://lists.swift.org/mailman/listinfo/swift-evolution<br class=""><br class="">or, if you would like to keep your feedback private, directly to the review manager.<br class=""><br class="">What goes into a review?<br class=""><br class="">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:<br class=""><br class=""><span class="Apple-tab-span" style="white-space:pre"> </span>* What is your evaluation of the proposal?<br class=""><span class="Apple-tab-span" style="white-space:pre"> </span>* Is the problem being addressed significant enough to warrant a change to Swift?<br class=""><span class="Apple-tab-span" style="white-space:pre"> </span>* Does this proposal fit well with the feel and direction of Swift?<br class=""><span class="Apple-tab-span" style="white-space:pre"> </span>* If you have you used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?<br class=""><span class="Apple-tab-span" style="white-space:pre"> </span>* How much effort did you put into your review? A glance, a quick reading, or an in-depth study?<br class=""><br class="">More information about the Swift evolution process is available at<br class=""><br class=""><span class="Apple-tab-span" style="white-space:pre"> </span>https://github.com/apple/swift-evolution/blob/master/process.md<br class=""><br class="">Thank you,<br class=""><br class=""><span class="Apple-tab-span" style="white-space:pre"> </span>- Rick<br class=""><span class="Apple-tab-span" style="white-space:pre"> </span> Review Manager<br class="">_______________________________________________<br class="">swift-evolution mailing list<br class="">swift-evolution@swift.org<br class="">https://lists.swift.org/mailman/listinfo/swift-evolution<br class=""></div></div></blockquote></div><br class=""></body></html>