Merge tests defined in extensions

[plemarquand]

Merge the XCTests and swift-testing tests defined in extensions into their parent TestItems.

This is done as another pass after the TestScanner visitors have walked the tree.

Fixes #1218

[plemarquand]

@ahoppen this is a pretty naive first pass at a solution. It does outline the expected behaviour with some new tests, which should be helpful going forward.

[plemarquand]

@swift-ci Please test

[ahoppen]

Hmm, I’m not sure if we can or want to do this in general. The underlying problem is if we have the following

FileA.swift

struct MyTests {
  @Test
  func runTest() {}
}

FileB.swift

private struct MyTests {
  @Test
  func runTest() {}
}

FileC.swift

extension MyTests {
  @Test
  func anotherTest() {}
}

Then MyTests from FileA.swift and FileB.swift are distinct test suites (yes we don’t handle that currently, but we should be able to when we add file names to the test IDs), so we shouldn’t merge them. Figuring out that extension MyTests should be merged with MyTests from FileA.swift requires module-wide name lookup which can’t be done syntactically.

That’s why I decided to consider each extension as a separate node within the tree of test items. That also has the advantage that there is a test item that covers extension MyTests, so that in VS Code you get an indicator in the side gutter to run the MyTests tests. I think after merging them, you won’t get that.

What do you think? Does my line of thinking make sense? How bad is having a separate item for an extension?

[plemarquand]

@ahoppen The example you outline is a tricky one for sure. I knew there would be dragons in the details on this one, which is why I called this out as being a naive implementation.

I think from a user experience perspective breaking out extensions in to their own "suites" doesn't match Swift's conceptual model of extensions. If extensions are additions to existing types, then in a test context I'd expect extensions to be an addition to an existing suite.

If it makes sense to a user to run all the tests in an extension as a group then I'd argue that group should be its own well defined suite.

Is wrangling extensions across the workspace a common problem in sourcekit? Extensions kind of break syntactic analysis at a file level.

[ahoppen]

The merging problem truly is harder than it seems at first glance. I left a few ideas for more tests that I think we should cover. It probably means adding more information to the syntactic test scanner about whether a test item is an extension or not, which we probably don’t want to expose as part of the textDocument/tests and workspaces/tests requests.

[ahoppen]

And another test case that I think would be worth adding. In this case we should pick the struct as the primary test suite location but I think right now we pick the extension MyTests.

FileA.swift

extension MyTests {
  @Test func twoIsThree() {}
}

FileB.swift

@Suite struct MyTests {
  @Test func oneIsTwo {}
}

I think this means we need to add some information about whether a test is an extension to the syntactic test scanner because otherwise we can’t distinguish the extension from the struct definition during the merge operation. Once we have that, we should really only merge extensions into their defining type. I think that has a few other advantages as well:

• We will never accidentally merge two private suites from different files.
• If we have an extension from a type that is syntactically ambiguous (eg. extension of a private suite whose name is used in two files), we could decide to not merge it for now.
• It can be more performant because we only need to try merging extensions.

[plemarquand]

This was a good test case to add for sure. I've refactored so now the tests in struct MyTests will appear in the list before those defined in extensions.

TestItem now has an isExtension flag which, while not only necessary to sort/merge things correctly, may also be interesting to tools developers. Thoughts?

[plemarquand]

@swift-ci Please test

[plemarquand]

@swift-ci Please test

[plemarquand]

@swift-ci Please test

[ahoppen]

Nice. Let’s 🚢 it

[ahoppen]

@swift-ci Please test Windows

[plemarquand]

@swift-ci Please test Windows