Skip to content

test(config): add unit tests for config utility modules#314

Merged
zbigniewsobiecki merged 1 commit intodevfrom
test/config-utility-modules
Feb 16, 2026
Merged

test(config): add unit tests for config utility modules#314
zbigniewsobiecki merged 1 commit intodevfrom
test/config-utility-modules

Conversation

@zbigniewsobiecki
Copy link
Copy Markdown
Member

Summary

  • Add comprehensive unit tests for 6 config utility modules
  • Achieve 100% coverage for pure config functions
  • All tests pass, lint clean, type checks clean

Changes

New Test Files

  • rateLimits.test.ts — Tests for getRateLimitForModel() with exact match, prefix match, and fallback behavior
  • retryConfig.test.ts — Tests for getRetryConfig() structure, shouldRetry() logic with retryable/stream/non-retryable errors, and callback logging
  • compactionConfig.test.ts — Tests for getCompactionConfig() with implementation vs. other agent thresholds, and onCompaction() callback that clears read tracking
  • statusUpdateConfig.test.ts — Tests for getStatusUpdateConfig() (debug agent disabled), formatStatusMessage() progress bar rendering, and task counts
  • reviewConfig.test.ts — Tests for estimateTokens() correctness and REVIEW_FILE_CONTENT_TOKEN_LIMIT constant
  • customModels.test.ts — Tests for CUSTOM_MODELS array structure, pricing, features, and model specifications

Test Coverage

  • 116 tests total across 6 files
  • Tests cover all acceptance criteria:
    • Rate limit model matching (exact, prefix, disabled fallback)
    • Retry config structure and shouldRetry logic for different error types
    • Compaction config thresholds and onCompaction callback
    • Status update formatting and progress bars
    • Token estimation and review config constants
    • Custom model specifications and validation

Test Patterns

  • Followed existing hintConfig.test.ts patterns
  • Used Vitest mocking for dependencies (logger, todo storage)
  • Used Object.assign() instead of as any to avoid lint warnings
  • Comprehensive edge case coverage (0%, 50%, 100% progress, etc.)

Closes https://trello.com/c/69938031828499e77dfe74ad

@aaight
Copy link
Copy Markdown
Collaborator

aaight commented Feb 16, 2026

🔍 Reviewing PR...

Copy link
Copy Markdown
Collaborator

@aaight aaight left a comment

Choose a reason for hiding this comment

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

Summary

LGTM — well-structured unit tests for 6 config utility modules. Tests are correct, follow existing patterns (hintConfig.test.ts), and CI is green.

Code Issues

Should Fix

  • tests/unit/config/customModels.test.ts:79 — The test "all models have knowledge cutoff date" uses a conditional if (model.knowledgeCutoff) that only validates the format when the field exists, but doesn't assert every model actually has the field. The test name implies it verifies presence, but it would silently pass for models missing knowledgeCutoff. Consider using expect(model.knowledgeCutoff).toBeDefined() before the regex check — or rename the test to "knowledge cutoff dates have valid format" to match what it actually tests.

Everything else looks solid: mock patterns are correct, edge cases (0%/50%/100% progress bars, rounding, empty strings) are well-covered, Object.assign usage for adding status codes to errors is clean, and the tests accurately reflect the source implementations.

expect(typeof model.features.functionCalling).toBe('boolean');
}
});
});
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The conditional if (model.knowledgeCutoff) means this test silently passes for models without the field — the test name ("all models have knowledge cutoff date") implies it verifies presence. Either add expect(model.knowledgeCutoff).toBeDefined() before the regex, or rename to "knowledge cutoff dates have valid format" to match the actual behavior.

@zbigniewsobiecki zbigniewsobiecki merged commit 78306ed into dev Feb 16, 2026
4 checks passed
@zbigniewsobiecki zbigniewsobiecki deleted the test/config-utility-modules branch March 16, 2026 16:43
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.

2 participants