Skip to content

test: port conformance tests for kin-openapi issues 201, 639, 707#3

Merged
nic-6443 merged 1 commit intomainfrom
port-issue-tests
Apr 21, 2026
Merged

test: port conformance tests for kin-openapi issues 201, 639, 707#3
nic-6443 merged 1 commit intomainfrom
port-issue-tests

Conversation

@jarvis9443
Copy link
Copy Markdown
Contributor

@jarvis9443 jarvis9443 commented Apr 21, 2026

Port conformance tests for three kin-openapi issues:

  • issue 201: duplicate path template with different HTTP methods — validates GET and POST both work on /users/{id}, path params extracted correctly
  • issue 639: request body decode edge cases — empty objects, optional body, additional properties, nested object validation
  • issue 707: path parameter edge cases — string vs integer type coercion, min/max constraints, multiple path params

Summary by CodeRabbit

  • Tests
    • Added conformance tests for OpenAPI 3.0 specification validation, including path parameter types and constraints
    • Expanded test coverage for request body validation with required and optional fields
    • Added test cases for error handling of invalid request formats and missing required data

- issue201: duplicate path template with different methods
- issue639: request body decode edge cases (empty objects, optional body,
  additional properties, nested validation)
- issue707: path parameter edge cases (type coercion, min/max constraints,
  multiple params)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 21, 2026

📝 Walkthrough

Walkthrough

Three new Lua conformance test files are added to the test suite, validating OpenAPI 3.0 specification compilation and request validation behavior across path parameters, request bodies, and multiple HTTP operations.

Changes

Cohort / File(s) Summary
Conformance Tests
t/conformance/test_issue201.lua, t/conformance/test_issue639.lua, t/conformance/test_issue707.lua
Added three test files that define OpenAPI 3.0 specifications and validate request-path and request-body validation via v:validate_request. Tests cover path parameter type validation, request body schema enforcement, and handling of required/optional fields across different endpoint configurations.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
E2e Test Quality Review ⚠️ Warning PR fails to explicitly verify extracted path parameters in test_issue201.lua as claimed; validator API returns only (ok, err) without exposing extracted parameters to tests. Extend validator API to return extracted parameters as third value, then add explicit assertions like T.is(params.id, '123') in test_issue201.lua for parameter verification.
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: porting three new conformance tests for specific kin-openapi issues (201, 639, 707). It's concise, specific, and clearly conveys the primary purpose of the changeset.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Security Check ✅ Passed Three new Lua conformance test files contain no hardcoded credentials, secrets, API keys, tokens, or sensitive data; they are in-memory tests with mock OpenAPI specifications and non-sensitive test data only.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch port-issue-tests

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (3)
t/conformance/test_issue201.lua (1)

47-48: Include compiler error details in the compile assertion

On Line 48, assert(v, "compile failed") drops useful failure context. Capturing and surfacing the compile error will make conformance failures much easier to diagnose.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@t/conformance/test_issue201.lua` around lines 47 - 48, Change the compile
assertion to capture and log the compiler error returned by ov.compile: call
ov.compile(spec) into two values (e.g., v and err) instead of a single v, and
then assert v with a message that concatenates or formats the err (e.g.,
"compile failed: " .. tostring(err)) so the actual compile error is surfaced
when the assertion fails; update the ov.compile(spec) usage and the assert call
accordingly.
t/conformance/test_issue707.lua (1)

77-83: Type-coercion behavior is implied, not directly asserted

These tests confirm route acceptance, but they don’t explicitly verify coerced path-parameter values/types for integer params. Add direct assertions on parsed parameter outputs to make the coercion contract explicit and prevent regressions.

Also applies to: 127-133

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@t/conformance/test_issue707.lua` around lines 77 - 83, Update the "issue707:
integer path param with valid integer (valid)" test to explicitly assert that
path params are coerced to an integer type: call v:validate_request(...) and
capture the returned parsed result (e.g., the response/result object alongside
ok/err), then add assertions that the parsed path parameter (for "/orders/999")
exists and is a number (not a string) and equals 999; repeat the same explicit
parsed-parameter assertions for the other similar test block referenced (around
lines 127-133). Ensure you reference the same validate_request call on the v
object and the T.describe test names when adding the assertions.
t/conformance/test_issue639.lua (1)

75-147: Consider adding a malformed-JSON negative case

Coverage is strong for schema edge cases, but adding one invalid JSON payload case (decode failure path) would make this conformance port more complete for request-body decoding behavior.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@t/conformance/test_issue639.lua` around lines 75 - 147, Add a negative test
case that sends malformed JSON to exercise the decode-failure path: inside
t/conformance/test_issue639.lua add a T.describe block that calls
v:validate_request for POST /items with headers/content_type "application/json"
and a body like '{"name": "widget", ' (invalid JSON), then assert the result is
not ok with T.ok(not ok, "should fail - malformed JSON") to ensure
v:validate_request's JSON decode error path is covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@t/conformance/test_issue201.lua`:
- Around line 50-86: The tests call v:validate_request but only assert
pass/fail; update the tests for the GET and POST cases to also assert the
extracted path parameter id is correct by reading the returned params from
v:validate_request (e.g., capture the returned table/value alongside ok,err) in
the "issue201: GET /users/123 (valid)" and "issue201: POST /users/123 with valid
body (valid)" cases and add assertions that params.id == "123" (and likewise
assert params.id == "abc" in the "GET /users/abc string id (valid)" test) to
ensure path parameter extraction is validated.

---

Nitpick comments:
In `@t/conformance/test_issue201.lua`:
- Around line 47-48: Change the compile assertion to capture and log the
compiler error returned by ov.compile: call ov.compile(spec) into two values
(e.g., v and err) instead of a single v, and then assert v with a message that
concatenates or formats the err (e.g., "compile failed: " .. tostring(err)) so
the actual compile error is surfaced when the assertion fails; update the
ov.compile(spec) usage and the assert call accordingly.

In `@t/conformance/test_issue639.lua`:
- Around line 75-147: Add a negative test case that sends malformed JSON to
exercise the decode-failure path: inside t/conformance/test_issue639.lua add a
T.describe block that calls v:validate_request for POST /items with
headers/content_type "application/json" and a body like '{"name": "widget", '
(invalid JSON), then assert the result is not ok with T.ok(not ok, "should fail
- malformed JSON") to ensure v:validate_request's JSON decode error path is
covered.

In `@t/conformance/test_issue707.lua`:
- Around line 77-83: Update the "issue707: integer path param with valid integer
(valid)" test to explicitly assert that path params are coerced to an integer
type: call v:validate_request(...) and capture the returned parsed result (e.g.,
the response/result object alongside ok/err), then add assertions that the
parsed path parameter (for "/orders/999") exists and is a number (not a string)
and equals 999; repeat the same explicit parsed-parameter assertions for the
other similar test block referenced (around lines 127-133). Ensure you reference
the same validate_request call on the v object and the T.describe test names
when adding the assertions.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c8ba215b-c173-4420-9a63-b210cf9b379a

📥 Commits

Reviewing files that changed from the base of the PR and between 04a4a1f and 27907f2.

📒 Files selected for processing (3)
  • t/conformance/test_issue201.lua
  • t/conformance/test_issue639.lua
  • t/conformance/test_issue707.lua

Comment thread t/conformance/test_issue201.lua
@nic-6443 nic-6443 merged commit 1452e0e into main Apr 21, 2026
3 checks passed
@nic-6443 nic-6443 deleted the port-issue-tests branch April 21, 2026 10:27
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