swiftlang / swift-package-manager

The Package Manager for the Swift Programming Language
Apache License 2.0
9.71k stars 1.33k forks source link

swift test [list] --skip-build still checkouts dependencies #7113

Open tristanlabelle opened 10 months ago

tristanlabelle commented 10 months ago

Description

swift test list always fetches and create working copies for git package dependencies, even if invoked with --skip-build and/or --skip-update.

This was discovered in the context of https://github.com/swift-server/vscode-swift/pull/638 . The Swift VS Code extension automatically tries to discover tests at startup by running this command, which for large projects ends up causing a long-lasting checkout operation in the background, which can then overlap with swift build commands and cause corrupted checkouts.

(The rationale for this bug also applies for bare swift test, without list)

Expected behavior

--skip-build should also imply not resolving packages.

Actual behavior

No response

Steps to reproduce

  1. Create a new swift project with one git package dependency
  2. Run swift test list --skip-build or swift test list --skip-build --skip-update

Swift Package Manager version/commit hash

Swift Package Manager - Swift 5.10.0-dev

Swift & OS version (output of swift --version ; uname -a)

compnerd.org Swift version 5.11-dev (LLVM 3d6e0f96eb2caa2, Swift f6e8fd2f369d95d) Target: x86_64-unknown-windows-msvc (main branch build from 2023-11-16)

grynspan commented 9 months ago

In order to list tests, the test target must be built and run, so the right solution may be to disallow these arguments with swift test and swift test list.

adam-fowler commented 9 months ago

In order to list tests, the test target must be built and run, so the right solution may be to disallow these arguments with swift test and swift test list.

That would be problematic for the VSCode extension and any other UI reliant on getting the test list using swift test list. Triggering a build, which could be a long process, to update a UI is not ideal.

We currently don't have a method to get the list of tests that doesn't require a build at some point but being able to use the results from previous builds helps make the UI more responsive. And if we ask to skip the build we would expect to skip the resolve that is triggered prior to that.

grynspan commented 9 months ago

I don't have any objection to having --skip-build or --skip-update imply "don't resolve dependencies", or alternatively to adding a new --skip-resolution option. The issue description seems to be describing a clean package build, so forgive me if I'm confusing things—to be clear, you're concerned with subsequent builds only, not the initial build?

adam-fowler commented 9 months ago

I guess there could be situation where this is run on a clean package and at this point it returns no tests. I'm not sure I want that to trigger a build or resolution though as it triggers it at an awkward place in the UI processing. I would prefer to have full control over when the build/resolution occurs.

tristanlabelle commented 9 months ago

My thinking is along @adam-fowler 's lines: it's useful to have a "readonly" query for tests for tooling purposes. Also, it's not useful that --skip-build will still update the dependencies if it's not going to be able to list tests anyways because it won't build afterwards.

grynspan commented 9 months ago

I think I understand that part—where I'm a bit confused (sorry!) is what behaviour would be expected from the VSCode side on the first call. Zero results returned until the first time the user builds the package? (I am not arguing for or against that, just trying to understand what the expected behaviour would be.)

adam-fowler commented 9 months ago

Currently in VSCode if tests are unavailable it displays a message telling the user to build the package to get them.

There is a SourceKit-LSP solution in development which will also include test locations but until then I have to rely on swift test list.

grynspan commented 9 months ago

Understood. We've got an eye on that SK-LSP PR (https://github.com/apple/sourcekit-lsp/pull/978, right?) as well.

adam-fowler commented 9 months ago

Yep that's the one