Automated Testing: Restrict dirname globals in test files - #80275
Conversation
aduth
added
the
[Type] Automated Testing
github-actions
Bot
added
[Package] Theme
|
Size Change: 0 B Total Size: 7.72 MB |
| files: [ '**/@(__tests__|test)/**/*.[tj]s?(x)' ], | ||
| // Jest harness under test/unit is plain CommonJS loaded by Node/Jest | ||
| // directly (not Babel-transformed), so import.meta.* is unavailable. | ||
| ignores: [ 'test/unit/**' ], |
There was a problem hiding this comment.
this override also matches test/e2e/** and test/performance/**, which do not use the babel-plugin-transform-import-meta transformer added in #79362
Playwright transpiles these files as CommonJS and every end-to-end shard now stops while loading test/e2e/playwright.config.ts, and the performance job stops on its config.
Could we exclude the Playwright trees from this rule and restore their __dirname / __filename usages until that runtime has an equivalent supported transform?
Alternatively, the Playwright loader would need to be made genuinely ESM-compatible before enforcing import.meta there. Fixing only the two config files would not be enough because the converted specs and setup module use the same CommonJS/import.meta mix.
| // globals configuration, but `no-undef` is disabled for TypeScript files, | ||
| // and TypeScript doesn't provide a way to disable the CommonJS globals. | ||
| { | ||
| files: [ '**/@(__tests__|test)/**/*.[tj]s?(x)' ], |
There was a problem hiding this comment.
This pattern may not cover all tests.
For example, tools/validation/validate-package-contents.test.js is collected and Babel-transformed by the root Jest config, still uses __dirname, and does not match this glob.
Maybe we should reuse the same glob used normally by Jest (*.test.* / *.spec.*) in addition to the directory patterns, while keeping the runtime-specific ignores? That would make the enforcement match the PR's stated scope.
There was a problem hiding this comment.
Hm, this one was inspired by an existing one in use already in developmentFiles. We have quite a few different patterns in this file, and it sounds like coverage is mixed. We should probably consolidate and find a pattern (like the one you suggest) that aligns well to what the tests actually execute.
gutenberg/tools/eslint/config.mjs
Line 72 in 60b366f
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
2 participants
What?
Updates ESLint rules to restrict the availability of
__dirnameand__filenameglobals in test files, and fixes existing issues.This is a continuation from #79362. #79362 made
import.metavalues available in these files. This pull request enforces their usage.Why?
See rationale in #79362, specifically:
How?
Uses
no-restricted-globalsESLint rule to forbid use of these globals. As explained in the inline code comment, I'd rather have done this by preventing those globals from being available in the first place rather than after-the-fact, and we could do this in ESLint through how globals are specified, but globals enforcement throughno-undefis disabled through use of recommendedtypescript-eslintconfiguration (source).Updates are straightforward replacement of
__dirname➡️import.meta.dirnameand__filename➡️import.meta.filename.Testing Instructions
npm run lint:jsshould pass.For bonus points, try adding
__dirnameor__filenameto a test file and verify thatnpm run lint:jsfails.Use of AI Tools
Used Cursor IDE + Auto (likely Composer) model to research and implement, though mostly on the side of updating files.