[PITCH] Enable Test Discovery by Default

This has been talked about but I said I'd put it into a proper pitch to get the ball finally rolling.

Swift 5.1 introduced Test Discovery on Linux adding an --enable-test-discovery flag you could pass to swift test so it would automatically pick up tests on Linux to run, without having to manually specifying them, which is prone to problems.

Since this has been in use for several months now without major issues, I propose we enable the flag by default. This simplifies testing on Linux, stops build errors when building on Linux without a LinuxMain.swift present and no flag, and generally makes life a bit easier, especially for newcomers.

Behaviour Changes

For macOS users there should be little to no difference. I do propose, however, that all platforms use the same discovery methods when running swift test. This reduces differences between the OSes which could cause bugs and test failures to slip through.

For other platform users, this means that you no longer need to pass --enable-test-discovery through to the build and test commands for your tests to run or compile.

There are edge cases where you may need to specify the tests you want to run due to subtle behaviour differences with test discovery. IIRC SwiftWasm has run into this. So I propose we add a new --use-test-manifest flag that will look for the equivalent of LinuxMain.swift and load tests from there. However, this won't be enabled by default and won't be included in the template provided by swift package init. (It should of course be documented though!) This allows those who need it to specify their tests.

This new manifest file should also be renamed away from LinuxMain.swift to reflect Swift's new cross-platform support, including Windows :tada: I propose naming it TestMain.swift, XCTMain.swift or XCTestManifests.swift.

22 Likes

Can you say more about what the consequences of this would be for macOS?

  • Are there downsides? XCTest discovers tests at runtime, so are there situations where this change will break existing tests?
  • Are there benefits (other than consistency between platforms, which you already mentioned)? For example, if we discover tests at compile time, does that save us startup time when running the tests, or is it a wash?

It is possible that tests that were picked up by the Obj-C runtime when running swift test will be missed if this were to be implemented. Hopefully, any instances of this would have been discovered whilst test discovery has been in use. Note that this affect instances of swift test only, it does not affect running tests inside Xcode - those would still be discovered with the Obj-C runtime

Any performance differences would need to be measured but I doubt it's going to be anything close to noticeable

Can we figure this out through automation in some way? For example, could the projects in the source compatibility suite have swift test run in both modes on macOS and then compare the logs to verify that they ran the same tests?

3 Likes

That's a great idea actually, a script to run through each project in there and compare the number of tests executed with the Obj-C runtime and test discovery should provide some confidence

2 Likes

You should be able to get the list of tests without actually executing them using the same mechanism as the existing --generate-linuxmain. Could we always perform this verification when running tests on macOS so that you get a warning if compile-time test discovery does not match run-time?

I'm a bit worried that we're creating a division where either you can have cross-platform consistency or you can have Xcode/swiftpm consistency, but not both. This makes me think we need to warn you about it (ideally from both swift test and Xcode), and/or provide a way to opt-out.

One of the problems with the existing --enable-test-discovery flag is that you need to provide it every time you run swift test. If this was something that was configured once (in Package.swift or some other config file), the burden would be much lower. I think we should learn from this that if we add ways to opt-out of compile-time test discovery.

1 Like

There are a number of known issues with test discovery: SR-12711, SR-11968, SR-12232, SR-11951

So I don't think we are in a position where we should change the behaviour of swift test on macOS.

There's already a huge difference between Xcode and SwiftPM consistency in that you can't link against UIKit or any other Apple framework in SwiftPM, whereas it's linked automagically when running inside Xcode. Ideally you'd have cross-platform and Xcode consistency but that's not something for the forums unfortunately.

This would be a bigger change than just making the flag default on Linux to get a behaviour close to macOS. Since SwiftPM doesn't support any kind of config file, it would have to live in the manifest and then you're down the rabbit hole of what to do if it's specified in the manifest and you're running on macOS or inside Xcode.

Surely a better solution would be to fix the bugs? However, I agree that changing behaviour on macOS may be opening a bigger can of worms than I initially intended and may be suited to a separate pitch.

I think the more relevant question here is consistency in testing behaviour, which as far as I know we have right now.

There is a .swiftpm/config file. It currently only contains package mirror information, so most people won't have run into it yet. But arguably the Package.swift is a better place for such a setting, since it's not really an option you can just turn on/off at will, and anyone who wants to run the tests will need to use the same setting.

As for whether to enable it on macOS, you could attach a PlatformsCondition to this setting like we do for other parts of the manifest.

Agreed, but since no proposal is required to fix bugs, I would think of this as a prerequisite for making such a pitch.

I'm wondering if we can avoid having a setting entirely. How about making the opt-in to test discovery based on the presence of LinuxMain.swift? If you have the file, you get the list of tests from it, but if you don't, you get test discovery. This way, we also avoid breaking existing test suites which may run into any of the known bugs with test discovery.

7 Likes

How about making the opt-in to test discovery based on the presence of LinuxMain.swift ?

I think this is a good solution. It's a bit less convenient for experimenting, but moving the file aside isn't a huge deal. I guess the other downside is that it's less discoverable for existing projects.

We could still keep the existing flag around for experimentation.

I'm not sure if discoverability is a big issue, if a project has a working setup, there is no need to switch to this and if they have issues, they should be able to find out about test discovery and how to enable it.

I like this idea and seems the best. Two issues I can see, first is @Aciid wanted the name changed since Windows support is coming (and other potential platforms in the future). Second is that projects like ArgumentParser contain a LinuxMain.swift, but that just contains an error telling users to run with --enableTestDiscovery. Though this is a fairly easy fix I guess

We will still have the same file, though, just with a different name, right?

That seems fine. Presumably we will keep the flag if someone has an existing main file and wants to try out discovery without jumping through hoops, such as moving the file aside. So packages that do this would simply not benefit from the new behaviour but would continue to function as they did before without changes.

+1 to the ideas that:

— Prefer configuration over CLI invocation (package always behaves the same without needing --enable-test-discovery). The idea of driving this off the presence of the to-be-renamed LinuxMain.swift seems promising.

and

— Make this a generic & consistent mechanism instead of a Linux-specific special case. Special cases are Yet Another Thing users need to learn and remember when they start using Swift packages.

Runtime test discovery seems generally preferable to build-time, so if we eventually had it on all platforms users could do away with build-time-discovered tests. (That would also make it consistent with the behavior most existing XCTest users are familiar with).

If you agree with the above, it seems like best practice would be to make packages that only support platforms which have runtime discovery use that, while packages that support other platforms as well should default to build-time discovery. But since packages are cross-platform by default, how should we init new packages with the most appropriate setup for new users? And what should we do to help users understand when they've chosen the wrong behavior for their package?

I am not sure I understand how a "wrong" behaviour could be chosen. I would think a new package would no longer get LinuxMain.swift under this proposal. This would mean it would get runtime discovery on macOS and build time discovery on other host platforms. If the package author runs into issues with the build time discovery, they can add a LinuxMain.swift.

1 Like

That seems like a very good suggestion, since it seems that the biggest risk with enabling the test discovery is to incorrectly stop running existing tests. For new packages, automatic test discovery should always be on (and we should try to fix the bugs as well).

It also makes conceptual sense: if LinuxMain.swift is there, it's taken as given — otherwise the set of tests is automatically discovered. This matches behavior of things like module maps, which are taken as given if present or automatically determined if they're not present.

2 Likes

I am not sure I understand how a "wrong" behaviour could be chosen. I would think a new package would no longer get LinuxMain.swift under this proposal.

I was thinking that the build-time discovery would update LinuxMain.swift, but I wasn't thinking clearly and that doesn't make sense. So this sounds reasonable for now:

I would think a new package would no longer get LinuxMain.swift under this proposal. This would mean it would get runtime discovery on macOS and build time discovery on other host platforms. If the package author runs into issues with the build time discovery, they can add a LinuxMain.swift .

The difference between macOS and Linux behavior is unfortunate and we should probably revisit it, but that could be done separately & after from this proposal.

I think that's the way to go. Aligning the behaviours sounds like it's a can of worms and enabling test discovery on Linux should be separate to that.

Ah yes, correct.

So to sum up, the current consensus seem to be:

  1. swift test on Linux uses test discovery by default
  2. Test discovery will not be used if SwiftPM detects a LinuxMain.swift and the tests are run with swift test on Linux
  3. Tests run on Linux with swift test --enable-test-discovery will always use test discovery, no matter if a LinuxMain.swift is present or not.
  4. Packages created with swift package init will not longer include a LinuxMain.swift
  5. LinuxMain.swift will be renamed to reflect the multi-platform support of Swift. My suggestion is XCTMain.swift

Is the next step a PR into SwiftPM or a full blown SE proposal or something in between?

8 Likes