It's helpful for stability to not have the path in the repo reflected in
test files, something I'm separately running into. So to reduce this,
align INCLUDE-FILE with other split behavior:
- Use the filename (with a "include_files/" subdir to disambiguate),
rather than the full path.
- Note if we eventually want to support splits in these, the same
approach could be extended.
- Only provide as an arg if the user requests files as args.
Factoring AddFile back because it's hard to share; I'm also advocating
to remove the prelude manifest, which would mean the remaining call
could be removed.
Currently file tests for LSP must provide carbon source code as an
escaped string within notify params, i.e.
```
[[@LSP-NOTIFY:textDocument/didOpen:
"textDocument": {
"uri": "file:/class.carbon",
"languageId": "carbon",
"text": "class A {\n fn F();\n fn G() {}\n}\n"
}
]]
```
This works fine for simple, single line files but gets annoying when
working with more complicated files which are necessary when testing
more complicated features e.g. goto-definition
```
--- class.carbon
class A {
fn F();
fn G() {}
}
--- STDIN
[[@LSP-NOTIFY:textDocument/didOpen:
"textDocument": {
"uri": "file:/class.carbon",
"languageId": "carbon",
"text": "AUTOFILL"
}
]]
```
This PR extends file_test_base to be able to parse the notify/call
params and inject files from the test_file's splits into the JSON input.
I purposely avoid using the clangd types and manually parse the
llvm::json::Value here to avoid introducing a depdendency on clangd to
the generic file_test_base, but happy to change if we think that is
fine. Also happy to accept other suggestions on alternative methods to
achieve same result.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
The INCLUDE-FILE option is only used in the toolchain tests for now. If
specified in a file test, the given file path is added to the test's
arguments. For toolchain tests this makes the file's package available
to the test. The `--custom-core` command line flag is added to the
driver, which avoids adding the production `Core` package to the command
line. Together, these allow a test to provide their own minimal `Core`
package.
For example, this would replace `Core` with the package and prelude in
`facet_types.carbon`.
```
// INCLUDE-FILE: toolchain/testing/min_prelude/facet_types.carbon
// EXTRA-ARGS: --custom-core
```
To support this:
* //testing knows how to parse INCLUDE-FILE out of the header of a test
file.
* //testing adds the file to the virtual file system, and includes it in
the test's arguments.
* //toolchain/driver grows the --custom-core command line flag to avoid
loading the production `Core` package.
Tests that were creating their own minimal prelude to define BitAnd on
types are now pointed to
toolchain/testing/min_prelude/facet_types.carbon as the prelude. They no
longer need to `import Core` in each test as a result.
Such tests are no longer `no_prelude`, but instead have their own
prelude. So they are moved to a `min_prelude` subdirectory.
Closes#5076
Noted CopyNameFromImportIR while glancing around (this one's interesting
because it's NameId, not void nor auto), did a scan just for a few other
cases. Not an exhaustive fix, and TBH assuming we'd prefer `auto ... ->
auto` since equivalent Carbon syntax would probably be `fn ... -> auto`
I'm refactoring logic to try to make these files more manageable,
particularly as I'm looking at ways to use more threads. I'm renaming
FileTestBase::TestContext to TestFile and FileTestBase::TestFile to
TestFile::Split to try to be more consistent in how we talk about test
files and file splits in general. This change is not expected to change
any behavior, it's just refactoring.
This PR has two commits:
- Copying files for viewing deltas in GH
- Moving logic
The delta of the two commits is the main PR. The second commit can be
viewed on its own to see the delta versus file_test_base.* files where
the logic currently rests (there's still a reordering in run_test.cpp as
part of making one function static).
Jon Ross-Perkins
DavidLoftusandJon Ross-Perkins
Dana Jansens