Skip to content

Comments

Add validation utility tests and truncate parameter validation#406

Merged
nikblanchet merged 2 commits intomainfrom
bugfix/shortuuid
Nov 28, 2025
Merged

Add validation utility tests and truncate parameter validation#406
nikblanchet merged 2 commits intomainfrom
bugfix/shortuuid

Conversation

@nikblanchet
Copy link
Owner

@nikblanchet nikblanchet commented Nov 28, 2025

Summary

Addresses findings from post-merge code review of shortuuid implementation (PR #404).

  • Add comprehensive test coverage for 5 validation utility functions that previously had zero tests
  • Add input validation for truncate parameter
  • Add documentation comment for BigInt edge case

Changes

Tests Added (cli/src/__tests__/unit/validation.test.ts)

Function Test Cases
isValidShortUuid() Valid 22-char, with hyphens, zero UUID encoding, too short, too long, invalid chars (0, 1, I, O, l), empty
isValidSessionId() Standard UUIDs, shortuuids, hyphenated shortuuids, invalid formats, empty
detectSessionIdFormat() UUID detection, shortuuid detection, hyphenated shortuuid, invalid input
normalizeSessionId() UUID preservation, shortuuid passthrough, hyphen stripping
formatSessionIdForDisplay() UUID truncation, shortuuid truncation with hyphens, already-hyphenated input, full format

Code Improvements (cli/src/utils/shortuuid.ts)

  • Add truncate parameter validation (throws if negative or non-integer)
  • Add edge case comment explaining BigInt 0n behavior

Skipped

  • Python Optional[int] change: The existing int | None syntax is the modern Python 3.10+ convention (PEP 604) and is correct
  • Map-based alphabet lookup: Micro-optimization, not critical

Test Plan

  • All 61 validation/shortuuid tests pass
  • All 1018 CLI tests pass
  • All 21 Python shortuuid tests pass
  • ESLint passes
  • Ruff passes
  • Prettier formatting applied

Generated with Claude Code

@nikblanchet
Copy link
Owner Author

Code Review: PR #406

Approved - Comprehensive validation test coverage successfully addresses post-merge findings from PR #404.

See detailed review: .scratch/code-review-pr-406-2025-11-28T15-00-48-08-00.md

Key Findings

Strengths:

  • Excellent test coverage: 26 new test cases across 5 validation functions
  • Proper truncate parameter validation with clear error messages
  • Well-documented BigInt edge case (zero UUID encoding)
  • Comprehensive alphabet validation (all 5 excluded chars tested)
  • Clean test organization matching project conventions

Minor improvement opportunity:

  • Consider adding explicit tests for truncate validation error cases (negative/non-integer values) to improve test discoverability

Enhancement ideas (low priority):

  • Add edge case tests (truncate: 0, NaN, Infinity)
  • Extract magic numbers to constants (ZERO_UUID_ENCODING, BASE57_EXCLUDED_CHARS)
  • Add JSDoc example for full-length formatting (truncate >= 22)

Test Results

  • Validation tests: 38/38 passed
  • Full CLI suite: 1017/1018 passed (1 unrelated failure)
  • Python tests: 21/21 passed
  • Linting: ESLint and Ruff pass

Compliance

All requirements met:

  • Validation utility tests added for 5 functions
  • Truncate parameter validation implemented
  • BigInt edge case documented
  • Scope correctly limited (skipped non-critical changes as noted)

Good work on the comprehensive test coverage and proper input validation!

nikblanchet added a commit that referenced this pull request Nov 28, 2025
- Add truncate validation error tests (negative, non-integer, NaN, Infinity)
- Add truncate edge case tests (0 returns empty, large value returns full)
- Extract magic numbers to constants (ZERO_UUID_ENCODING, BASE57_EXCLUDED_CHARS)
- Add JSDoc example for truncate >= 22 (full formatted output)

Addresses code review findings from PR #406 first review.

Generated with [Claude Code](https://claude.com/claude-code)
Steered and verified by @nikblanchet
- Add comprehensive tests for 5 validation functions:
  - isValidShortUuid(): valid/invalid shortuuids, hyphenated input
  - isValidSessionId(): UUID, shortuuid, and hybrid validation
  - detectSessionIdFormat(): format detection tests
  - normalizeSessionId(): UUID preservation, hyphen stripping
  - formatSessionIdForDisplay(): truncation and hyphen formatting

- Add truncate parameter validation in formatDisplay():
  - Throws if truncate is negative or not an integer
  - Protects against invalid input values

- Add BigInt edge case comment explaining 0n behavior

Addresses post-merge review findings from PR #404.

Generated with [Claude Code](https://claude.com/claude-code)
Steered and verified by @nikblanchet
- Add truncate validation error tests (negative, non-integer, NaN, Infinity)
- Add truncate edge case tests (0 returns empty, large value returns full)
- Extract magic numbers to constants (ZERO_UUID_ENCODING, BASE57_EXCLUDED_CHARS)
- Add JSDoc example for truncate >= 22 (full formatted output)

Addresses code review findings from PR #406 first review.

Generated with [Claude Code](https://claude.com/claude-code)
Steered and verified by @nikblanchet
@nikblanchet nikblanchet merged commit 46ab56b into main Nov 28, 2025
6 checks passed
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