test(linter): Add JSON Schema validation tests for oxlintrc configuration#16822
test(linter): Add JSON Schema validation tests for oxlintrc configuration#16822
Conversation
1916266 to
7d10179
Compare
crates/oxc_linter/tests/fixtures/schema_validation/valid/ignore_patterns.json
Outdated
Show resolved
Hide resolved
crates/oxc_linter/tests/fixtures/schema_validation/valid/full_config.json
Show resolved
Hide resolved
CodSpeed Performance ReportMerging #16822 will not alter performanceComparing Summary
Footnotes
|
connorshea
left a comment
There was a problem hiding this comment.
This looks good to me generally, but I also did part of the work, so can't really approve :)
There was a problem hiding this comment.
The test suite adds valuable schema regression coverage, but it currently relies on npm/oxlint/configuration_schema.json existing at test time, which can make cargo test non-hermetic and flaky in CI or clean checkouts. The insta snapshots also capture unstable, dependency-version-sensitive jsonschema error strings, which may cause noisy churn. Consider adding a runtime fallback (or explicit precondition) for schema loading and snapshot a more stable, sorted representation of validation errors.
Additional notes (2)
-
Readability |
crates/oxc_linter/tests/schema_validation_test.rs:14-32
get_project_root().unwrap()is called multiple times. In tests this is fine, but it also means failures will be less informative and you’ll do redundant filesystem work. Consolidating to a singleproject_root()helper improves readability and makes it easier to change behavior (e.g., fallback logic) in one place. -
Maintainability |
crates/oxc_linter/tests/schema_validation_test.rs:111-126
snap_namecurrently includes the.jsonextension (e.g.invalid_env_wrong_type.json_errors). That’s not wrong, but it’s a bit noisy and makes renames (like switching fixtures to.jsoncin the future) cause snapshot churn. Also, snapshot naming depends on the filename string rather than the path, which becomes awkward if you later enumerate fixtures from the directory.
Summary of changes
What changed
✅ Added JSON Schema validation coverage for .oxlintrc
- Introduced a new Rust test suite at
crates/oxc_linter/tests/schema_validation_test.rsthat:- Loads the JSON Schema from
npm/oxlint/configuration_schema.json. - Validates a curated set of valid and invalid config fixtures.
- Snapshots invalid validation output via
instafor stable regression tracking. - Verifies the schema declares Draft 7 via the
$schemafield and is compilable.
- Loads the JSON Schema from
🧪 Added fixtures + snapshots
- Added fixture files under:
crates/oxc_linter/tests/fixtures/schema_validation/valid/*.jsoncrates/oxc_linter/tests/fixtures/schema_validation/invalid/*.json
- Added
instasnapshot files undercrates/oxc_linter/tests/snapshots/*capturing failing validation results.
📦 Dependency + repo hygiene updates
- Added
jsonschema = "0.37"as a dev-dependency incrates/oxc_linter/Cargo.toml. - Updated
.gitignoreto ignore*.rlib.
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive JSON Schema validation tests for the oxlintrc configuration file to prevent regressions in the schema generation process. The tests validate both valid and invalid configuration files against the generated schema, ensuring that the schema correctly accepts proper configs and rejects improper ones.
Key changes:
- Added
jsonschemacrate as a dev-dependency for schema validation testing - Created 11 valid test fixtures covering various configuration scenarios (plugins, categories, environments, rules, overrides, etc.)
- Created 7 invalid test fixtures testing common misconfiguration errors
- Implemented 3 test functions with snapshot testing for validation errors
- Updated
.gitignoreto exclude*.rlibbuild artifacts
Reviewed changes
Copilot reviewed 27 out of 29 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
crates/oxc_linter/Cargo.toml |
Adds jsonschema dev-dependency for validation testing |
crates/oxc_linter/tests/schema_validation_test.rs |
Implements schema validation tests with fixture loading and snapshot testing |
crates/oxc_linter/tests/fixtures/schema_validation/valid/*.json |
Provides 11 valid configuration test cases covering different features |
crates/oxc_linter/tests/fixtures/schema_validation/invalid/*.json |
Provides 7 invalid configuration test cases for error validation |
crates/oxc_linter/tests/snapshots/invalid_*.snap |
Snapshot files capturing expected validation error messages |
.gitignore |
Adds *.rlib to ignore Rust library build artifacts |
Cargo.lock |
Updates lock file with jsonschema and its transitive dependencies |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
bd5614b to
36fc2f4
Compare
Co-authored-by: connorshea <2977353+connorshea@users.noreply.github.com>
Co-authored-by: connorshea <2977353+connorshea@users.noreply.github.com>
Co-authored-by: connorshea <2977353+connorshea@users.noreply.github.com>
Co-authored-by: connorshea <2977353+connorshea@users.noreply.github.com>
- Remove *.a and *.o from .gitignore (not needed) - Add more patterns to ignore_patterns.json test fixture including **/*.ts - Add jsPlugins configuration to full_config.json test fixture - Fix clippy uninlined_format_args warnings in test file Co-authored-by: connorshea <2977353+connorshea@users.noreply.github.com>
Ensure that the snapshots exist and can display the validation results.
Add test to validate that this fails when there is an unknown field in an override config.
As requested, removing the *.rlib pattern from .gitignore. Co-authored-by: camc314 <18101008+camc314@users.noreply.github.com>
36fc2f4 to
decfb4e
Compare
|
I'm going to close this for now, I think it's worth exploring later though |
jsonschemacrate as dev-dependency tooxc_linter/Cargo.toml💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.