refactor(ts-plugin): split go-to-definition e2e tests into per-behavior cases#378
Merged
Merged
Conversation
|
27be195 to
2ae09aa
Compare
…or cases Each test now owns a minimal fixture and asserts a single behavior, organized by the .d.ts/mapping pattern under test. The previous structure shared one large fixture (5 files, 8+ tokens) across 17 cases inside a single test.each block, which made it brittle to add new behaviors. The new structure has 14 tests grouped into 4 describe blocks: - jumps to the top of a CSS module file (5 tests) - jumps to a local token declaration (3 tests) - transitively resolves a re-export to its source declaration (3 tests) - jumps from a CSS-side @value ... from binding to its source declaration (3 tests) Adds: - setupFixture / getLoc / getRange in e2e-test/test-util/fixture.ts. The API mirrors createIFF and accepts files directly (including tsconfig.json), so each test controls its own config placement. - buildStylesImport / buildTSConfigJSON in packages/ts-plugin/test/builder.ts for building default / namespace import statements and tsconfig.json fixture content. cmkOptions.enabled defaults to true so callers only specify the options that matter for the test.
2ae09aa to
d32bf36
Compare
Merged
5 tasks
mizdra
added a commit
that referenced
this pull request
May 3, 2026
…s into per-behavior cases (#379) * test(ts-plugin): split find-all-references and rename-symbol e2e tests into per-behavior cases Continue the e2e test refactor started in #378. Each test now owns a minimal fixture and asserts a single behavior, organized by the .d.ts/mapping pattern under test. Also revisit go-to-definition.test.ts so all three feature test files share the same describe structure. This is the second PR of a multi-PR series (see #378 for the first PR). Shared describe structure (group 1 differs by feature, groups 2-5 are common): - Group 1: feature-specific Volar.js auto-mapping - go-to-definition: jumps to the top of a CSS module file (5 tests) - find-all-references: finds all references to the styles binding (1 test) - rename-symbol: (none — module specifier rename is sendGetEditsForFileRename territory and styles binding rename is core TS behavior, not ts-plugin's) - Group 2: <feature> a local token (TS-side trigger, 3 tests) - Group 3: transitively resolves a re-export to its source declaration (3 tests) - Group 4: <feature> from a CSS-side class declaration (1 test, newly added) - Group 5: <feature> from a CSS-side @value ... from binding (3 tests) Per-file totals: - go-to-definition: 15 tests x 2 namedExports variants = 30 - find-all-references: 11 tests x 2 namedExports variants = 22 (was 17 cases sharing one fixture inside a single test.each) - rename-symbol: 10 tests x 2 namedExports variants = 20 (was 13 cases inside a single test.each, only namedExports: false) rename-symbol now also covers namedExports: true via describe.each. * refactor(ts-plugin): move test/builder.ts under src/test/ to match other packages * test(ts-plugin): rename multi-declaration test to a scenario qualifier Other test names in the same describe blocks describe a trigger position or fixture shape (e.g. "from styles.<token> access"), but this one was phrased as a result ("returns all declarations ..."). Drop the verb so the name reads as a scenario qualifier on the parent describe and stays consistent across all three feature files. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
3 tasks
mizdra
added a commit
that referenced
this pull request
May 4, 2026
…avior cases (#380) * test(ts-plugin): split completion and code-fix e2e tests into per-behavior cases Refactor both test files so each `test` block owns a minimal fixture and asserts a single behavior, organized by the .d.ts/mapping pattern under test. Follows the structure established in #378 and #379. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(ts-plugin): drop unused JSX compilerOptions from styles priority test The fixture only contains `styles;` and never renders any JSX, so the `jsx` and `types` compilerOptions are not needed for that test. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(ts-plugin): cover named token suppression when prioritizeNamedImports is false Add the negative-side cases for the `namedExports: true && !prioritizeNamedImports` branches so that both the positive (current `prioritizeNamedImports: true`) and negative behaviors of the suppression logic are pinned: - completion: named tokens are filtered out of suggestions - code-fix: named import code fixes are excluded Reorganize the existing `prioritizeNamedImports (namedExports: true)` describe into two parallel `prioritizeNamedImports: false / true` blocks under a single parent so the on/off pair is visually adjacent. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(ts-plugin): drop redundant (default) suffix from describe names `prioritizeNamedImports: false` already conveys the configured value; the `(default)` annotation is redundant and risks going stale if the default changes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(ts-plugin): cover generated-file exclusion and multi-import fixMissingCSSRule Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(ts-plugin): cover both PROPERTY_DOES_NOT_EXIST diagnostics in fixMissingCSSRule Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 4, 2026
mizdra
added a commit
that referenced
this pull request
May 5, 2026
) * test(ts-plugin): split refactor / rename-file / disabled / invalid-syntax / pure-css-file / ignore-generated-files e2e tests into per-behavior cases Each test now owns a minimal fixture and asserts a single behavior, completing the multi-PR series started in #378. Hard-coded line/offset literals are replaced with getRange / getLoc, and the inline diagnostic snapshot in ignore-generated-files is replaced with a structural toStrictEqual. Also adds a missing case to refactor.test.ts: the Create CSS Module file refactor is suppressed when the paired *.module.css already exists. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(ts-plugin): rename ignore-generated-files test to focus on the .d.ts exclusion behavior The previous name described the observable diagnostic; the new name foregrounds the actual invariant under test: ts-plugin removes generated .d.ts files from the module resolution path even when their directory is listed in rootDirs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Refactor
packages/ts-plugin/e2e-test/feature/go-to-definition.test.tsso that eachtestblock owns a minimal fixture and asserts a single behavior, organized by the .d.ts/mapping pattern under test. The previous structure shared one large fixture (5 files, 8+ tokens) across 17 cases inside a singletest.each, which made it brittle to add new behaviors.This is the first PR of a multi-PR series. Subsequent PRs will apply the same approach to the other
e2e-test/files (find-all-references, rename-symbol, completion, etc.).What's new
packages/ts-plugin/e2e-test/test-util/fixture.ts: addssetupFixture(files)— mirrorscreateIFF's shape; callers pass files directly (includingtsconfig.json), so each test can place its config wherever it likes.getLoc(file, search, index?)— 1-based line/offset of the first character ofsearchinfile. Throws ifsearchis ambiguous andindexis omitted.getRange(file, search, index?)—{ start, end }range withendexclusive (matches tsserver's convention).packages/ts-plugin/test/builder.ts(new): code-fragment builders for fixture filesbuildTSConfigJSON({ compilerOptions?, cmkOptions? })— defaultscmkOptions.enabledtotrue.buildStylesImport(specifier, { namedExports })— builds default / namespacestylesimport statements at call sites.packages/ts-plugin/e2e-test/feature/go-to-definition.test.ts: rewritten as 14 behavior-focused tests, each with its own minimal fixture, organized into 4describeblocks. Each fixture uses contiguous token / filename numbering (e.g.a.module.css+b.module.css,.a-1,b_1) so it only references the files and tokens the test actually needs.Test structure
Each describe corresponds to a distinct .d.ts/mapping pattern produced by
dts-generator.ts:Total: 14 tests × 2 variants (
namedExports: false / true) = 28 tests.Per-syntax parser coverage (e.g.
.foo,@value foo: red;,@keyframes foo) lives indts-generator.test.ts; here we only cover one representative for each .d.ts/mapping pattern.Test plan
vp test --project e2e packages/ts-plugin/e2e-test/feature/go-to-definition.test.ts— 28 passedvp test --project e2e(full e2e suite) — no regressionsvp check(format + lint + type) — pass