Skip to content

Add infrastructure to define indexed single-file workspaces inside the tests #914

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 9 commits into from
Oct 24, 2023

Conversation

ahoppen
Copy link
Member

@ahoppen ahoppen commented Oct 20, 2023

The new approach has a few advantages over the olde TIBS-based approach:

  1. The source file being tested is defined within the test case itself and not in a separate file, which makes it easier to understand the test case since the auxiliaury file doesn’t need to be opened. Finding it inside Sources/SKTestSupport/INPUTS is already hard for people that are not familiar with the codebase.
  2. The build setup is significantly simpler since it doesn’t rely on ninja. It is thus easier to understand what is run during the test.
  3. We can use the emoji location markers to refer to test locations, like we do for files that are opened using TestSourceKitLSPClient.openDocument.

This commit only migrates call hierarchy testing to the new design. If we like it, I’ll migrate the other test workspaces as well.

Copy link
Contributor

@bnbarham bnbarham left a comment

Choose a reason for hiding this comment

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

I prefer source next to the test, so in general I'm in favor of this. I'd be interested in seeing a multi-file test or one that has additional args though.

@ahoppen ahoppen force-pushed the ahoppen/single-indexed-file-workspace branch from 0018e14 to 0f29a1d Compare October 23, 2023 23:21
@ahoppen
Copy link
Member Author

ahoppen commented Oct 23, 2023

I'd be interested in seeing a multi-file test or one that has additional args though.

I migrated a bunch more tests from the TIBS-based test inputs to define the test inputs inline. I haven’t really needed to make any adjustments to the test workspaces for the last few migrations, so I think the design is more or less complete and it’s now just a matter of converting the remaining test cases to the new infrastructure.

@ahoppen ahoppen force-pushed the ahoppen/single-indexed-file-workspace branch from 0f29a1d to 734e4e1 Compare October 23, 2023 23:28
@ahoppen
Copy link
Member Author

ahoppen commented Oct 23, 2023

And today I learned about Process.checkNonZeroExit in swift-tools-support-core. That’s sufficient for the needs of this repo so at least we don’t need to introduce another copy of ProcessRunner.swift here 🎉

@ahoppen
Copy link
Member Author

ahoppen commented Oct 23, 2023

@swift-ci Please test

…e tests

The new approach has a few advantages over the olde TIBS-based approach:
1. The source file being tested is defined within the test case itself and not in a separate file, which makes it easier to understand the test case since the auxiliaury file doesn’t need to be opened. Finding it inside `Sources/SKTestSupport/INPUTS` is already hard for people that are not familiar with the codebase.
2. The build setup is significantly simpler since it doesn’t rely on `ninja`. It is thus easier to understand what is run during the test.
3. We can use the emoji location markers to refer to test locations, like we do for files that are opened using `TestSourceKitLSPClient.openDocument`.

This commit only migrates call hierarchy testing to the new design. If we like it, I’ll migrate the other test workspaces as well.
…orkspace that can be opened in SourceKitLSP
We don’t actually need on-disk files here and can just open them in memory.
We can just open the file in-memory instead
The tests never performed any cross-file testing so we can just import them as standalone files in-memory.
… arbitrary languages

This allows us to remove the `BasicCXX` test directory.
@ahoppen ahoppen force-pushed the ahoppen/single-indexed-file-workspace branch from 734e4e1 to 48b617c Compare October 24, 2023 15:44
@ahoppen
Copy link
Member Author

ahoppen commented Oct 24, 2023

@swift-ci Please test

@ahoppen
Copy link
Member Author

ahoppen commented Oct 24, 2023

@swift-ci Please test Windows

@ahoppen ahoppen merged commit 643d458 into swiftlang:main Oct 24, 2023
@ahoppen ahoppen deleted the ahoppen/single-indexed-file-workspace branch October 24, 2023 23:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants