Skip to content

Change main file tests to define sources inline and remove tibs testing infrastructure #952

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

Merged
merged 2 commits into from
Oct 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 1 addition & 76 deletions Documentation/Development.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ $ swift build

Install the following dependencies of SourceKit-LSP:

* libsqlite3-dev libncurses5-dev python3 ninja-build
* libsqlite3-dev libncurses5-dev python3

```sh
$ export PATH="<path_to_swift_toolchain>/usr/bin:${PATH}"
Expand Down Expand Up @@ -134,81 +134,6 @@ Where "N" configures the log verbosity and is one of the following numbers: 0 (e

As much as is practical, all code should be covered by tests. New tests can be added under the `Tests` directory and should use `XCTest`. The rest of this section will describe the additional tools available in the `SKTestSupport` module to make it easier to write good and efficient tests.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume your opinion here is that the tests are now obvious enough without any description?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My opinion here is that we should write more documentation that’s up-to-date.

This paragraph in particular doesn’t add any value IMO. It’s mostly just: There should be tests and we use XCTest, which, I think is obvious.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be clear, my comment was just on the first changed line.

My opinion here is that we should write more documentation that’s up-to-date.

Sounds good 👍

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, for those, I would say that

As much as is practical, all code should be covered by tests. New tests can be added under the Tests directory and should use XCTest.

is obvious and not something that needs to be mentioned in documentation.

### Test Projects (Fixtures)

SourceKit test projects should be put in the `SKTestSupport/INPUTS` directory. Generally, they should use the [Tibs](#tibs) build system to define their sources and targets. An exception is for tests that need to specifically test the interaction with the Swift Package Manager. An example Tibs test project might look like:

```
SKTestSupport/
INPUTS/
MyTestProj/
a.swift
b.swift
c.cpp
```

Where `project.json` describes the project's targets, for example

```
{ "sources": ["a.swift", "b.swift", "c.cpp"] }
```

Tibs supports more advanced project configurations, such as multiple swift modules with dependencies, etc. For much more information about Tibs, including what features it supports and how it works, see [Tibs](#tibs).

### SKTibsTestWorkspace

The `SKTibsTestWorkspace` pulls together the various pieces needed for working with tests, including opening a connection to the language server, building the project to produce index data, loading source code into open documents, etc.

To create a `SKTibsTestWorkspace`, use the `staticSourceKitTibsWorkspace` method (the intent is to provide a `mutableSourceKitTibsWorkspace` method in the future for tests that mutate source code).

```swift
func testFoo() {
// Create the workspace, including opening a connection to the TestServer.
guard let ws = try staticSourceKitTibsWorkspace(name: "MyTestProj") else { return }
let loc = ws.testLoc("myLocation")

// Build the project and populate the index.
try ws.buildAndIndex()

// Open a document from the test project sources.
try ws.openDocument(loc.url, language: .swift)

// Send requests to the server.
let response = try ws.sk.sendSync(...)
}
```

#### Source Locations

It is common to want to refer to specific locations in the source code of a test project. This is supported using inline comment syntax.

```swift
Test.swift:
func /*myFuncDef*/myFunc() {
/*myFuncCall*/myFunc()
}
```

In a test, these locations can be referenced by name. The named location is immediately after the comment.

```swift

let loc = ws.testLoc("myFuncDef")
// TestLocation(url: ..., line: 1, column: 19)
```

`TestLocation`s can be easily converted to LSP `Location` and `Position`s.

```swift
Location(ws.testLoc("aaa:call"))
Position(ws.testLoc("aaa:call"))
```

### Long tests

Tests that run longer than approx. 1 second are only executed if the the `SOURCEKIT_LSP_ENABLE_LONG_TESTS` environment variable is set to `YES` or `1`. This, in particular, includes the crash recovery tests.

## Tibs

We use Tibs, the "Test Index Build System" from the IndexStoreDB project to provide build system support for test projects, including getting compiler arguments and building an index.
For much more information about Tibs, see [IndexStoreDB/Documentation/Tibs.md](https://github.com/apple/indexstore-db/blob/main/Documentation/Tibs.md).
4 changes: 0 additions & 4 deletions Sources/SKTestSupport/INPUTS/MainFiles/a.swift

This file was deleted.

4 changes: 0 additions & 4 deletions Sources/SKTestSupport/INPUTS/MainFiles/b.swift

This file was deleted.

1 change: 0 additions & 1 deletion Sources/SKTestSupport/INPUTS/MainFiles/bridging.h

This file was deleted.

7 changes: 0 additions & 7 deletions Sources/SKTestSupport/INPUTS/MainFiles/c.cpp

This file was deleted.

7 changes: 0 additions & 7 deletions Sources/SKTestSupport/INPUTS/MainFiles/d.cpp

This file was deleted.

25 changes: 0 additions & 25 deletions Sources/SKTestSupport/INPUTS/MainFiles/project.json

This file was deleted.

1 change: 0 additions & 1 deletion Sources/SKTestSupport/INPUTS/MainFiles/shared.h

This file was deleted.

9 changes: 0 additions & 9 deletions Sources/SKTestSupport/INPUTS/MainFiles/unique.h

This file was deleted.

6 changes: 4 additions & 2 deletions Sources/SKTestSupport/MultiFileTestWorkspace.swift
Original file line number Diff line number Diff line change
Expand Up @@ -113,11 +113,13 @@ public class MultiFileTestWorkspace {
/// Opens the document with the given file name in the SourceKit-LSP server.
///
/// - Returns: The URI for the opened document and the positions of the location markers.
public func openDocument(_ fileName: String) throws -> (uri: DocumentURI, positions: DocumentPositions) {
public func openDocument(_ fileName: String, language: Language? = nil) throws -> (
uri: DocumentURI, positions: DocumentPositions
) {
guard let fileData = self.fileData[fileName] else {
throw Error.fileNotFound
}
let positions = testClient.openDocument(fileData.markedText, uri: fileData.uri)
let positions = testClient.openDocument(fileData.markedText, uri: fileData.uri, language: language)
return (fileData.uri, positions)
}

Expand Down
217 changes: 0 additions & 217 deletions Sources/SKTestSupport/SKSwiftPMTestWorkspace.swift

This file was deleted.

Loading