test(linter/plugins): remove defunct test case#16794
Conversation
github-actions
bot
added
A-linter
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
![charliecreates[bot]](https://avatars.githubusercontent.com/in/1140780?s=60&v=4){kind=small}
There was a problem hiding this comment.
No issues found in the modified code. This change cleanly removes a defunct fixture and its related files without leaving partial edits in the shown diff.
Summary of changes
What changed
Removed a defunct linter plugin test fixture under apps/oxlint/test/fixtures/sourceCode_late_access_after_only/:
- Deleted fixture configuration:
apps/oxlint/test/fixtures/sourceCode_late_access_after_only/.oxlintrc.json - Deleted fixture inputs:
files/1.js,files/2.js - Deleted snapshot output:
output.snap.md - Deleted the associated plugin implementation:
plugin.ts(including acreateOncerule andafter()reporting)
This aligns with the PR context that the scenario the fixture attempted to validate is no longer representable after #14401.
There was a problem hiding this comment.
Pull request overview
This PR removes a defunct test fixture (sourceCode_late_access_after_only) that was attempting to test an impossible scenario. The test was meant to verify that context.sourceCode is available in the after hook when the AST is not traversed, but the test's implementation included a Program visitor function, which causes AST traversal. The scenario is impossible because if we remove the Program visitor to avoid AST traversal, the after hook won't run at all (as demonstrated by the hooksOnlyRule pattern in the createOnce fixture, where hooks without visitor functions are never executed).
Key Changes
- Removes the entire
sourceCode_late_access_after_onlytest fixture directory and all its files - Eliminates 5 files: plugin implementation, config file, two test input files, and expected output snapshot
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
apps/oxlint/test/fixtures/sourceCode_late_access_after_only/plugin.ts |
Removed the plugin implementation that attempted to test an impossible scenario |
apps/oxlint/test/fixtures/sourceCode_late_access_after_only/.oxlintrc.json |
Removed the configuration file for the defunct test |
apps/oxlint/test/fixtures/sourceCode_late_access_after_only/files/1.js |
Removed first test input file |
apps/oxlint/test/fixtures/sourceCode_late_access_after_only/files/2.js |
Removed second test input file |
apps/oxlint/test/fixtures/sourceCode_late_access_after_only/output.snap.md |
Removed expected output snapshot |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Merge activity
|
This test case is defunct since #14401. It doesn't test what it's meant to. From the comment in the plugin used in this test fixture: ```js // Purpose of this test fixture is to ensure that source text and AST are available in `after` hook // via `context.sourceCode` when the AST is not traversed ``` But the visitor returned by `createOnce` contains a visit function for `Program`, so the AST *is* traversed. On the other hand, if we remove that `Program` visit method, then it's an empty visitor, so the AST won't be traversed, but the `after` hook won't run either. Therefore, the case which this fixture is designed to test is impossible. Delete the fixture.
graphite-app
bot
force-pushed
the
12-13-test_linter_plugins_remove_defunct_test_case
branch
from
be35cb1 to
7277d4c
Compare
graphite-app
bot
deleted the
12-13-test_linter_plugins_remove_defunct_test_case
branch
1 participant
This test case is defunct since #14401. It doesn't test what it's meant to.
From the comment in the plugin used in this test fixture:
But the visitor returned by
createOncecontains a visit function forProgram, so the AST is traversed. On the other hand, if we remove thatProgramvisit method, then it's an empty visitor, so the AST won't be traversed, but theafterhook won't run either.Therefore, the case which this fixture is designed to test is impossible. Delete the fixture.