refactor: update JSON schema tests to use go library #563
Conversation
…se Go-based validation
WalkthroughReplaces npm/Node.js-based JSON schema validation with a Go-native approach using jsonschema/v6 in tests. Removes Node.js setup from the CI workflow. Adds sample versioned config files under config/jsonschema/. Updates go.mod to include the jsonschema dependency. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant T as Go Test Runner
participant CS as compileSchema
participant JS as jsonschema/v6
participant FS as File System
T->>FS: Read schema and example/invalid config files
T->>CS: compileSchema(version)
CS->>JS: Unmarshal + Compile schema
JS-->>CS: Compiled schema
CS-->>T: Return compiled schema
T->>JS: Validate content against compiled schema
JS-->>T: Validation result (pass/fail)
Note over T,JS: Node.js/AJV step removed from flow
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #563 +/- ##
=======================================
Coverage 79.69% 79.69%
=======================================
Files 137 137
Lines 13467 13467
=======================================
Hits 10732 10732
Misses 2310 2310
Partials 425 425
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
config/jsonschema/schema_test.go (1)
411-417: Fix: replace slices.Collect(...) with maps.Keys(...) (compile-time error).
slices.Collectexpects an iterator/Seq, not a slice.maps.Keys(props)already returns a slice, so wrapping it inslices.Collect(...)will not compile. Also drop the now-unusedslicesimport.Apply this diff:
@@ - "slices" @@ - m.EXPECT().Properties().Return(slices.Collect(maps.Keys(props))) + m.EXPECT().Properties().Return(maps.Keys(props))
🧹 Nitpick comments (7)
config/jsonschema/schema_test.go (7)
80-81: Tighten version detection to avoid false positives (e.g. "21").Current regex matches
"version": "21"as v2. Anchor to the closing quote or parse JSON for robustness.Apply this diff:
- version2Matcher := regexp.MustCompile(`"version":\s*"2`) + version2Matcher := regexp.MustCompile(`"version"\s*:\s*"2"\b`)Or parse a tiny struct for accuracy:
- schema := schema1 - if version2Matcher.Find(content) != nil { - schema = schema2 - } + schema := schema1 + var meta struct{ Version string `json:"version"` } + _ = json.Unmarshal(content, &meta) + if meta.Version == "2" { + schema = schema2 + }
97-105: Capture loop vars for parallel subtests and use filepath.Base.Protect against accidental capture by copying
filenamebeforet.Parallel(). Also preferfilepath.Basefor OS‑specific paths.Apply this diff:
- t.Run(path.Base(filename), func(t *testing.T) { - if !strings.HasSuffix(filename, ".json") { - filename = rewriteToJson(t, filename) + fn := filename + t.Run(filepath.Base(fn), func(t *testing.T) { + if !strings.HasSuffix(fn, ".json") { + fn = rewriteToJson(t, fn) } - filename, _ = filepath.Abs(filename) + fn, _ = filepath.Abs(fn) @@ - content, e = os.ReadFile(filename) + content, e = os.ReadFile(fn) @@ - t.Parallel() + t.Parallel()Also applies to: 110-121
114-120: Avoid logging full config payloads on failure.Dumping entire files can leak secrets in real configs. Log the filename and a trimmed snippet instead.
Apply this diff:
- if err != nil { - t.Log(string(content)) - } + if err != nil { + const max = 512 + preview := content + if len(preview) > max { + preview = preview[:max] + } + t.Logf("validation failed for %s; first %d bytes:\n%s", filepath.Base(fn), len(preview), string(preview)) + }
88-125: Optional: use WalkDir and early‑continue for clarity.
filepath.WalkDiravoids extraos.Statcalls and simplifies the callback; using early returns makes the intent crisper.Happy to provide a diff if you want to switch.
126-127: Make the lower bound deterministic.
assert.Greater(t, testCount, 8)is brittle. Assert against the discovered fixture count or a named constant that reflects the curated set.
137-161: Mirror the var‑capture and logging tweaks in the “invalid” suite.Apply the same
fn := filenamecopy and log the filename when the unexpected success occurs.Apply this diff:
- t.Run(path.Base(filename), func(t *testing.T) { - if !strings.HasSuffix(filename, ".json") { - filename = rewriteToJson(t, filename) + fn := filename + t.Run(filepath.Base(fn), func(t *testing.T) { + if !strings.HasSuffix(fn, ".json") { + fn = rewriteToJson(t, fn) } - filename, _ = filepath.Abs(filename) + fn, _ = filepath.Abs(fn) @@ - content, e = os.ReadFile(filename) + content, e = os.ReadFile(fn) @@ - if err == nil { - t.Log(string(content)) - } + if err == nil { + t.Logf("unexpectedly valid: %s", filepath.Base(fn)) + }
5-30: Go toolchain ≥1.23 confirmed; stdlibmaps/slices(iterator APIs) available
The stdlibmapsandslicespackages (with iterator-support functions likeslices.Collectandmaps.Keys) were added in Go 1.23 (go.daehub.com). Both go.mod (go 1.24.7) and CI matrix (go-version: 1.24) ensure the toolchain is ≥1.23.
Theslicesimport in config/jsonschema/schema_test.go remains necessary (used at line 412); drop it only after refactoring or replacing allslices.Collectcalls in that file.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (7)
.github/workflows/build.yml(0 hunks)config/jsonschema/schema_test.go(24 hunks)config/jsonschema/v1.conf(1 hunks)config/jsonschema/v1.yaml(1 hunks)config/jsonschema/v2.conf(1 hunks)config/jsonschema/v2.yaml(1 hunks)go.mod(1 hunks)
💤 Files with no reviewable changes (1)
- .github/workflows/build.yml
🧰 Additional context used
🧬 Code graph analysis (1)
config/jsonschema/schema_test.go (4)
config/version.go (3)
Version(5-5)Version01(9-9)Version02(10-10)config/jsonschema/schema.go (1)
WriteJsonSchema(20-29)restic/commands.go (1)
Option(28-33)config/info.go (1)
NamedPropertySet(66-69)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build and test (1.24, ubuntu-latest)
- GitHub Check: Build and test (1.24, windows-latest)
- GitHub Check: Build and test (1.24, macos-latest)
🔇 Additional comments (2)
config/jsonschema/schema_test.go (2)
48-56: Parallelisation looks good.Nice use of
t.Parallel()to speed up the suite; structure preserves deterministic setup and per‑case isolation.Also applies to: 58-63, 170-171, 279-279, 286-286, 293-293, 300-300, 307-307, 339-340, 464-465, 554-555, 573-574, 595-595, 610-611, 631-632, 654-654, 664-664, 686-686, 696-696, 706-706, 716-716, 726-727, 730-730, 739-740
32-46: Simplify schema compilation using AddResource & update validation API
- Replace the double-parse (
UnmarshalJSON) and re-registration with a single call toCompiler.AddResource("mem://schema.json", bytes.NewReader(schemaBuffer.Bytes())), thenCompile("mem://schema.json").- Change any
ValidateInterfacecalls to(*jsonschema.Schema).Validate(v any) error, per santhosh-tekuri/jsonschema v6.func compileSchema(t *testing.T, version config.Version) *jsonschema.Schema { t.Helper() - schemaBuffer := &bytes.Buffer{} - err := WriteJsonSchema(version, "", schemaBuffer) + schemaBuffer := &bytes.Buffer{} + err := WriteJsonSchema(version, "", schemaBuffer) require.NoError(t, err) - schemaJSON, err := jsonschema.UnmarshalJSON(schemaBuffer) - require.NoError(t, err) - compiler := jsonschema.NewCompiler() - err = compiler.AddResource("schema.json", schemaJSON) - require.NoError(t, err) - schema, err := compiler.Compile("schema.json") + compiler := jsonschema.NewCompiler() + err = compiler.AddResource("mem://schema.json", bytes.NewReader(schemaBuffer.Bytes())) + require.NoError(t, err) + schema, err := compiler.Compile("mem://schema.json") require.NoError(t, err) return schema }
Refactor: Remove Node.js dependency from JSON schema validation tests
This PR removes the Node.js dependency previously required for JSON schema validation in tests and replaces it with a pure Go solution using the
github.com/santhosh-tekuri/jsonschema/v6library.Changes:
github.com/santhosh-tekuri/jsonschema/v6for schema compilation and validationBenefits:
The functionality remains the same - all JSON schema validation tests continue to work as expected, but now run entirely within the Go ecosystem.