-
Notifications
You must be signed in to change notification settings - Fork 263
Add Unit Testing Infrastructure #61
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
I'm excited to explore the idea of a unit test suite for corelibs-xctest. Thanks for working on this! I see how That being said, I'm aware I may be close-minded on this subject--I've typically only ever tested One follow-up question for you: is there a |
After further thought, I'll answer my own question: I wouldn't mind a test for I'm still not convinced of testing |
Neither am I! And I'm actually glad that you are trying to talk me out of this madness 😆 But I'm going to stand my ground for now at least, and let's see where this discussion goes! Because this project is aiming to hold tightly to an existing API and be very conservative about exposing additional public API, I have the impression that it shares some characteristics with legacy codebases in terms of testability. Writing that makes me a bit sad.. I wonder how we can reconcile these goals with the desire of having a public API more amenable to unit testing? I have no particular attachment to the particular unit tests that I included in this PR to bootstrap the suite, and your point about the test increasing the codebase's rigidity is valid. I must admit to being unsure how to approach the The concrete bit of work that motivated me to produce this PR was adding the ability to use a command line argument to specify a particular test case to execute (See here and here.) I felt that the Functional test suite on its own wasn't going to let me apply TDD for the new feature. I'm not anticipating exposing any new public API for this work, though, which pushed me down the path you see in this PR. With regards to other bits of work, it seems to me that unit tests for e.g. an That was a bit scatterbrained, but hopefully reveals more of my thoughts about this. Feedback please! 😄 |
Ah, thanks for the links to the command-line interface work--I hadn't been aware of that! I can see what you mean about the functional test suite: I think it'll be great at verifying the intended behavior, but not conducive to the unit-based testing that TDD would require. I'm not quite convinced by your thoughts on Another approach to unit testing would be to split this up into separate modules, then only test the public API of those modules. The logic to parse command-line arguments, for example, seems like it could be extracted into its own framework. It sounds like we'll need some more opinions here. @mike-ferris-apple? @ddunbar? |
I like the concept of breaking this up into modules, but I don't have a clear picture of what that would look like in terms of how the project is built. (Incidentally, that reminds me of this - I hope you can feel my internal conflict about testing As another concrete case which to contribute to this discussion, here are the tests I produced while TDDing my way towards #64. I only included the functional test in that PR as I didn't want to conflate things too much there. |
A few scattered comments... First, a purely technical thing. @briancroom , you said in the original description that you're compiling the sources for XCTest into your test bundle to get access to internal stuff. Did you consider compiling XCTest for testability and using @testable import instead? That would seem the more normal way to do this. On the Linux end, we could consider having debug builds of XCTest be compiled for testability always to facilitate this. (Xcode does this by default for the Debug build configuration as well.) On to the more philosophical stuff. In a significant code base I don't think it's avoidable to have unit tests need to have access to and use internal API. (That's why the testability feature was added last year to Swift...) Generally speaking when designing a public API, I don't think sufficient testability of the breadth of the implementation should be a forcing factor to making API public. There will be aspects of the internals of the framework that it is important to ensure are functioning properly so that higher level testing can assume those pieces are working, but that doesn't mean those aspects should themselves be public. So, in general I don't have a problem with unit tests testing internals. That said, it would be nice, as @modocache points out, not to have to rely on aspects of the internals that we know we'd like to remove (like our global state) as much as possible. For the test that is being implemented here, I would much rather see that use XCTestObservation (of course that requires that we have that bit of API, but with @briancroom 's recent work on XCTestCase I believe we're ready for a trivial implementation of the observation stuff to be pretty, well, trivial.) But I'd also love to see any implementation of XCTestObservationCenter come along with unit tests that test the basic mechanism of test observation in isolation (just using the observation center, an observer and calls into the internal methods that cause the observers to be notified, no test cases involved), which will require use of internal API in that area. Of course, we'll also want testing that the test cases themselves are integrated with the observation center appropriately. These tests can be more straight-forward and probably won't require non-public API. With all that testing, we can then have confidence that further unit tests that are predicated on the observation system can be relied upon to work as intended and, in turn, test what they're meant to test. Getting that in place, with the infrastructure that @briancroom has implemented here, will put us in a good place to test all sorts of behavior having to do with what tests run, in what order, and what kinds of failures they report all through public API... |
Yeah I went down that route first, which worked great on OS X in Xcode, but ran into some challenges with the build script on Linux which made me decide to try this approach instead. In particular, my attempts to update the build script to integrate the unit tests this way would have resulted in some troublesome UX for people using the script. Enabling testability for the library for debug mode makes sense, but then what happens when the script is used in release mode (the default)? Do we just skip the unit tests? That seems potentially misleading, because the fact that the script exited with I considered having the script always build a version of the library with testablity enabled and putting it somewhere else for the unit tests to link against, but that is still misleading in the case of the That said, I agree that importing the library with |
These are issues that we face generally. As, more and more, we have to build differently in order to be able to do (some kinds of) testing, it makes things more complicated. What it comes down to is that tests that need access to non-public API cannot be run against production-built stuff. I guess this is one more reason to prefer tests using only public API, but I still think that's impractical for the finest grain testing that one would want/need in many cases. It's certainly not a huge deal to compile the framework code directly into the tests until/unless we come up with good solutions to the issues you raise. That itself has similar drawbacks. If the tests and the release framework are built at the same time, you know they came from the same code, but they were still built differently (the presence of the test code that may be using the internals in different ways than the rest of the code base may cause different amounts optimization of the internal APIs. In an ideal world, that wouldn't matter, and even in the real world it probably matters only rarely, but it's still a difference. |
Just wanted to drop a note that I haven't forgotten about this, and hope to get a chance to revisit this soon, especially now that our public API has a larger surface area than when I was first working on it! |
Yes, I think this deserves another discussion now that we're mirroring the XCTest API more closely. It may help us verify subtle behavior that would be very unwieldy to put in a functional test. For example, "if I call |
My $.04 here:
|
The Swift project moved the default branch to More detail about the branch update - https://forums.swift.org/t/updating-branch-names/40412 |
Motivation
Corelibs XCTest already has an mature setup for executing end-to-end functional tests against itself which is doing an outstanding job of providing confidence that the library will work as expected when linked into clients' test suites. Sometimes, though, it feels more appropriate to write more granular unit tests against particular APIs provided by the library. This PR aims to satisfy that missing component by adding a suite of unit tests which can be run from Xcode on OS X, or from the build script on other platforms.
Details
The unit test suite is located in the
Tests/Unit/
directory, as a sibling to the existing functional test suite. It uses Corelibs XCTest to test itself, building an app whosemain.swift
invokesXCTMain
with the list ofXCTestCase
subclasses which contain the tests themselves. I implemented a simple pair of tests aroundXCTAssert
behavior to act as a seed and example test for the suite.Ordinarily when writing unit tests for a library, I would prefer to link the library into the test target and restrict the tests to working against the public API so as to prevent them on relying too much on implementation details. For this suite, however, I came to the conclusion that the public API (at least in its present form) lends itself rather poorly to unit testing because it generally requires calling into
XCTMain
to set up state, and that function can't be called from a unit test without exiting the process altogether! Because of this, I decided to simply build the library source files together with the unit test sources, allowing for judicious use ofinternal
API in unit tests. (Note that the standard integration of the Corelibs XCTest dynamic library into client executables is thoroughly by the Functional test suite.)For OS X, the Xcode project now has a new
SwiftXCTestUnitTests
app target which executes the test suite when run. For Linux, the build script'stest
action has been extended to also build and run the unit tests, although, notably, it does not link the unit tests against an already-built XCTest dynamic library the way that the functional tests do.CI
To get CI to run the new test suite, the overall Swift build script will need to be updated (around here) to execute the unit test suite on OS X., On Linux no additional work is required because it is already taken care of by XCTest's own build script.