feat: testFunc method on schemas for easier custom tests#82
Conversation
WalkthroughThis pull request introduces a new method, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Schema
participant Validator
User->>Schema: Create schema instance (e.g., BoolSchema, NumberSchema, etc.)
User->>Schema: Call TestFunc(customTestFunc, options...)
Schema->>Schema: Construct a Test object with customTestFunc
Schema->>Schema: Append Test to internal tests list
User->>Validator: Validate input using the schema
Validator->>Schema: Invoke registered custom test(s)
Schema->>Validator: Return test result (pass/fail)
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Deploying zog with
|
| Latest commit: |
ec629a0
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://65cc5bb8.zog-3a0.pages.dev |
| Branch Preview URL: | https://feat-testfuncs.zog-3a0.pages.dev |
![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
🧹 Nitpick comments (9)
boolean.go (1)
90-95: Consider adding validation for nil testFunc.While the implementation is good, it might be worth adding a nil check for the
testFuncparameter to prevent potential panics.func (v *BoolSchema) TestFunc(testFunc p.TestFunc, options ...TestOption) *BoolSchema { + if testFunc == nil { + panic("testFunc cannot be nil") + } test := TestFunc("", testFunc) v.Test(test, options...) return v }numbers.go (1)
153-158: Consider adding validation for nil testFunc.While the implementation is good, it should include a nil check for the
testFuncparameter, similar to the suggestion forBoolSchema.func (v *NumberSchema[T]) TestFunc(testFunc p.TestFunc, options ...TestOption) *NumberSchema[T] { + if testFunc == nil { + panic("testFunc cannot be nil") + } test := TestFunc("", testFunc) v.Test(test, options...) return v }numbers_test.go (1)
284-289: Enhance test coverage for the custom test function.The test function only verifies the success case. Consider adding test cases for:
- When the custom test function returns
false.- Edge cases like zero, negative numbers, etc.
Apply this diff to improve test coverage:
-func TestNumberCustomTest(t *testing.T) { +func TestNumberCustomTest(t *testing.T) { + tests := []struct { + name string + input int + expected bool + expectErr bool + }{ + { + name: "Success case", + input: 5, + expected: true, + }, + { + name: "Failure case", + input: 0, + expected: false, + expectErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + validator := Int().TestFunc(func(val any, ctx Ctx) bool { + num := val.(int) + return num > 0 + }, Message("custom")) + + dest := 0 + errs := validator.Parse(tt.input, &dest) + + if tt.expectErr { + assert.NotEmpty(t, errs) + } else { + assert.Empty(t, errs) + } + assert.Equal(t, tt.input, dest) + }) + } }boolean_validate_test.go (1)
378-390: Enhance test coverage for the custom test function.The test function only verifies the success case. Consider adding test cases for:
- When the custom test function returns
false.- Edge cases like
falsevalue.Apply this diff to improve test coverage:
-func TestBoolValidateCustomTest(t *testing.T) { +func TestBoolValidateCustomTest(t *testing.T) { + tests := []struct { + name string + input bool + expected bool + expectErr bool + }{ + { + name: "Success case", + input: true, + expected: true, + }, + { + name: "Failure case", + input: false, + expected: false, + expectErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + validator := Bool().TestFunc(func(val any, ctx Ctx) bool { + b := val.(bool) + return b + }, Message("custom")) + + dest := tt.input + errs := validator.Validate(&dest) + + if tt.expectErr { + assert.NotEmpty(t, errs) + } else { + assert.Empty(t, errs) + } + assert.Equal(t, tt.expected, dest) + }) + } }struct_test.go (2)
141-152: Enhance test coverage for the custom test function.The test function only verifies the success case. Consider adding test cases for:
- When the custom test function returns
false.- Edge cases like zero or negative numbers.
Apply this diff to improve test coverage:
- // Create a custom test function - customTest := func(val any, ctx ParseCtx) bool { - // Custom test logic here - num := val.(int) - return num > 0 - } - - // Create a schema with a custom test - schema := Struct(Schema{ - "str": String().Required(), - "num": Int().TestFunc(customTest), - }) + tests := []struct { + name string + input int + expected bool + expectErr bool + }{ + { + name: "Success case", + input: 10, + expected: true, + }, + { + name: "Failure case - zero", + input: 0, + expected: false, + expectErr: true, + }, + { + name: "Failure case - negative", + input: -1, + expected: false, + expectErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + schema := Struct(Schema{ + "str": String().Required(), + "num": Int().TestFunc(func(val any, ctx ParseCtx) bool { + num := val.(int) + return num > 0 + }), + }) + + var obj CustomStruct + data := map[string]any{ + "str": "hello", + "num": tt.input, + } + + errs := schema.Parse(data, &obj) + if tt.expectErr { + assert.NotNil(t, errs) + } else { + assert.Nil(t, errs) + assert.Equal(t, "hello", obj.Str) + assert.Equal(t, tt.input, obj.Num) + } + }) + }
171-186: Uncomment the error code assertion.The error code assertion has been commented out. Since error codes are important for error handling, consider keeping the assertion.
Apply this diff to restore the error code assertion:
- // assert.Equal(t, "customTest", errs["$root"][0].Code()) + assert.Equal(t, "customTest", errs["$root"][0].Code())string.go (1)
165-170: LGTM! Consider adding a custom error code parameter.The implementation is clean and follows the method chaining pattern. However, consider adding an optional error code parameter to allow users to customize the error message.
Apply this diff to add an optional error code parameter:
-func (v *StringSchema) TestFunc(testFunc p.TestFunc, options ...TestOption) *StringSchema { - test := TestFunc("", testFunc) +func (v *StringSchema) TestFunc(testFunc p.TestFunc, errCode string, options ...TestOption) *StringSchema { + test := TestFunc(errCode, testFunc)docs/docs/core-concepts/1-anatomy-of-schema.md (2)
64-64: Replace hard tab with spaces.Replace the hard tab with spaces to maintain consistent formatting.
Apply this diff to fix the formatting:
- ErrCode zconst.ZogIssueCode // the error code to use if the validation fails. This helps identify the type of error, for example ErrCodeMin identifies the Min() test + ErrCode zconst.ZogIssueCode // the error code to use if the validation fails. This helps identify the type of error, for example ErrCodeMin identifies the Min() test🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
64-64: Hard tabs
Column: 1(MD010, no-hard-tabs)
78-80: Enhance example to show error code usage.The example could be improved by showing how to use a custom error code with
TestFunc.Apply this diff to enhance the example:
-z.String().TestFunc(func(data any, ctx z.Ctx) bool { +z.String().TestFunc(func(data any, ctx z.Ctx) bool { return data == "test" -})) +}, "custom_test_error"))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
boolean.go(1 hunks)boolean_test.go(1 hunks)boolean_validate_test.go(1 hunks)docs/docs/core-concepts/1-anatomy-of-schema.md(2 hunks)numbers.go(1 hunks)numbers_test.go(1 hunks)numbers_validate_test.go(1 hunks)slices.go(1 hunks)slices_test.go(1 hunks)slices_validate_test.go(2 hunks)string.go(1 hunks)string_test.go(1 hunks)string_validate_test.go(1 hunks)struct.go(1 hunks)struct_test.go(3 hunks)time.go(1 hunks)time_test.go(1 hunks)time_validate_test.go(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
docs/docs/core-concepts/1-anatomy-of-schema.md
64-64: Hard tabs
Column: 1
(MD010, no-hard-tabs)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (13)
time_validate_test.go (1)
118-120: LGTM! Simplified custom test function syntax.The changes improve readability by using the new
TestFuncmethod, which provides a cleaner way to define custom test functions.boolean.go (1)
82-88: LGTM! Well-implemented test method.The
Testmethod correctly applies options and maintains method chaining.time_test.go (1)
123-125: LGTM! Consistent with the new API.The changes align with the new
TestFuncmethod pattern, maintaining consistency across the codebase.slices_test.go (2)
219-223: LGTM! The test function has been simplified.The change aligns with the PR objective of making custom tests easier to define by removing the need for explicit test function naming.
230-230: Verify if error code assertion should be removed.The commented-out assertion suggests a change in error code handling. Please verify if this is intentional and aligns with the broader changes in error handling across the codebase.
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Check for similar error code assertions in other test files. # Test: Search for error code assertions. Expect: Pattern of commented or removed assertions. rg -A 1 'assert\.Equal\(t, .+, errs\[.+\]\.Code\(\)\)'Length of output: 685
Subject: Verified – Removal of error code assertions in slices tests is intentional
The investigation shows that the error code assertions in slices tests (e.g., in
slices_test.goandslices_validate_test.go) are commented out, while similar assertions remain active in struct tests with a different expected code format. This indicates that the removal was deliberate to accommodate the updated error handling approach for slices.time.go (1)
166-171: LGTM! The TestFunc method implementation is clean and consistent.The implementation follows the same pattern as other schema types and maintains proper method chaining.
Please verify if using an empty string as the error code is intentional:
✅ Verification successful
🌐 Web query:
What is the recommended practice for handling error codes in Go validation libraries like Zod?💡 Result:
Error handling in Go validation follows different patterns than TypeScript's Zod due to language design differences, but several best practices emerge from Go's ecosystem:
Core Principles for Go Validation Errors
1. Custom Error Types with Context
- Define validation-specific error types implementing
errorinterface- Include contextual details (field path, expected/received values):
type ValidationError struct { Field string Message string Code int }2. Sentinel Errors for Specific Cases
- Use package-level variables for common validation failures:
var ( ErrInvalidEmail = errors.New("invalid email format") ErrRequiredField = errors.New("field is required") )3. Structured Error Returns
- Return collections of errors for batch validation:
func ValidateUser(u User) []ValidationError { var errs []ValidationError if u.Email == "" { errs = append(errs, ValidationError{ Field: "email", Message: ErrRequiredField.Error(), Code: 1001, }) } return errs }Recommended Patterns from Search Results
Error Wrapping (Go 1.13+)
- Use
fmt.Errorfwith%wverb to preserve error chain[1][9]:if err := validate(input); err != nil { return fmt.Errorf("validation failed: %w", err) }Functional Options Validation
- Implement reusable validation rules as functions[4]:
type Validator func(interface{}) error func WithMinLength(n int) Validator { return func(v interface{}) error { s, ok := v.(string) if !ok || len(s) < n { return fmt.Errorf("minimum length %d required", n) } return nil } }HTTP Error Handling
- Convert validation errors to structured HTTP responses[10]:
func handleError(w http.ResponseWriter, errs []ValidationError) { w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusBadRequest) json.NewEncoder(w).Encode(map[string]interface{}{ "errors": errs, }) }Comparison with Zod-like Features
Aspect Zod (TypeScript) Go Equivalent Error Structure ZodErrorwithissuesSlice of custom error structs Error Codes Predefined ZodIssueCodeCustom constants/sentinel errors Path Tracking patharray propertyField names in error structs Custom Messages Error maps Error struct methods/String() impl Batch Validation Automatic aggregation Manual error slice collection For complex validations, consider using established Go libraries like:
go-playground/validator: Struct tag-based validationasaskevich/govalidator: Common validation ruleshashicorp/go-cty: Type system with validationThe key differentiator in Go is explicit error handling - rather than throwing exceptions, validation errors should be treated as normal return values that callers must consciously handle[1][7]. This aligns with Go's philosophy of making error conditions explicit rather than hidden through exception propagation.
Citations:
- 1: https://tillitsdone.com/blogs/effective-error-handling-in-go/
- 2: https://zod.dev/ERROR_HANDLING
- 3: https://tillitsdone.com/blogs/zod-error-handling-guide/
- 4: https://dev.to/ansu/best-practices-for-building-a-validation-layer-in-go-59j9
- 5: https://zod.dev/ERROR_HANDLING?id=error-handling-for-forms
- 6: https://github.com/colinhacks/zod/actions
- 7: https://www.jetbrains.com/guide/go/tutorials/handle_errors_in_go/best_practices/
- 8: https://github.com/colinhacks/zod/blob/main/ERROR_HANDLING.md
- 9: https://dev.to/mpiorowski/typescript-with-gorust-errors-no-trycatch-heresy-49mf
- 10: https://boldlygo.tech/posts/2024-01-08-error-handling/
The implementation is consistent with the intent of the library. In similar schema types throughout the codebase (e.g., for string, number, etc.), a call to the internal helper function (named
TestFunc) is made with an empty error code string when a code is not explicitly provided. The empty string here acts as a default “no code” signal, aligning with the library’s pattern rather than implying an error. In Go’s ecosystem, while many validation libraries may use structured or numeric error codes, this library’s design choice to use an empty string here is intentional and consistent with its method chaining pattern.Actionable Note:
- No changes are needed regarding the empty string for the error code in the
TestFuncmethod intime.go.slices_validate_test.go (2)
157-162: LGTM! The test function has been simplified.The change aligns with the PR objective of making custom tests easier to define.
176-176: Verify if error code assertion should be removed.The commented-out assertion suggests a change in error code handling. Please verify if this is intentional and aligns with the broader changes in error handling across the codebase.
numbers_validate_test.go (1)
274-278: LGTM! The test function has been simplified.The change aligns with the PR objective of making custom tests easier to define.
struct.go (1)
297-302: LGTM! The implementation is clean and consistent.The
TestFuncmethod follows the same pattern as other schema types, making it easy to understand and maintain.string_test.go (1)
101-118: LGTM! Well-structured test function.The test function is well-designed, covering both success and failure cases for custom validation, with proper error message verification.
string_validate_test.go (1)
106-122: LGTM! Comprehensive test coverage.The test function effectively validates the custom test functionality using both success and failure cases, with proper error message verification.
slices.go (1)
280-285: LGTM! Clean implementation of TestFunc.The implementation is consistent with other schema types and properly leverages the existing
Testmethod.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
1 participant
Summary by CodeRabbit