test: port conformance tests for kin-openapi issues 201, 639, 707#3
test: port conformance tests for kin-openapi issues 201, 639, 707#3
Conversation
- 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>
📝 WalkthroughWalkthroughThree 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
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
t/conformance/test_issue201.lua (1)
47-48: Include compiler error details in the compile assertionOn 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 assertedThese 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 caseCoverage 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
📒 Files selected for processing (3)
t/conformance/test_issue201.luat/conformance/test_issue639.luat/conformance/test_issue707.lua
Port conformance tests for three kin-openapi issues:
/users/{id}, path params extracted correctlySummary by CodeRabbit