Skip to content

(janitor/test-prune): Remove tautological/duplicate tests in utils.test.ts - #5492

Merged
pandemicsyn merged 1 commit into
mainfrom
janitor/prune-utils-test-noise
Aug 25, 2026
Merged

(janitor/test-prune): Remove tautological/duplicate tests in utils.test.ts#5492
pandemicsyn merged 1 commit into
mainfrom
janitor/prune-utils-test-noise

Conversation

@kilo-code-bot

@kilo-code-bot kilo-code-bot Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Summary

Removes three low-value tests from apps/web/src/lib/utils.test.ts that
cannot detect a plausible production regression and add ongoing
maintenance cost without protecting any additional behavior. No
production code changed.

Removed tests and surviving coverage

1. assertNotNullish — "should properly narrow types (compile-time test)"

  • Claimed to test: TypeScript type narrowing after assertNotNullish.
  • Why it's safe to remove: The test's own name/comment says it's a
    "compile-time test." If the type declaration on assertNotNullish
    were wrong, tsc/pnpm typecheck would fail — not this test. At
    runtime, the test only calls assertNotNullish with non-null values
    (no-throw path) and then calls built-in String.toUpperCase/
    Number.toFixed, which exercise no additional branch of
    assertNotNullish.
  • Surviving coverage: should not throw when value is not null or undefined (non-null/undefined inputs, no-throw path) and should throw a custom error message when provided (null/undefined inputs,
    throw path and custom message) in the same describe('assertNotNull')
    block fully cover the function's only two runtime branches.
  • Regression this could not detect: None beyond what the two
    neighboring tests already detect — a broken narrowing type
    declaration is caught by typecheck, not by this test.

2. toNonNullish — "should properly narrow return type (compile-time test)"

  • Same pattern as (1), for the sibling function toNonNullish (which
    simply calls assertNotNullish and returns the value).
  • Surviving coverage: should return the value when it is not null or undefined and should throw an error when value is null or undefined in describe('requireNotNull') cover both runtime
    branches (return value, throw with default/custom message).

3. parseResultJsonWithZodSchema — "should preserve console.log behavior on JSON parse error"

  • Claimed to test: Console logging behavior on a JSON parse
    failure — but the test body never references console.log or
    spies on it.
  • Why it's safe to remove: It is an exact duplicate of the test
    immediately above it — same createMockResponse(400, 'Bad Request', null, true) setup, same rejects.toThrow('Failed to fetch data: Bad Request') assertion — minus that test's jsonMock call-count
    check. It tests strictly less than its neighbor while claiming to
    test something (console logging) that isn't asserted at all.
  • Surviving coverage: should handle error response when JSON parsing fails (immediately preceding test) exercises the identical
    code path with an equal-or-stronger assertion (adds the
    jsonMock call-count check).
  • Regression this could not detect: None — any regression in the
    JSON-parse-failure path is already caught by the preceding test with
    an equal or stronger assertion.

Verification

  • apps/web tests require a reachable PostgreSQL instance via
    setupFilesAfterEnv (workerSetup.ts), which is unavailable in this
    sandbox (no docker/postgres). Ran a full repo-wide tsc --noEmit
    attempt, but it exceeded the sandbox's memory/time limits and could
    not complete.
  • Ran oxlint --config .oxlintrc.json apps/web/src/lib/utils.test.ts
    directly on the changed file: 0 warnings, 0 errors.
  • Ran oxfmt on the changed file (repo formatter) — no formatting
    drift beyond the deletions.
  • Manually re-read apps/web/src/lib/utils.ts (assertNotNullish,
    toNonNullish, parseResultJsonWithZodSchema) to confirm every
    runtime branch removed tests exercised remains covered by a
    surviving test with an equal or stronger assertion.
  • git diff --check — no whitespace issues.

No production code, other test files, or unrelated tests were touched.

- Remove the two 'compile-time test' cases for assertNotNullish and
  toNonNullish. Both self-describe as TypeScript narrowing checks; any
  regression there would be caught by tsc/typecheck, not by these
  runtime assertions. The runtime throw/no-throw behavior they exercise
  is already fully covered by the adjacent tests in the same describe
  blocks.
- Remove 'should preserve console.log behavior on JSON parse error' in
  parseResultJsonWithZodSchema tests. It never asserts anything about
  console.log, and duplicates the exact setup/execution/assertion of
  'should handle error response when JSON parsing fails' one test
  above it (same createMockResponse(400, 'Bad Request', null, true)
  input, same rejects.toThrow assertion).
@kilo-code-bot kilo-code-bot Bot added the janitor Automated dead-code/duplication cleanup label Aug 25, 2026
@pandemicsyn pandemicsyn changed the title (janitor/test-prune) Remove tautological/duplicate tests in utils.test.ts (janitor/test-prune): Remove tautological/duplicate tests in utils.test.ts Aug 25, 2026
@pandemicsyn
pandemicsyn merged commit 4485174 into main Aug 25, 2026
12 checks passed
@pandemicsyn
pandemicsyn deleted the janitor/prune-utils-test-noise branch August 25, 2026 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

janitor Automated dead-code/duplication cleanup

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant