-
Notifications
You must be signed in to change notification settings - Fork 13.2k
Remove compiler runner libFiles option entirely #63060
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR removes the @libFiles compiler test runner option in favor of using standard /// <reference path="/.lib/..." /> directives. This consolidates the two methods previously available for including library files from /tests/lib, standardizing on the reference directive approach which was already more widely used (220 times vs 93 times for @libFiles).
Changes:
- Removed
libFilesoption from test harness infrastructure - Updated test baselines to reflect new reference directive positioning
- Modified baseline output to preserve full test paths instead of removing prefixes
Reviewed changes
Copilot reviewed 300 out of 497 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/harness/harnessIO.ts | Removed libFiles option from test harness and updated file output formatting |
| tests/baselines/reference/*.errors.txt | Updated error line numbers due to added reference directives |
| tests/baselines/reference/*.types | Updated type baseline line numbers |
| tests/baselines/reference/*.symbols | Updated symbol baseline line numbers |
| tests/baselines/reference/*.js | Added /// <reference path="/.lib/..." /> directives to compiled output |
|
|
||
| function fileOutput(file: documents.TextDocument, harnessSettings: TestCaseParser.CompilerSettings): string { | ||
| const fileName = harnessSettings.fullEmitPaths ? Utils.removeTestPathPrefixes(file.file) : ts.getBaseFileName(file.file); | ||
| return "//// [" + fileName + "]\r\n" + Utils.removeTestPathPrefixes(file.text); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was mistakenly changing paths in emitted reference directives, leaving them in a confusingly broken state. See the first commit for examples.
|
|
||
| //// [jsxDeclarationsWithEsModuleInteropNoCrash.d.ts] | ||
| /// <reference path="..react16.d.ts" preserve="true" /> | ||
| /// <reference path="../.lib/react16.d.ts" preserve="true" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was broken before due to an overzealous replacement.
We've had two ways to include things from
/.lib;@libFiles: react.d.tsand/// <reference path="/.lib/react.d.ts" />. We use the former 93 times, and the latter 220 times.I think it'd be best to get rid of this special option and just use plain references, now that we don't use
@libFiles: lib.d.tsafter #63056.Only sucky thing is that it changes positions and therefore a lot of baselines change.