fix: Stop importing the generated test-utils barrel from two wrappers - #4949
Merged
gethinwebster merged 2 commits intoSep 3, 2026
Merged
Conversation
TrevorBurnham
marked this pull request as ready for review
August 30, 2026 15:30
TrevorBurnham
requested review from
gethinwebster
and removed request for
a team
August 30, 2026 15:30
gethinwebster
previously approved these changes
Aug 31, 2026
SpyZzey
force-pushed
the
fix-test-utils-barrel-imports
branch
from
August 31, 2026 13:19
73eedb5 to
834d76b
Compare
pan-kot
approved these changes
Sep 1, 2026
gethinwebster
enabled auto-merge
September 1, 2026 11:58
SpyZzey
force-pushed
the
fix-test-utils-barrel-imports
branch
from
September 2, 2026 09:37
5db3603 to
98dba76
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4949 +/- ##
==========================================
- Coverage 97.66% 93.76% -3.91%
==========================================
Files 959 958 -1
Lines 31342 27136 -4206
Branches 11577 9761 -1816
==========================================
- Hits 30611 25443 -5168
+ Misses 724 613 -111
- Partials 7 1080 +1073 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
`src/test-utils/dom/index.ts` is generated and gitignored, and it imports every component wrapper, so a wrapper importing it back is circular. `ban-files` already bans it, and these two files are the only importers left. They escaped the ban by writing `'../index.js'` rather than `'../index'`. The rule resolves a specifier against the files on disk and only probed extensions appended to the literal path, so `src/test-utils/dom/index.js` and `index.js.ts` never matched the banned `src/test-utils/dom/index.ts`. `@cloudscape-design/test-utils-core` exports `createWrapper` from both its `/dom` and `/selectors` entry points, with the same implementation the generated barrel copies, so this is behaviour-identical in both variants. Importing it from `test-utils-core/dom` also lets the selectors converter rewrite the path, which drops the barrel import from the generated selectors output as well. 547 test-utils tests pass with 3 unchanged snapshots, plus 165 popover and button-group tests. Lint is unchanged at 0 errors.
SpyZzey
force-pushed
the
fix-test-utils-barrel-imports
branch
from
September 3, 2026 09:10
98dba76 to
e005fcd
Compare
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.
While working on #4948, I noticed that the ban-files rule defined in the build-tools repo has a bug that's causing it to miss two existing violations in this repo. This PR fixes those violations, before I fix the rule itself.
The violation
src/test-utils/dom/index.tsimports every component wrapper. A wrapper importing it back is therefore circular, which is whatban-filesis configured to prevent. The twoimport createWrapper from '../index.js'lines removed by this PR should have triggered a rule violation like this:The reason they didn't trigger it is that they include the
.jsextension, whichban-filesdoesn't expect. The rule only probed extensions appended to the literal path, so it testedsrc/test-utils/dom/index.jsandsrc/test-utils/dom/index.js.tsrather than matching the bannedsrc/test-utils/dom/index.ts.The fix
@cloudscape-design/test-utils-corealready exportscreateWrapperfrom both its/domand/selectorsentry points, and its implementation is the same one the generated barrel copies:So swapping the import is behaviour-identical, and the call sites are untouched. Importing from
test-utils-core/domalso means the selectors converter rewrites the path to/selectorsfor free, so the generated selectors output stops importing its barrel too.