Skip to content

test: discover development routes in the shared schema suite - #2248

Merged
epipav merged 4 commits into
mainfrom
test/IN-1349-schema-suite-discovers-routes
Sep 21, 2026
Merged

epipav merged 4 commits into
mainfrom
test/IN-1349-schema-suite-discovers-routes

Conversation

@epipav

@epipav epipav commented Sep 21, 2026

Copy link
Copy Markdown
Collaborator

Why

The cross-route suite in api/tests/common-schemas.test.ts ran off literal route lists (seriesRoutes, rangeRoutes, and an integer-summary table). A new endpoint that nobody added to those lists silently skipped the shared granularity wording check, the inclusive-start / exclusive-end bounds check, and the unit check. None of the five batch-2 endpoints (#2247, #2245, #2244, #2243, #2246) is in the lists. Raised by @themarolt on #2243. Adding each route in its own PR would put five edits on the same lines of one shared test, the conflict pattern #2241 removed for route registration.

What

The suite now reads the development routes from the served /v1-alpha/openapi.json:

  • every development route must take the common range and document the inclusive start and exclusive end
  • every route with a granularity parameter must mark it required, use the shared enum, and carry Granularity.description
  • every response property shaped like a period summary (current, previous, percentageChange, changeValue), whatever its name, must state a unit on the three values and keep percentageChange a nullable number. Integer counts must say (count ...); numbers must say (seconds), (percent) or (percentage points). The unit may be followed by a sentence, so nullable notes (merge-lead-time) pass.
  • a guard asserts at least five routes, at least one series route and at least one summary were discovered

The module side is already pinned by tests/v1-alpha-autoload.test.ts (filename = path leaf), so between the two, a development module cannot be served without being checked.

Verification

  • On main: 61 tests in the file, full api suite green, lint, tsc-check, format:check green.
  • Against each open batch-2 branch (test file copied in, run, restored): merge-lead-time 67, review-time-by-pr-size 62, code-review-participants 63, review-comments 64, code-reviews 64, all green. So no open PR goes red when this merges, and each gets the checks it was missing.

Jira: IN-1349

@epipav
epipav requested review from gaspergrom and themarolt and a balanced review from Copilot September 21, 2026 19:22
@epipav epipav self-assigned this Sep 21, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Referenced OpenAPI schemas can bypass the new summary validation.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
What changed in this PR

Updates shared schema tests to automatically cover all development routes exposed by the v1-alpha OpenAPI specification.

Changes:

  • Discovers development, series, and summary routes dynamically.
  • Validates shared ranges, granularity, units, and summary types.
File Description
api/​tests/​common-schemas.test.ts Replaces static route lists with OpenAPI-based discovery and validation.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread api/tests/common-schemas.test.ts Outdated
Copilot AI review requested due to automatic review settings September 21, 2026 19:39

@themarolt themarolt left a comment

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.

lgtm - one question on moving the spec fetch to module scope

Comment thread api/tests/common-schemas.test.ts Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

The generalized assertions can still accept inconsistent units and invalid shared query schemas.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
Resolved since last review (1)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Validate discovered route query shape against DateRangeQuery

api/​tests/​common-schemas.test.ts:481

This verifies the date wording but not the shared query shape, so a discovered route can omit repos or make a date required or non-date while still passing. Assert that repos, startDate, and endDate are present, optional, and use the types and formats from DateRangeQuery.

Comment thread api/tests/common-schemas.test.ts Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

The route discovery and numeric-unit checks contain mismatches that can reject valid future endpoints.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 2 Medium severity

Open (2)
Resolved since last review (1)

Comment thread api/tests/common-schemas.test.ts
Comment thread api/tests/common-schemas.test.ts

@themarolt themarolt left a comment

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.

lgtm - the status guard and the $ref resolve cover my earlier point on the module-scope fetch

Copilot AI review requested due to automatic review settings September 21, 2026 21:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

The refactor closes the route coverage gap without introducing unresolved issues.

Review effort: Balanced
Findings: None

Resolved since last review (2)

@epipav
epipav merged commit 5c3dc17 into main Sep 21, 2026
11 checks passed
@epipav
epipav deleted the test/IN-1349-schema-suite-discovers-routes branch September 21, 2026 21:53
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.

3 participants