chore: remove `outFile`,`module:amd` config from test cases by camc314 · Pull Request #64122 · microsoft/TypeScript

Cameron (camc314) · 2026-09-01T16:12:13Z

Summary

There is currently no test coverage for some error codes (e.g. TS8013, TS8016 errors).

This is because these test cases have @outFile/module=amd specified in their configuration. Any test fixture with @outFile/module=amd is skipped.

This PR is split into 5x logical commits:

remove outFile-specific test cases
remove incidental outFile test options
remove AMD specific test cases
remove incidental AMD test options
remove the test harness logic to skip these test cases.

This adds the missing baselines back into the repo allowing for these code paths to be properly tested.

This also has the advantage of removing the dead test cases that are not supported (and will not be supported), following the removal/deprecation of these options.

For context, we (oxc) use TS as baselines to ensure that we are correctly reporting errors. While trying to upgrade to the new repo structure, I've noticed that we are (incorrectly) expecting some stuff to parse when it should be failing - this PR helps to address that.

typescript-automation · 2026-09-01T16:15:57Z

This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise.

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Note

This error may be related to your runner configuration. You can now configure runners for Copilot code review separately from Copilot cloud agent by creating a copilot-code-review.yml file with your setup steps. Read the docs for details.

Cameron (camc314) · 2026-09-01T16:19:20Z

~~Hmm actually this makes sense to just remove outFile from all fixtures so that they can start to be tested - i'll update this PR.~~

Jake Bailey (jakebailey) · 2026-09-01T17:26:34Z

 func SkipUnsupportedCompilerOptions(t *testing.T, options *core.CompilerOptions) {
 	t.Helper()
 	switch options.Module {
-	case core.ModuleKindAMD, core.ModuleKindUMD, core.ModuleKindSystem:


Why remove the skips here? Is this leftover from you finding tests to fix? I'd hope we would still error if someone tries to introduce them somehow.

Why remove the skips here? Is this leftover from you finding tests to fix? I'd hope we would still error if someone tries to introduce them somehow.

It felt like the logical thing to do. But I see your point - let me push a commit that does t.Fail() if an old compiler option is specified.

Ah there's actually a bunch of stuff here I missed - let me take another proper look - apologies for marking it draft a little too early!

Should be fixed now - I've changed these cases to fail with t.Fatalf rather than silently skip. This flagged some other issues, so I've changed those tests using outFile to outDir, and module:amd to module:commonjs where needed to prevent excess diagnostics

Thanks; I wonder how much more it'd take to do this for all of the other options.

Probably also the func is misnamed but, it is somewhat temporary.

Thanks; I wonder how much more it'd take to do this for all of the other options.

happy to send followup PRs if this is directionally right - these were the first two I noticed that caused us issues. But I don't want to make this any harder to review

Sort of surprised this matters for you at all? These are our internal tests, right?

We read all of the fixtures to know what we should be succeeding/failing to parse. (since there's not really a TS conformance suite anywhere).

Since some of these have no errors.txt (because they were skipped), our harness was expecting these to parse without errors (incorrect behaviour)

We could just filter out cases that are skipped here, but then we risk not testing everything and possibly introducing regressions.

Jake Bailey (jakebailey)

I had copilot look at everything and it noticed a few things.

The inline comments cover the concrete test/baseline issues. Two additional cleanup/hardening notes do not land on changed lines:

tsc/testdata/tests/cases/compiler/isolatedModulesOut.ts is the sole remaining legacy @out fixture and is still permanently listed in skippedTests; it seems like it should be removed with the other outFile-only cases.
SkipUnsupportedCompilerOptions only sees fourslash global directives. An embedded tsconfig.json containing module: "amd" or outFile still runs; I confirmed both with temporary probes. There are no such configs left in this PR, so this is future-proofing rather than a current failing fixture, but the new rejection is not comprehensive.

For context, I also ran npx hereby build, npx hereby test, npx hereby lint, and npx hereby format at this commit; they all passed.

Jake Bailey (jakebailey) · 2026-09-01T22:51:23Z

-// @Filename: file0.ts
-/// <reference path="file1.ts" />
-/// <reference path="file2.ts" />
-/// <amd-dependency path="/js/libs/hgn.js!app/templates/home" name="compiler"/>


This was a live test with existing JS/symbol/type baselines. The only AMD-specific part is this one directive; the rest covers removeComments stripping triple-slash references from files that still emit JavaScript, which the surviving ...OnNotEmittedNode test does not cover. Could we retain this test and remove only this directive?

Restored > 0164435

Jake Bailey (jakebailey) · 2026-09-01T22:51:23Z

-// @Filename: file2.ts
-/// <reference path="file0.ts" />
-/// <reference path="file1.ts" />
-/// <amd-dependency path="/js/libs/hgn.js!app/templates/home" name="compiler"/>


Same concern here: this test was running before the PR, and its primary empty-file/triple-slash-reference coverage is not AMD-specific. Removing this directive should let us retain the test and its existing baselines.

Restored > 0164435

Jake Bailey (jakebailey) · 2026-09-01T22:51:23Z

@@ -1,5 +1,5 @@
 // @target: es2015
-// @module: amd
+// @module: commonjs


This now becomes byte-for-byte identical to pathMappingBasedModuleResolution2_node.ts. The AMD/CommonJS distinction was what made the _classic/_node pair exercise different resolution behavior, so this silently loses the classic scenario. Could this use an explicit supported classic-resolution configuration, or otherwise be retired rather than duplicated?

Updated here > 3b74e9d

Jake Bailey (jakebailey) · 2026-09-01T22:51:24Z

@@ -1,6 +1,6 @@
 // @target: es2015
 // @strict: false
-//@module: amd
+//@module: commonjs


This file is now byte-for-byte identical to sourceMapValidationExportAssignmentCommonjs.ts, so the newly generated baselines duplicate existing CommonJS coverage rather than preserving a distinct scenario. I think this should be removed rather than converted into a duplicate.

Fixed in 3b74e9d

Jake Bailey (jakebailey) · 2026-09-01T22:51:24Z

@@ -1,5 +1,5 @@
 // @target: es6
-// @module: amd
+// @module: commonjs


This change makes test 3 identical to test 1; similarly, test 4 becomes identical to test 2. These look like AMD-only matrix entries that should be removed instead of rewritten into duplicate CommonJS tests.

updated in bbb8283460

Jake Bailey (jakebailey) · 2026-09-01T22:51:24Z

 	defer testutil.RecoverAndFail(t, "Panic on fourslash test")
 	const content = `// @AllowSyntheticDefaultImports: false
-// @Module: amd
+// @Module: commonjs


After this change, this test is identical to TestImportNameCodeFixNewImportAllowSyntheticDefaultImports3 except for the Go function name. Could we remove the unsupported AMD matrix entry instead of keeping duplicate CommonJS coverage?

yep fixed here > 1a68930

Jake Bailey (jakebailey) · 2026-09-01T22:51:24Z

@@ -1,4 +1,4 @@
-//@module: amd
+//@module: commonjs


The test still says it should emit react-router in the AMD dependency list, but its new baseline emits CommonJS require calls. This rewrite no longer exercises the stated behavior; the test should either be retired or updated to assert a genuinely relevant CommonJS behavior.

Deleted the test as it's no longer relevant > 568f7ce

Jake Bailey (jakebailey) · 2026-09-01T22:51:24Z

-    "compilerOptions": {
-        "module": "amd",
-        "target": "ES3",
+    "compilerOptions": {


This edit changes only the touched lines from CRLF to LF, leaving this fixture with mixed line endings. The same happened in deprecatedCompilerOptions6.ts, emitHelpersWithLocalCollisions.ts, moduleImportedForTypeArgumentPosition.ts, tsconfigMapOptionsAreCaseInsensitive.ts, and varArgsOnConstructorTypes.ts. Could these edits preserve each file’s existing CRLF endings?

oops - my find and replace has not worked as expected > 987c8e5

Jake Bailey (jakebailey) · 2026-09-01T22:51:24Z

@@ -0,0 +1,29 @@
+//// [tests/cases/conformance/statements/VariableStatements/usingDeclarations/usingDeclarationsTopLevelOfModule.2.ts] ////


The old usingDeclarationsTopLevelOfModule.2(module=commonjs).js baseline still exists and is byte-for-byte identical to this new unparameterized baseline. Since the module variation collapsed to one value, the old parameterized baseline should be deleted; unused-baseline tracking is currently disabled, so CI does not catch it.

Removed here > 62c5f229a2

Cameron (camc314) · 2026-09-02T08:19:00Z

tsc/testdata/tests/cases/compiler/isolatedModulesOut.ts is the sole remaining legacy @out fixture and is still permanently listed in skippedTests; it seems like it should be removed with the other outFile-only cases.

Fixed in Remove the last legacy outFile fixture

SkipUnsupportedCompilerOptions only sees fourslash global directives. An embedded tsconfig.json containing module: "amd" or outFile still runs; I confirmed both with temporary probes. There are no such configs left in this PR, so this is future-proofing rather than a current failing fixture, but the new rejection is not comprehensive.

Fixed in 5a7da7f - it now fails on invalid 4slash configs.

Thanks - updated all comments based on the feedback.

typescript-automation Bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Sep 1, 2026

Cameron (camc314) marked this pull request as ready for review September 1, 2026 16:15

Copilot AI balanced review requested due to automatic review settings September 1, 2026 16:15

Copilot started reviewing on behalf of Cameron (camc314) September 1, 2026 16:16 View session

Copilot AI reviewed Sep 1, 2026

Cameron (camc314) marked this pull request as draft September 1, 2026 16:19

Cameron (camc314) changed the title ~~chore: re-enable test coverage for TS8013, TS8016~~ chore: remove outFile config from most test cases Sep 1, 2026

Cameron (camc314) marked this pull request as ready for review September 1, 2026 16:27

Cameron (camc314) marked this pull request as draft September 1, 2026 16:34

Cameron (camc314) changed the title ~~chore: remove outFile config from most test cases~~ chore: remove outFile,module:amd config from test cases Sep 1, 2026

Cameron (camc314) force-pushed the c/test-cov-8013-8016 branch from 4f948ef to ec02195 Compare September 1, 2026 16:47

Cameron (camc314) marked this pull request as ready for review September 1, 2026 16:50

test: remove outFile-specific test cases

06f0f3e

Cameron (camc314) force-pushed the c/test-cov-8013-8016 branch from 86bafc9 to 60eb165 Compare September 1, 2026 16:52

Cameron (camc314) mentioned this pull request Sep 1, 2026

fix(coverage): support new TypeScript fixture layout oxc-project/oxc#26226

Draft

Jake Bailey (jakebailey) reviewed Sep 1, 2026

View reviewed changes

Cameron (camc314) marked this pull request as draft September 1, 2026 17:40

Cameron (camc314) force-pushed the c/test-cov-8013-8016 branch from 60eb165 to d7f5e11 Compare September 1, 2026 18:38

Cameron (camc314) added 4 commits September 1, 2026 20:17

test: remove incidental outFile options

9d89225

test: remove AMD-specific test cases

5f8cad7

test: remove incidental AMD module options

cbf5623

test: reject outFile and AMD test options

5f3eef8

Cameron (camc314) force-pushed the c/test-cov-8013-8016 branch from d7f5e11 to 5f3eef8 Compare September 1, 2026 19:19

Cameron (camc314) marked this pull request as ready for review September 1, 2026 19:21

Jake Bailey (jakebailey) reviewed Sep 1, 2026

View reviewed changes

Cameron (camc314) added 3 commits September 2, 2026 08:30

Keep the triple-slash comment emit coverage

0164435

Drop the duplicated classic path mapping test

3007025

Remove duplicate export assignment source map coverage

3b74e9d

Cameron (camc314) added 7 commits September 2, 2026 08:47

Remove the stale using declaration baseline

62c5f22

Preserve CRLF endings in compiler fixtures

987c8e5

Remove the duplicate synthetic default import test

1a68930

Retire the AMD-specific JSX emit test

568f7ce

Remove duplicate destructuring module tests

bbb8283

Remove the last legacy outFile fixture

cb615cb

Check compiler options in fourslash configs

5a7da7f

		@@ -0,0 +1,29 @@
		//// [tests/cases/conformance/statements/VariableStatements/usingDeclarations/usingDeclarationsTopLevelOfModule.2.ts] ////

Uh oh!

Conversation

Cameron (camc314) commented Sep 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

typescript-automation Bot commented Sep 1, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Cameron (camc314) commented Sep 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Jake Bailey (jakebailey) left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Cameron (camc314) Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Cameron (camc314) commented Sep 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Cameron (camc314) commented Sep 1, 2026 •

edited

Loading

Cameron (camc314) commented Sep 1, 2026 •

edited

Loading

Cameron (camc314) Sep 2, 2026 •

edited

Loading