Repository navigation
test: prune redundant utils tests and fold lookup tables - #1099
Merged
Merged
Conversation
Delete tests that cannot fail when the product breaks: - `promiseWithResolvers` tests exercised native `Promise.withResolvers` after the helper was removed in 3dec506 - `LogLevels` ordering test is implied by the exact-value test - `warns normally when suppressWarnings is not set` duplicates `info level outputs all messages` - `modules defaults to empty object` duplicates the css `defaults` test - `returns result with packageJsonPath property` had a guarded assertion; fold it into `reads package.json from directory` as a real nested lookup check Fold hand-unrolled input/output tests into `test.each` tables, keeping every row and test name. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PxVsmvB9xfZKfVnUonY8kf
✅ Deploy Preview for tsdown-main ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
tsdown
create-tsdown
@tsdown/css
@tsdown/exe
tsdown-migrate
commit: |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The test-only changes preserve existing coverage, strengthen package lookup checks, and have no identified blocking issues.
0 open findings
What changed in this PR
Simplifies tsdown’s utility and CSS tests without changing production code.
Changes:
- Removes six redundant or obsolete test cases.
- Consolidates 47 cases into nine
test.eachtables, preserving names and assertions. - Strengthens package lookup coverage with a nested-directory check.
| File | Description |
|---|---|
src/utils/package.test.ts |
Consolidates format/type cases and strengthens parent-package lookup coverage. |
src/utils/logger.test.ts |
Removes duplicate checks and parameterizes format tests. |
src/utils/json.test.ts |
Parameterizes indentation tests. |
src/utils/general.test.ts |
Consolidates utility cases and removes native Promise tests. |
src/utils/fs.test.ts |
Parameterizes ancestor and extension tests. |
packages/css/src/options.test.ts |
Removes a duplicate CSS modules default check. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
sxzz
approved these changes
Oct 14, 2026
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.
Requested by Brooklyn · project thread
AI usage
Description
Before: a few unit tests in
src/utilsandpackages/csscould not fail when the product broke. Two tested the nativePromise.withResolversafter the helper they once covered was removed, three repeated assertions a stronger neighbour already makes, and one had its only assertion behind anif. Several describe blocks were hand-unrolled input/output tables.After: those 6 cases are gone, and one of them is folded into a real nested-lookup check that catches a regression the old test missed. 47 hand-unrolled cases are now 9
test.eachtables with every row and test name kept. Only test files change.How: each candidate was defended by mutating product code with the most realistic regression only it would catch, running the whole suite, and reverting. Every deleted case has another test that fails on that mutation.
Totals
readPackageJson › returns result with packageJsonPath propertyintoreads package.json from directory).test.each: 47 rows in 9 tables; case count unchanged.Per file
src/utils/general.test.ts›promiseWithResolvers(2 cases:returns a promise that resolves,resolve is callable before awaiting)Promise.withResolvers") removed the helper fromgeneral.tsand rewrote these tests to call the built-in, so they import no product code.resolveininitBundleByPkg). The nominees pass; 5 e2e cases and issues "exports" feature produces incorrect output for multiple entry points #566 / Bug: Scoped CSS not transforming correctly; styles leaking to global scope without data-v attributes #837 fail.src/utils/logger.test.ts›LogLevels › silent < error < warn < info(1 case)has correct numeric orderingasserts the exact values 0/1/2/3, which imply the ordering.exports › no entries.has correct numeric ordering.src/utils/logger.test.ts›suppressWarnings › warns normally when suppressWarnings is not set(1 case)createLogger › info level outputs all messages(same setup; that test already assertswarnis called once).suppressWarningssuppresses everything) also failsinfo level outputs all messages,warn level outputs warnings and errors,warnOnce only warns once, bothfailOnWarnlogger tests,e2e › failOnWarnandexports › no entries.info level outputs all messagesandwarn level outputs warnings and errors.packages/css/src/options.test.ts›modules defaults to empty object(1 case)defaults, whosetoEqualalready assertsmodules: {}.?? {}→?? undefined) also failsdefaultsandcustom options are passed through.defaults.src/utils/package.test.ts›returns result with packageJsonPath property(1 case, folded)if (result).package.jsonsearch). The old test passed it.reads package.json from directorynow also reads from<testDir>/src/nestedand asserts it resolves to the parentpackage.json. Under mutation 6 the folded test fails, along with 15 e2e/shims cases.Folds (lookup tables →
test.each, every row and name kept)src/utils/general.test.tstoArraysrc/utils/general.test.tsresolveCommasrc/utils/general.test.tsslashsrc/utils/fs.test.tslowestCommonAncestorsrc/utils/fs.test.tsstripExtnamesrc/utils/json.test.tsdetectIndenttabrowsrc/utils/logger.test.tsprettyFormatsrc/utils/package.test.tsgetPackageType(non-throwing rows)src/utils/package.test.tsnormalizeFormatmodulerowThe collected test names for all 47 rows are identical before and after.
Kept after defense — do not re-nominate
No nominee survived its defense. Considered and left alone because they are not in a pruning class:
packages/migrate/tests/index.test.ts › migrates single explicitly specified dir,src/features/plugin.test.ts › plain Rolldown plugin without hooks is a no-op,src/utils/package.test.ts › reads package.json with type field,src/utils/chunks.test.ts › returns empty array for empty input, theexports.test.tsinline snapshots and thetestBuildfile snapshots (they pin specific rendered states).Case counts (Vitest collection)
src/utils/general.test.tssrc/utils/logger.test.tssrc/utils/package.test.tssrc/utils/fs.test.tssrc/utils/json.test.tspackages/css/src/options.test.ts0 failures before and after, so 0 new failures. The 4
tests/exe.test.tsskips are identical before and after.Mutations
Each was applied to product code, the whole suite was run, then the change was reverted (
git diffshowed no product files afterwards).src/features/pkg/index.ts:36,resolvereplaced by a no-op: 5 e2e cases and issues "exports" feature produces incorrect output for multiple entry points #566 / Bug: Scoped CSS not transforming correctly; styles leaking to global scope without data-v attributes #837 fail; thepromiseWithResolversnominees pass.src/utils/logger.ts:23-24, warn/info levels swapped: both LogLevels tests, 7 more logger tests andexports › no entriesfail.src/utils/logger.tscreateSuppressWarnings, with no patterns returns() => true: the nominee plus the 7 tests listed under file 3 fail.packages/css/src/options.ts:240,?? {}→?? undefined:defaults, the nominee andcustom options are passed throughfail.src/utils/package.ts:19, readsdir + '/package.json'instead of the found path: 207 cases fail, including the nominee.src/utils/package.ts:16,findPackage({ cwd: dir, last: dir })(no upward search): the foldedreads package.json from directoryand 15 e2e/shims cases fail; the old nominee passed.src/utils/package.ts:40, removedcase 'module':: only thenormalizes "module" to "es"row fails.src/utils/general.ts:34,replaceAll→replace: theslashrow anddeps.test › parseNodeModulesPath normalizes backslashes…fail.src/utils/general.ts:10, removed thedefaultValuebranch: 216 failures, including both default rows.src/utils/fs.ts:51, returns''with no extension: 2stripExtnamerows andentry.test › toObjectEntry array entryfail.src/utils/json.ts:54, tab case returnsmatch[1].length:detectIndent › tabandwriteJsonFile › preserves tab indentationfail.src/utils/fs.ts:26, single path returns the path instead of its dirname: 152 failures, including the row.src/utils/general.ts:18,split(',')→split(', '): 2resolveCommarows andcss options › css.target with comma-separated valuesfail.src/utils/logger.ts:179,'ESM'→'ES': theprettyFormates row fails.src/utils/package.ts:28,if (!pkg?.type) return 'commonjs': the 2 "returns undefined" rows fail.What ran and what did not
tsc --noEmitover the project;eslint --max-warnings 0andprettier --checkon the touched files. Dependencies were installed with pnpm 12.11.2 (thedevEnginesversion) and--frozen-lockfile.pnpm lint, the docs build, and the exe tests (skipped on this platform before and after).Excluded pull requests
Files changed by these open PRs were skipped: #1098, #1097, #1093, #1092, #1089, #1085, #1080, #1078, #1077, #1074, #1068, #1049, #920. None of the files this PR changes is touched by them.
Age check (cutoff: 30 days, 2026-09-14)
promiseWithResolvers× 2LogLevels › silent < error < warn < infowarns normally when suppressWarnings is not setmodules defaults to empty objectreturns result with packageJsonPath propertyNone of the deleted cases was added for a bug, and none is in a permission, security or entitlement suite.
Checks
tsc --noEmitclean.eslint --max-warnings 0on the touched files clean.prettier --checkon the touched files clean.To revert:
git revert 7de10074d02924a2de03a1214386c7ccc448a326.🤖 Generated with Claude Code
https://claude.ai/code/session_01PxVsmvB9xfZKfVnUonY8kf