search

swiftlang / vscode-swift

Visual Studio Code Extension for Swift
https://marketplace.visualstudio.com/items?itemName=sswg.swift-lang
Apache License 2.0
708 stars 47 forks source link

Support parameterized swift-testing tests #797

Closed plemarquand closed 1 month ago

plemarquand commented 1 month ago

Swift-testing emits a test event at the beginning of a test run that enumerates all the parameterized test cases to be executed.

This patch waits for the test event and then generates vscode.TestItems for each parameterized test execution, parenting them to their test. Then when we recieve a runStarted event we enqueue all the tests along with the newly created parameterized TestItems.

Before a new test run starts we remove the existing parameterized TestItems so we can regenerate them, as there may be a different number of parameterized tests run with every execution.

adam-fowler commented 1 month ago

This doesn't appear to work. SKLSP is returning parameterized tests as funcName(_ :), while swift-testing is expecting funcName(_:) which doesn't include the space.

plemarquand commented 1 month ago

@adam-fowler sorry I forgot to mention this depends on https://github.com/apple/sourcekit-lsp/pull/1209. For now to test you'd need to build your own sourcekit-lsp with that change included, as it looks like the nightly toolchain builds aren't working.

adam-fowler commented 1 month ago

Outside of the fact I can't compile, other issues include I cannot run a test suite that includes a parameterised test, and deleting tests doesn't always delete the test in the test explorer.

plemarquand commented 1 month ago

If you delete a parameterised test it doesn't delete it from the test explorer

Fixed, the code to prune the test item tree works bottom up and assumed items being pruned would have no children. Added a check to still prune if all children are parameterized test results.

Tests with named parameters don't run. Looks like SKLSP is giving you the wrong name for the test eg

I couldn't reproduce this, trying on both macOS and in the Linux dev container. What were you seeing? The tests would run/complete but you wouldn't get new parameterized test results in the Test Explorer?

Parameterized test items don't have a location

This is by design, since results don't really have a "location" per se. The parent TestItem should always be visible in the Test Explorer even scrolling a long list of items, and the tests location remains available off that. Its also a handy way to distinguish if an item in the TestItem hierarchy is a parameterized test result entry or not.

If I ask one of them to run nothing happens. Ideally it would only run the test with that one case. If you can't do that I'm not sure if it is worth it keeping a test item for every case.

It isn't possible today, but we can look at it for a subsequent PR.

adam-fowler commented 1 month ago

I couldn't reproduce this, trying on both macOS and in the Linux dev container. What were you seeing? The tests would run/complete but you wouldn't get new parameterized test results in the Test Explorer?

Ok. I think this was a symptom of the first issue. I had a function where the parameter didn't have a label and I edited it to include a label for the parameter, but it never deleted the test items and I was still running with those old test items. Your fix for the deletion of test items for parametized test functions fixes this also.

adam-fowler commented 1 month ago

If I ask one of them to run nothing happens. Ideally it would only run the test with that one case. If you can't do that I'm not sure if it is worth it keeping a test item for every case.

It isn't possible today, but we can look at it for a subsequent PR.

Can we at least get it to run the test the parameter belongs to

plemarquand commented 1 month ago

If I ask one of them to run nothing happens. Ideally it would only run the test with that one case. If you can't do that I'm not sure if it is worth it keeping a test item for every case.

It isn't possible today, but we can look at it for a subsequent PR.

Can we at least get it to run the test the parameter belongs to

I can, but I worry having the behaviour change in a subsequent release would be confusing. In the future it would go from running all the test parameters to running the test with just one.

Tests with parameter types that aren't Codable wouldn't be able to be run individually, and so they'd lose the play button (or rerun the test with all parameters, but I think that'd be even more confusing).

plemarquand commented 1 month ago

@adam-fowler you were right that it didn't make sense for these parameterized test items to have run buttons at all. I've added a "runnable" tag to all the test items that can be run, associated that with the run profiles, and this omits the button on parameterized tests as you'd expect.

https://github.com/swift-server/vscode-swift/assets/667875/542210a0-e5b8-4472-af10-f67e8ec3d200