test: port conformance tests from lua-resty-openapi-validate#1
test: port conformance tests from lua-resty-openapi-validate#1
Conversation
Port all test scenarios from lua-resty-openapi-validate (Go FFI) to ensure feature parity with lua-resty-openapi-validator (pure Lua). Tests ported (7 files, cache.t skipped as N/A): - body.t: request body validation (3.0) - body_31.t: nullable type arrays, const keyword (3.1) - header.t: header params with pattern constraints - path_params.t: path param type/range validation - path_items_31.t: components/pathItems $ref (3.1) - query_params.t: required query params with min/max - query_params_31.t: exclusiveMinimum/Maximum (3.1) All tests pass with no functional gaps found.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds new OpenAPI fixtures and corresponding Lua conformance tests exercising request validation for bodies, headers, path parameters, and query parameters across OpenAPI 3.0 and 3.1 specs. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
t/conformance/test_validate_query_params.lua (1)
31-51: Add explicit min/max failure assertions for query params.Current cases verify required and skip behavior, but not direct range violations; this leaves a gap for regressions in numeric bound checks.
Suggested additional tests
-- TEST 3: skip query params validation T.describe("query_params: skip query params", function() @@ end) +-- TEST 4: page below minimum boundary +T.describe("query_params: page below minimum", function() + local ok, err = validator:validate_request({ + method = "GET", + path = "/users", + query = { page = "0", limit = "10" }, + }) + T.ok(not ok, "page below minimum fails") + T.like(err, "page", "error mentions page") +end) + +-- TEST 5: limit above maximum boundary +T.describe("query_params: limit above maximum", function() + local ok, err = validator:validate_request({ + method = "GET", + path = "/users", + query = { page = "1", limit = "999" }, + }) + T.ok(not ok, "limit above maximum fails") + T.like(err, "limit", "error mentions limit") +end) + T.done()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@t/conformance/test_validate_query_params.lua` around lines 31 - 51, Add explicit tests to assert numeric range validation for query params by extending test_validate_query_params.lua: create a case that calls validator:validate_request with query values outside the allowed bounds (e.g., page below min or above max and limit below min or above max) and assert not ok and that err mentions "page" and "limit" respectively; also add separate tests for each param to confirm distinct min and max violations are reported (use the existing validator:validate_request invocation pattern and T.ok/T.like assertions).t/conformance/test_validate_header.lua (1)
9-15: Extract duplicatedread_filehelper into shared test utility.This helper is repeated across the new conformance files, which increases maintenance cost for small IO behavior changes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@t/conformance/test_validate_header.lua` around lines 9 - 15, The duplicated local helper read_file should be moved into a shared test utility module (e.g., create a module like test_util with a function read_file(path) that opens, reads, closes and returns the file contents and asserts on open failure); then remove the local read_file definitions from test_validate_header.lua and other conformance files and replace them with a require of that module and calls to test_util.read_file (or import the function via local read_file = require("test_util").read_file) so all tests reuse the single implementation.
🤖 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_validate_header.lua`:
- Line 27: The authorization header in the test uses a real-looking JWT literal;
replace that value with a clearly synthetic token to avoid secret scanners
(e.g., use "Bearer test-token" or "Bearer <REDACTED_JWT>") by updating the
["authorization"] entry in the test table inside test_validate_header.lua so it
no longer contains a JWT-like string.
In `@t/specs/header.json`:
- Around line 13-29: Remove the reserved header parameter entries
("Authorization" and "Content-Type") from the parameters list and instead model
auth and media type properly: define an OAuth2/HTTP bearer scheme under
components.securitySchemes (referencing the "Authorization" header currently
defined) and add a top-level or operation-level security requirement
(referencing that scheme) instead of a header parameter; for "Content-Type",
remove the header parameter and specify the media type in requestBody.content or
responses.content (referencing where the current "Content-Type" parameter is
declared) so the OpenAPI fixture uses components.securitySchemes/security and
requestBody.content/responses.content rather than header parameters.
---
Nitpick comments:
In `@t/conformance/test_validate_header.lua`:
- Around line 9-15: The duplicated local helper read_file should be moved into a
shared test utility module (e.g., create a module like test_util with a function
read_file(path) that opens, reads, closes and returns the file contents and
asserts on open failure); then remove the local read_file definitions from
test_validate_header.lua and other conformance files and replace them with a
require of that module and calls to test_util.read_file (or import the function
via local read_file = require("test_util").read_file) so all tests reuse the
single implementation.
In `@t/conformance/test_validate_query_params.lua`:
- Around line 31-51: Add explicit tests to assert numeric range validation for
query params by extending test_validate_query_params.lua: create a case that
calls validator:validate_request with query values outside the allowed bounds
(e.g., page below min or above max and limit below min or above max) and assert
not ok and that err mentions "page" and "limit" respectively; also add separate
tests for each param to confirm distinct min and max violations are reported
(use the existing validator:validate_request invocation pattern and T.ok/T.like
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: 4668b995-2236-4031-80dd-0388987ab7d5
📒 Files selected for processing (14)
t/conformance/test_validate_body.luat/conformance/test_validate_body_31.luat/conformance/test_validate_header.luat/conformance/test_validate_path_items_31.luat/conformance/test_validate_path_params.luat/conformance/test_validate_query_params.luat/conformance/test_validate_query_params_31.luat/specs/body.jsont/specs/body_31.jsont/specs/header.jsont/specs/path_items_31.jsont/specs/path_params.jsont/specs/query_params.jsont/specs/query_params_31.json
| "name": "Authorization", | ||
| "in": "header", | ||
| "required": true, | ||
| "schema": { | ||
| "type": "string", | ||
| "pattern": "^Bearer [A-Za-z0-9-_=]+\\.[A-Za-z0-9-_=]+\\.[A-Za-z0-9-_.+/=]*$" | ||
| } | ||
| }, | ||
| { | ||
| "name": "Content-Type", | ||
| "in": "header", | ||
| "required": true, | ||
| "schema": { | ||
| "type": "string", | ||
| "pattern": "^application/json$" | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In OpenAPI 3.0 and 3.1 Parameter Object rules, what is the behavior for header parameters named "Accept", "Authorization", and "Content-Type"?
💡 Result:
Behavior in OpenAPI 3.0 and 3.1 when defining Parameter Objects with in: "header" and names: 1) name: "Accept" (header parameter) - OpenAPI 3.0: Not allowed; header parameters named "Accept" are not allowed and should be modeled via response media types under responses[...].content. - OpenAPI 3.1: Same rule—if in is "header" and name is "Accept", the parameter definition SHALL be ignored. 2) name: "Authorization" (header parameter) - OpenAPI 3.0: Not allowed; header parameters named "Authorization" are not allowed and should be modeled via securitySchemes and security. - OpenAPI 3.1: Same rule—if in is "header" and name is "Authorization", the parameter definition SHALL be ignored. 3) name: "Content-Type" (header parameter) - OpenAPI 3.0: Not allowed; header parameters named "Content-Type" are not allowed and should be modeled via requestBody.content (for request) and responses[...].content (for response). - OpenAPI 3.1: Same rule—if in is "header" and name is "Content-Type", the parameter definition SHALL be ignored. Net effect: - In OpenAPI 3.1, these header-parameter definitions are explicitly ignored by validators/tools. - In OpenAPI 3.0, these header-parameter definitions are disallowed; instead use requestBody/content, responses/content, and/or security for authorization.
Citations:
- 1: https://swagger.io/docs/specification/v3_0/describing-parameters/
- 2: https://spec.openapis.org/oas/v3.1.0.html
- 3: https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.1.0.md
- 4: https://docs.bump.sh/guides/openapi/specification/v3.1/understanding-structure/parameters
- 5: Accept/Content-Type/Authorization Ignored OAI/OpenAPI-Specification#2777
Remove reserved header parameter names to ensure fixture compliance.
Line 13 (Authorization) and line 22 (Content-Type) are disallowed as header parameters in OpenAPI 3.0 and 3.1. Compliant validators will silently ignore these parameter definitions, making the fixture non-portable across tools.
♻️ Suggested fixture adjustment
- "name": "Authorization",
+ "name": "X-Authorization",- "name": "Content-Type",
+ "name": "X-Content-Type",Use securitySchemes and security for authorization, and requestBody.content or responses.content for Content-Type modeling instead.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "name": "Authorization", | |
| "in": "header", | |
| "required": true, | |
| "schema": { | |
| "type": "string", | |
| "pattern": "^Bearer [A-Za-z0-9-_=]+\\.[A-Za-z0-9-_=]+\\.[A-Za-z0-9-_.+/=]*$" | |
| } | |
| }, | |
| { | |
| "name": "Content-Type", | |
| "in": "header", | |
| "required": true, | |
| "schema": { | |
| "type": "string", | |
| "pattern": "^application/json$" | |
| } | |
| } | |
| "name": "X-Authorization", | |
| "in": "header", | |
| "required": true, | |
| "schema": { | |
| "type": "string", | |
| "pattern": "^Bearer [A-Za-z0-9-_=]+\\.[A-Za-z0-9-_=]+\\.[A-Za-z0-9-_.+/=]*$" | |
| } | |
| }, | |
| { | |
| "name": "X-Content-Type", | |
| "in": "header", | |
| "required": true, | |
| "schema": { | |
| "type": "string", | |
| "pattern": "^application/json$" | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@t/specs/header.json` around lines 13 - 29, Remove the reserved header
parameter entries ("Authorization" and "Content-Type") from the parameters list
and instead model auth and media type properly: define an OAuth2/HTTP bearer
scheme under components.securitySchemes (referencing the "Authorization" header
currently defined) and add a top-level or operation-level security requirement
(referencing that scheme) instead of a header parameter; for "Content-Type",
remove the header parameter and specify the media type in requestBody.content or
responses.content (referencing where the current "Content-Type" parameter is
declared) so the OpenAPI fixture uses components.securitySchemes/security and
requestBody.content/responses.content rather than header parameters.
|
Addressed CodeRabbit feedback: Thread 1 (JWT literal): Replaced the jwt.io example JWT with a synthetic Thread 2 (Reserved header names): These tests are intentionally ported from |
2 participants
Port all test scenarios from lua-resty-openapi-validate (Go FFI) to ensure feature parity with lua-resty-openapi-validator (pure Lua).
Tests ported (7 files, cache.t skipped as N/A):
Also copies 7 spec fixture files needed by these tests.
All tests pass with no functional gaps found — the pure Lua validator handles all scenarios that the Go FFI version does.
Summary by CodeRabbit