Skip to content
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

[SR-6743] SwiftPM: Skipping the build of tests #4845

Closed
krzyzanowskim opened this issue Jan 12, 2018 · 12 comments
Closed

[SR-6743] SwiftPM: Skipping the build of tests #4845

krzyzanowskim opened this issue Jan 12, 2018 · 12 comments

Comments

@krzyzanowskim
Copy link
Contributor

Previous ID SR-6743
Radar None
Original Reporter @krzyzanowskim
Type New Feature
Status Resolved
Resolution Invalid
Additional Detail from JIRA
Votes 0
Component/s Package Manager
Labels New Feature
Assignee None
Priority Medium

md5: 46f992b1ff478959aad8050f968c2ee5

Issue Description:

When running the project with swift run it is desirable to skip tests at all. Tests are useful for development, but not necessarily for release. eg. if I deploy sources and want to build and run, I don't really need to build tests.

More general swift build and swift run should allow skipping building test targets.

-As of today, it is possible using build flags "*swift run -Xswiftc DNOTESTS*" and adding a condition in the Package.swift Actually this is not possible, the flag is not used to build Packages.swift

#if !NOTESTS
targets += [.testTarget(name: "ApplicationTests" , dependencies: [.target(name: "Application")])]
#endif

something like:

swift run --skip-tests

swift build --skip-tests

would build/run without tests.

@ankitspd
Copy link
Member

swift build and swift run should never build tests by default. You can pass --build-tests to swift build to build them. I think you might be running into a bug, can you attach a sample package where you're seeing this behavior?

@krzyzanowskim
Copy link
Contributor Author

Sure, look here (clone branch SR-6743) to reproduce

https://github.com/krzyzanowskim/OnlineSwiftPlayground/blob/e9696be2481ac6c1045d0032f7973af5862487e4/Package.swift#L1

try with

swift run -c release

error: could not find target(s): ApplicationTests; use the 'path' property in the Swift 4 manifest to set a custom target path

@ankitspd
Copy link
Member

Ah, this happens because we create the build plan regardless of what will be actually compiled. If you fix the error by adding the ApplicationTests target, running `swift run` will not compile it.

@krzyzanowskim
Copy link
Contributor Author

I don't understand hm... I already have the test target in place:

https://github.com/krzyzanowskim/OnlineSwiftPlayground/blob/e9696be2481ac6c1045d0032f7973af5862487e4/Package.swift#L21

What is the fix exactly?

@ankitspd
Copy link
Member

The SR-6743 branch does not have ApplicationTests target but master does, so you already fixed the issue!

I was trying to say that swift run fails on the SR-6743 branch not because swiftpm is trying to compile the ApplicationTests target but because it is not able to find it.

@krzyzanowskim
Copy link
Contributor Author

Can you please check it again for me? this is the file from the branch in question. It has the test target in Package.swift

https://github.com/krzyzanowskim/OnlineSwiftPlayground/blob/SR-6743/Package.swift

@ankitspd
Copy link
Member

Yes but that test target directory does not exist in that branch. https://github.com/krzyzanowskim/OnlineSwiftPlayground/tree/SR-6743 doesn't have a Tests/ApplicationTests directory.

@krzyzanowskim
Copy link
Contributor Author

Thanks. This is intentional though. As I don't expect the test being built in the first place, the test target directory is missing and expectancy is that testTarget is just skipped for any operation.

@krzyzanowskim
Copy link
Contributor Author

The same goes to gave reverse option, to build all the products for swift run --build-all

I'd love to re-open the issue. I can work on PR to make it happen.

@ankitspd
Copy link
Member

SwiftPM roughly works in the following way:

  1. Resolve external dependencies
  2. Load packages on disk to an internal representation
  3. Resolve dependencies between all package, target and product
  4. Build package graph based on above resolution
  5. Create build manifest using the package graph
  6. Invoke llbuild with the manifest to perform the compilation tasks

`swift build` will by default build everything except test targets unless you pass the option to build them (`-build-tests`). Similarly, `swift test` will build everything by default and there is an option to skip the building (`-skip-build`) and proceed straight to test running.

These options are meant to control how llbuild is invoked. The manifest will always have complete list of tasks (including tests). The error you were running into was at the 2nd stage because there was a target defined in the manifest which wasn't present on the disk. Even though you were not trying building it, SwiftPM still has to load it. This is important because it needs to form the correct internal representation in order to perform rest of the operations.

Another advantage of this approach is that we can cache the build manifest and re-contruct it only if something changes. This is experimentally implemented on master using `--enable-build-manifest-caching` option.

The suggested `-build-all` option on `swift run` makes sense to me (we should call it `-build-tests` though) but declaring a missing target should still be an error IMO.

@krzyzanowskim
Copy link
Contributor Author

exactly. While `swift build` does build all targets, the `swift run` is not (atm). That needs two calls to build and run it all:

swift build && swift run

since `swift run` already builds the executables, it could build the other products too (not tests). Let's say I have two products: executable and library.

@ankitspd
Copy link
Member

Oh you're right, we only build the executable that you're running. Ok, --build-all makes sense for swift run!

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
@shahmishal shahmishal transferred this issue from apple/swift May 4, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants