Skip to content

chore(tests): consolidate unit tests subsumed by property tests#422

Merged
BYK merged 1 commit intomainfrom
chore/consolidate-unit-property-tests
Mar 13, 2026
Merged

chore(tests): consolidate unit tests subsumed by property tests#422
BYK merged 1 commit intomainfrom
chore/consolidate-unit-property-tests

Conversation

@BYK
Copy link
Member

@BYK BYK commented Mar 13, 2026

Remove ~620 lines of unit tests across 6 files where the same invariants
are already covered by companion property-based tests. This is a follow-up to
#418 which removed standalone low-value tests.

What changed

Each unit test file was audited against its .property.test.ts counterpart.
Tests that verify the same invariant the property tests generate random inputs
for were removed. Tests that remain fall into categories the property tests
don't cover:

  • Edge cases outside property generators (self-hosted DSNs, trailing
    slashes, whitespace/empty strings, null intermediates in dot-paths)
  • Specific output documentation (column titles, exact format strings,
    rendered vs plain mode)
  • Concurrency/timing tests (parallel execution, single-resolution)
  • Integration tests (writeJson/writeJsonList APIs, fixture-based patching)
  • Functions only in unit files (isAuthenticated, getActiveEnvVarName)

Files trimmed

File Lines removed
test/lib/dsn.test.ts -205
test/lib/formatters/json.test.ts -136
test/lib/db/auth.test.ts -89
test/lib/bspatch.test.ts -86
test/lib/promises.test.ts -73
test/lib/formatters/trace.test.ts -31

@github-actions
Copy link
Contributor

github-actions bot commented Mar 13, 2026

Semver Impact of This PR

🟢 Patch (bug fixes)

📋 Changelog Preview

This is how your changes will appear in the changelog.
Entries from this PR are highlighted with a left border (blockquote style).


New Features ✨

Init

  • Add --team flag to relay team selection to project creation by MathurAditya724 in #403
  • Enforce canonical feature display order by betegon in #388
  • Accept multiple delimiter formats for --features flag by betegon in #386
  • Add git safety checks before wizard modifies files by betegon in #379
  • Add experimental warning before wizard runs by betegon in #378
  • Add init command for guided Sentry project setup by betegon in #283

Issue List

  • Auto-compact when table exceeds terminal height by BYK in #395
  • Redesign table to match Sentry web UI by BYK in #372

Other

  • (auth) Allow re-authentication without manual logout by BYK in #417
  • (trial) Auto-prompt for Seer trial + sentry trial list/start commands by BYK in #399
  • Support SENTRY_HOST as alias for SENTRY_URL by betegon in #409
  • Add --dry-run flag to mutating commands by BYK in #387
  • Return-based output with OutputConfig on buildCommand by BYK in #380
  • Add --fields flag for context-window-friendly JSON output by BYK in #373
  • Magic @ selectors (@latest, @most_frequent) for issue commands by BYK in #371
  • Input hardening against agent hallucinations by BYK in #370
  • Add response caching for read-only API calls by BYK in #330

Bug Fixes 🐛

Dsn

Init

  • Make URLs clickable with OSC 8 terminal hyperlinks by MathurAditya724 in #423
  • Remove implementation detail from help text by betegon in #385
  • Truncate uncommitted file list to first 5 entries by MathurAditya724 in #381

Other

  • (api) Convert --data to query params for GET requests by BYK in #383
  • (docs) Remove double borders and fix column alignment on landing page tables by betegon in #369
  • Show human-friendly names in trial list and surface plan trials by BYK in #412
  • Add trace ID validation to trace view + UUID dash-stripping by BYK in #375

Documentation 📚

  • Update credential storage docs and remove stale config.json references by betegon in #408

Internal Changes 🔧

Init

  • Remove --force flag by betegon in #377
  • Remove dead determine-pm step label by betegon in #374

Tests

  • Consolidate unit tests subsumed by property tests by BYK in #422
  • Remove redundant and low-value tests by BYK in #418

Other

  • (log/list) Convert non-follow paths to return CommandOutput by BYK in #410
  • Convert list command handlers to return data instead of writing stdout by BYK in #404
  • Split api-client.ts into focused domain modules by BYK in #405
  • Migrate non-streaming commands to CommandOutput with markdown rendering by BYK in #398
  • Convert Tier 2-3 commands to return-based output and consola by BYK in #394
  • Convert remaining Tier 1 commands to return-based output by BYK in #382
  • Converge Tier 1 commands to writeOutput helper by BYK in #376

Other

  • Minify JSON on read and pretty-print on write in init local ops by MathurAditya724 in #396

🤖 This preview updates automatically when you update the PR.

@BYK BYK marked this pull request as ready for review March 13, 2026 18:38
@github-actions
Copy link
Contributor

github-actions bot commented Mar 13, 2026

Codecov Results 📊

111 passed | Total: 111 | Pass Rate: 100% | Execution Time: 0ms

📊 Comparison with Base Branch

Metric Change
Total Tests
Passed Tests
Failed Tests
Skipped Tests

✨ No test changes detected

All tests are passing successfully.

✅ Patch coverage is 100.00%. Project has 1065 uncovered lines.
✅ Project coverage is 95.03%. Comparing base (base) to head (head).

Coverage diff
@@            Coverage Diff             @@
##          main       #PR       +/-##
==========================================
+ Coverage    95.03%    95.03%        —%
==========================================
  Files          159       159         —
  Lines        21431     21431         —
  Branches         0         0         —
==========================================
+ Hits         20366     20366         —
- Misses        1065      1065         —
- Partials         0         0         —

Generated by Codecov Action

@BYK BYK force-pushed the chore/consolidate-unit-property-tests branch from 5d1bb95 to ec96ff3 Compare March 13, 2026 18:40
@BYK BYK force-pushed the chore/consolidate-unit-property-tests branch from ec96ff3 to e86305a Compare March 13, 2026 18:46
@BYK
Copy link
Member Author

BYK commented Mar 13, 2026

Addressed both BugBot findings — good catches:

  1. Empty spans NaN duration test: Restored in trace.test.ts. The property generator's spanListArb uses minLength: 1, so the empty-array edge case for duration was indeed uncovered.

  2. modules/ inferPackagePath test: Restored in dsn.test.ts. The property generator's monorepoPathArb uses constantFrom("packages", "apps", "services", "libs") but MONOREPO_ROOTS includes "modules" — exactly the kind of edge case outside the generator's range that should stay.

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Remove ~620 lines of unit tests that duplicate invariants already
covered by companion property-based tests. Each unit test file now
has a header comment documenting which invariants are in the property
file and what remains in the unit file.

Files trimmed:
- test/lib/dsn.test.ts: Remove extractOrgIdFromHost, parseDsn (basic),
  isValidDsn, createDsnFingerprint (ordering/dedup), createDetectedDsn
  (basic) — all covered by dsn.property.test.ts. Keep: self-hosted DSN
  fingerprints, trailing slash edge case, path segments, packagePath.
- test/lib/promises.test.ts: Remove basic true/false/error tests —
  covered by promises.property.test.ts. Keep: concurrency timing tests,
  single-resolution guarantee.
- test/lib/bspatch.test.ts: Remove offtin unit tests and parsePatchHeader
  rejection tests — covered by bspatch.property.test.ts. Keep: fixture-
  based patch application, SHA-256 verification, truncated patch.
- test/lib/db/auth.test.ts: Remove priority, source tracking, refresh
  skip, isEnvTokenActive basics — covered by auth.property.test.ts.
  Keep: whitespace edge cases, isAuthenticated, getActiveEnvVarName,
  shape assertions.
- test/lib/formatters/json.test.ts: Remove filterFields basics and
  parseFieldsList dedup/whitespace — covered by json.property.test.ts.
  Keep: dot-notation edge cases, null/undefined in objects, writeJson,
  writeJsonList, formatJson APIs.
- test/lib/formatters/trace.test.ts: Remove redundant newline/contains
  checks — covered by trace.property.test.ts. Keep: specific format
  values, rendered vs plain mode, computeTraceSummary edge cases.
@BYK BYK force-pushed the chore/consolidate-unit-property-tests branch from e86305a to d6c7c74 Compare March 13, 2026 18:54
@BYK
Copy link
Member Author

BYK commented Mar 13, 2026

Addressed both round-2 BugBot findings:

  1. formatTracesHeader newline tests: Restored for both rendered and plain modes. The property test only checks formatTraceRow newlines, not formatTracesHeader. Also fixed the header comment that incorrectly claimed this was covered.

  2. Negative header values test: Restored in bspatch.test.ts. The property generator explicitly clears sign bits (patched[15] %= 128) before calling parsePatchHeader, so it never exercises the negative-length error path. Fixed the header comment too.

@BYK BYK merged commit 0671cfa into main Mar 13, 2026
22 checks passed
@BYK BYK deleted the chore/consolidate-unit-property-tests branch March 13, 2026 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant