Skip to content

[Issue #353] Expand CLI check spec unit tests#507

Open
bryan-thompsoncodes wants to merge 4 commits intoHHS:mainfrom
bryan-thompsoncodes:chore/353-expand-cli-unit-tests
Open

[Issue #353] Expand CLI check spec unit tests#507
bryan-thompsoncodes wants to merge 4 commits intoHHS:mainfrom
bryan-thompsoncodes:chore/353-expand-cli-unit-tests

Conversation

@bryan-thompsoncodes
Copy link

@bryan-thompsoncodes bryan-thompsoncodes commented Feb 13, 2026

Summary

Changes proposed

Added 10 new unit tests covering validation scenarios documented in the check command README that were previously untested:

check-schema-compatibility.test.ts (5 tests):

  • Simple type match → no error
  • Enum values match exactly → no error
  • Base has extra enum values, impl is subset → no error
  • Missing optional property → no error
  • Extra properties when additionalProperties is not set → flags error

check-matching-routes.test.ts (5 tests):

  • Extra status codes in implementation → ignored
  • Extra MIME types in implementation response → ignored
  • Missing MIME type in implementation response → error
  • Extra query parameters in implementation → not flagged
  • Missing optional query parameter → flagged

Context for reviewers

This PR only adds tests — no source code was changed. All 119 tests pass (up from 109).
The tests were identified by auditing the validation cases in lib/cli/src/commands/check/README.md against the existing test suite to find gaps. Each new test follows the existing Arrange/Act/Assert pattern and uses the // ############ section header convention already established in the test files.

Behavioral notes discovered during testing:

A few tests document cases where the current implementation diverges from the README spec. These are worth noting for future work:

README says Code does Test documents
Missing optional prop → Warn Silently ignored (0 errors) should not error when missing property is optional
Missing optional query param → Warn Errors with "Missing required" message should flag missing optional query parameter
Extra query params → Warn Not detected at all should not flag extra query parameters
additionalProperties undefined → allow extras Flags as EXTRA_FIELD error should allow extra properties when additionalProperties is not set

These may warrant follow-up issues for aligning the implementation with the README spec.

Verification:

npm test (from lib/cli)
Test Suites: 18 passed, 18 total
Tests: 119 passed, 119 total

Additional information

No screenshots needed — test-only change. The coverage report remains stable at 88% statement coverage overall.

Add 10 new tests covering validation scenarios documented in the
check command README that were previously untested:

- Schema: simple type match, enum match, base extra enums, missing optional prop, default additionalProperties
- Routes: extra status codes, extra/missing mime types, extra/missing optional query params
Rename tests that document behavior diverging from the README spec to
explicitly note the deviation, and add TODO comments linking to the
specific README cases so the tests get updated when the impl is fixed.
@bryan-thompsoncodes bryan-thompsoncodes changed the title Expand CLI check spec unit tests (#353) [Issue#353] Expand CLI check spec unit tests Feb 13, 2026
@bryan-thompsoncodes bryan-thompsoncodes changed the title [Issue#353] Expand CLI check spec unit tests [Issue #353] Expand CLI check spec unit tests Feb 13, 2026
@jcrichlake
Copy link
Collaborator

@bryan-thompsoncodes looks like some of the CI checks failed, could you run those locally and push up any changes? I'll look into the PR: Labeler

@bryan-thompsoncodes
Copy link
Author

@jcrichlake absolutely, looks like I missed a prettier cleanup. Showing all green locally now

Needs the pull_request_target: trigger to access the right permissions
For more details see: actions/labeler#399
@widal001
Copy link
Collaborator

Since the remaining failure is an pnpm audit flag that will be addressed in #505 if we're okay hanging tight until after we merge that PR and either rebasing onto main or merging main into this branch, that should resolve that outstanding issue.

Similarly I updated the PR labeler in this commit: ci: Updates event trigger for PR labeler

So it's no longer failing, but unfortunately won't run until that commit is merged into main for the reasons described here (TL;DR is that pull_request_target checks out the code on main instead of the branch so it's still referencing the old file)

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.

[CLI] Expand CLI unit tests

3 participants