-
Notifications
You must be signed in to change notification settings - Fork 93
feat: [#441] Add Fluent Json Api to test Json response [#4] #711
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThe pull request introduces significant enhancements to JSON assertion capabilities within the testing framework. The Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #711 +/- ##
==========================================
+ Coverage 69.70% 69.89% +0.19%
==========================================
Files 197 197
Lines 15298 15455 +157
==========================================
+ Hits 10663 10803 +140
- Misses 4059 4062 +3
- Partials 576 590 +14 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.50
.
Benchmark suite | Current: 9b1bc7a | Previous: 1cc1f60 | Ratio |
---|---|---|---|
BenchmarkFile_ReadWrite |
316950 ns/op 1800 B/op 21 allocs/op |
202099 ns/op 1799 B/op 21 allocs/op |
1.57 |
BenchmarkFile_ReadWrite - ns/op |
316950 ns/op |
202099 ns/op |
1.57 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pretty good 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (11)
contracts/testing/assertable_json.go (1)
5-12
: LGTM! Consider adding documentation for complex assertions.The new interface methods provide a comprehensive set of JSON assertions following a fluent interface pattern. The additions are well-structured and logically grouped.
Consider adding GoDoc comments for methods with complex behavior, particularly:
Count
: Clarify if it works with array lengths, nested object counts, etc.HasAny
/HasAll
: Document behavior with nested pathsWhere
/WhereNot
: Specify type comparison behaviorcontracts/testing/test_response.go (2)
44-47
: Add documentation for the new JSON assertion methods.The new methods look good and provide valuable JSON assertion capabilities. However, they would benefit from documentation explaining their behavior, especially:
- Whether
AssertJson
performs deep or shallow matching- The exact equality semantics of
AssertExactJson
- The scope of
AssertJsonMissing
(keys only or values too)- Expected usage pattern for
AssertFluentJson
44-47
: Consider breaking down the TestResponse interface.The interface is growing quite large with 40+ methods. Consider:
- Breaking it into smaller, focused interfaces (e.g.,
JSONAssertions
,StatusAssertions
,HeaderAssertions
)- Using composition to maintain the current API while improving maintainability
Example approach:
type JSONAssertions interface { Json() (map[string]any, error) AssertJson(map[string]any) TestResponse AssertExactJson(map[string]any) TestResponse AssertJsonMissing(map[string]any) TestResponse AssertFluentJson(func(json AssertableJSON)) TestResponse } type StatusAssertions interface { AssertStatus(status int) TestResponse AssertOk() TestResponse // ... other status methods } type TestResponse interface { JSONAssertions StatusAssertions // ... other interfaces }testing/test_response_test.go (3)
184-188
: Rename test function to follow naming convention.The function name
TestServerError
doesn't follow the "Assert" prefix convention used by other test functions in this file.-func TestServerError(t *testing.T) { +func TestAssertServerError(t *testing.T) {
269-275
: Enhance test coverage with complex JSON structures.The current test only verifies simple key-value pairs. Consider adding test cases for:
- Nested JSON objects
- Array handling
- Different data types (boolean, null, etc.)
func TestAssertJson(t *testing.T) { res := createTestResponse(http.StatusOK) - res.Body = io.NopCloser(strings.NewReader(`{"key1": "value1", "key2": 42}`)) + res.Body = io.NopCloser(strings.NewReader(`{ + "key1": "value1", + "key2": 42, + "nested": { + "array": [1, 2, 3], + "bool": true, + "null": null + } + }`)) r := NewTestResponse(t, res) - r.AssertJson(map[string]any{"key1": "value1"}) + r.AssertJson(map[string]any{ + "key1": "value1", + "nested": map[string]any{ + "array": []any{1.0, 2.0, 3.0}, + "bool": true, + }, + }) }
293-307
: Add test cases for advanced fluent assertions.The current test demonstrates basic assertions. Consider adding test cases for:
- Array operations (count, first, last)
- Type assertions
- Regular expression matching
- Error cases
func TestAssertFluentJson(t *testing.T) { - sampleJson := `{"name": "krishan", "age": 22, "email": "krishan@example.com"}` + sampleJson := `{ + "name": "krishan", + "age": 22, + "email": "krishan@example.com", + "tags": ["go", "testing"], + "scores": { + "math": 95, + "science": 88 + } + }` res := createTestResponse(http.StatusOK) res.Body = io.NopCloser(strings.NewReader(sampleJson)) r := NewTestResponse(t, res) r.AssertFluentJson(func(json contractstesting.AssertableJSON) { - json.Has("name").Where("name", "krishan") - json.Has("age").Where("age", float64(22)) - json.Has("email").Where("email", "krishan@example.com") + // Test nested objects + json.Has("scores").Where("scores.math", float64(95)) + + // Test array operations + json.Has("tags").Count("tags", 2) + + // Test type assertions + json.Type("age", "number") + json.Type("tags", "array") + + // Test regex matching + json.Where("email", "^[a-z]+@example\\.com$") }).AssertFluentJson(func(json contractstesting.AssertableJSON) { - json.Missing("non_existent_field") + // Test error cases + json.Missing("invalid_field") + json.Missing("scores.invalid") }) }testing/test_response.go (2)
310-322
: Consider enhancing error handling for JSON parsing failuresWhile the implementation is correct, consider making JSON parsing errors more explicit to help developers debug test failures.
func (r *TestResponseImpl) AssertJson(data map[string]any) contractstesting.TestResponse { content, err := r.getContent() - assert.Nil(r.t, err) + assert.Nil(r.t, err, "Failed to read response content") assertableJson, err := NewAssertableJSON(r.t, content) - assert.Nil(r.t, err) + assert.Nil(r.t, err, "Failed to parse response as JSON: %v", err)
344-354
: Consider enhancing error messages for better debuggingWhile the implementation is solid, consider making the error messages more specific to help identify issues during test failures.
func (r *TestResponseImpl) AssertFluentJson(callback func(json contractstesting.AssertableJSON)) contractstesting.TestResponse { content, err := r.getContent() - assert.Nil(r.t, err) + assert.Nil(r.t, err, "Failed to read response content for fluent JSON assertion") assertableJson, err := NewAssertableJSON(r.t, content) - assert.Nil(r.t, err) + assert.Nil(r.t, err, "Failed to parse response as JSON for fluent assertion: %v", err)testing/assertable_json.go (3)
14-18
: Use consistent capitalization of 'JSON' in type and function namesThe type
AssertableJsonImpl
uses 'Json', while the constructorNewAssertableJSON
uses 'JSON'. For consistency and to follow Go naming conventions, consider renamingAssertableJsonImpl
toAssertableJSONImpl
.
20-32
: Handle JSON arrays inNewAssertableJSON
or document the limitationCurrently,
NewAssertableJSON
expects a JSON object and will fail if the input JSON is an array or other type. Consider enhancing it to handle JSON arrays or updating the documentation to specify that only JSON objects are supported.
98-100
: Consider removing thegetDecoded()
method and accessr.decoded
directlySince
getDecoded()
simply returnsr.decoded
, andr.decoded
is an unexported field used only within the struct methods, you could accessr.decoded
directly to simplify the code.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (2)
mocks/testing/AssertableJSON.go
is excluded by!mocks/**
mocks/testing/TestResponse.go
is excluded by!mocks/**
📒 Files selected for processing (6)
contracts/testing/assertable_json.go
(1 hunks)contracts/testing/test_response.go
(1 hunks)testing/assertable_json.go
(1 hunks)testing/assertable_json_string.go
(0 hunks)testing/test_response.go
(2 hunks)testing/test_response_test.go
(4 hunks)
💤 Files with no reviewable changes (1)
- testing/assertable_json_string.go
🔇 Additional comments (3)
contracts/testing/assertable_json.go (1)
5-12
: Verify implementation handles edge cases correctly.
Since these methods will be implemented by AssertableJsonImpl, we should verify the implementation handles various edge cases properly.
Ensure the implementation properly handles:
- nil values and empty structures
- deeply nested JSON objects
- type mismatches in Where/WhereNot
- non-existent paths in nested structures
✅ Verification successful
Based on the implementation details and test coverage shown in the output, I can now provide a final response regarding the edge case handling:
Implementation properly handles edge cases with robust validation
The implementation in AssertableJsonImpl
shows proper edge case handling:
- Nil/empty checks: Uses
maps.Get()
andmaps.Has()
which safely handle nil maps and missing keys - Type validation: Performs type assertions with proper error messages via
assert
package - Nested structures: Properly traverses nested JSON via the maps package utilities
- Error handling: Proper error handling during JSON unmarshaling in
NewAssertableJSON()
The implementation is well-tested with extensive test coverage across various test files, validating both success and error scenarios.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Look for the implementation and test files to verify edge case handling
# Find the implementation file
echo "Looking for AssertableJsonImpl implementation..."
rg -l "type AssertableJsonImpl struct"
# Find related test files
echo "Looking for related test files..."
fd -g "*test.go" | xargs rg -l "AssertableJsonImpl"
# Look for specific edge case tests
echo "Checking for edge case tests..."
rg -A 5 "func Test.*JSON.*nil|func Test.*JSON.*Empty|func Test.*JSON.*Nested"
Length of output: 456
Script:
#!/bin/bash
# Let's examine the implementation and test files more thoroughly
# First, let's look at the actual implementation
echo "=== Implementation details ==="
rg -A 10 "type AssertableJsonImpl struct" testing/assertable_json.go
# Check the implementation of specific methods
echo -e "\n=== Method implementations ==="
for method in "Count" "Has" "HasAll" "HasAny" "Missing" "MissingAll" "Where" "WhereNot"; do
echo -e "\n--- $method implementation ---"
rg -A 10 "func.*$method.*AssertableJSON" testing/assertable_json.go
done
# Look for test cases with more flexible pattern
echo -e "\n=== Test cases ==="
rg -A 10 "func Test.*JSON" testing/assertable_json.go
# Check if there are any error handling or validation checks
echo -e "\n=== Error handling and validation ==="
rg "Error|error|nil|validate|check" testing/assertable_json.go
Length of output: 6575
Script:
#!/bin/bash
# Let's check the maps package implementation since it's crucial for edge case handling
# Check maps package implementation
echo "=== Maps package implementation ==="
rg -A 10 "func.*Get.*map\[string\]any"
rg -A 10 "func.*Has.*map\[string\]any"
rg -A 10 "func.*HasAny.*map\[string\]any"
# Look for test files
echo -e "\n=== Test files content ==="
fd -g "*_test.go" | xargs rg -l "AssertableJSON|maps.Get|maps.Has"
# If we find test files, examine their content
echo -e "\n=== Test cases content ==="
fd -g "*_test.go" | xargs rg -B 2 -A 10 "func Test"
# Check for any panic handling or recovery mechanisms
echo -e "\n=== Panic handling ==="
rg "panic|recover|defer" testing/assertable_json.go
Length of output: 233936
testing/test_response.go (2)
Line range hint 36-42
: LGTM: Clean transition to new JSON assertion implementation
The change from NewAssertableJSONString
to NewAssertableJSON
maintains the same functionality while aligning with the new implementation structure.
324-329
: LGTM: Clean implementation of exact JSON matching
The implementation correctly performs deep equality checking of JSON structures with clear error messaging.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (9)
contracts/testing/test_request.go (1)
16-18
: LGTM! Comprehensive authentication methods with room for expansion.The authentication methods provide good coverage for common scenarios. Consider future enhancements for:
- OAuth2 client credentials flow
- API key authentication
- Custom authentication schemes
testing/assertable_json.go (5)
14-18
: Add documentation for exported type.Consider adding GoDoc comments for the exported
AssertableJson
struct to improve code documentation.+// AssertableJson provides a fluent interface for making assertions about JSON data +// in tests. It wraps the testing context and JSON data for convenient assertions. type AssertableJson struct {
20-32
: Add input validation and documentation for constructor.The constructor should validate the input JSON string and have proper documentation.
+// NewAssertableJSON creates a new AssertableJSON instance from a JSON string. +// It decodes the JSON string into a map and returns an error if the decoding fails. func NewAssertableJSON(t *testing.T, jsonStr string) (contractstesting.AssertableJSON, error) { + if t == nil { + return nil, fmt.Errorf("testing.T instance cannot be nil") + } + if jsonStr == "" { + return nil, fmt.Errorf("JSON string cannot be empty") + }
80-88
: Improve nil handling in Where method.The Where method should handle nil values gracefully and provide clear error messages.
func (r *AssertableJson) Where(key string, value any) contractstesting.AssertableJSON { r.Has(key) actual := maps.Get(r.getDecoded(), key) + if actual == nil && value != nil { + assert.Fail(r.t, fmt.Sprintf("Property [%s] is null but expected value was [%v].", key, value)) + return r + } assert.Equal(r.t, value, actual, fmt.Sprintf("Expected property [%s] to have value [%v], but got [%v].", key, value, actual)) return r }
98-100
: Consider making getDecoded private.The
getDecoded
method appears to be an internal helper but is exported. Consider making it private since it's only used internally.-func (r *AssertableJson) getDecoded() map[string]any { +func (r *AssertableJson) getDecoded() map[string]any {
1-100
: Consider adding test coverage metrics.While the implementation is solid, ensure comprehensive test coverage for this testing utility itself. Consider adding test cases that cover:
- Edge cases with different JSON types
- Error scenarios
- Method chaining
- Complex nested JSON structures
This is particularly important as this code is part of the testing framework itself.
testing/test_request.go (3)
34-37
: Add documentation for FlushHeaders methodConsider adding a doc comment explaining that this is a destructive operation that removes all previously set headers. This helps prevent unexpected behavior.
+// FlushHeaders removes all previously set headers from the request. +// Use this when you need to start with a clean slate for header configuration. func (r *TestRequest) FlushHeaders() contractstesting.TestRequest {
69-75
: Add input validation for token parameterWhile the implementation is correct, consider adding validation for empty tokens to prevent invalid Authorization headers.
func (r *TestRequest) WithToken(token string, ttype ...string) contractstesting.TestRequest { + if token == "" { + return r.WithoutToken() + } tt := "Bearer" if len(ttype) > 0 { tt = ttype[0] }
Missing test coverage for authentication-related methods in TestRequest
The authentication-related methods (
WithToken
,WithBasicAuth
,WithoutToken
,FlushHeaders
) intesting/test_request.go
lack test coverage. While there are extensive tests for other response-related functionality, these new methods remain untested.Required test coverage:
FlushHeaders()
: Verify headers are clearedWithToken()
: Test both default Bearer and custom token typesWithBasicAuth()
: Verify correct Base64 encoding and header formatWithoutToken()
: Ensure Authorization header is removed🔗 Analysis chain
Line range hint
34-84
: Verify test coverage for new authentication methodsThe new authentication-related methods form a cohesive feature set. Let's verify the test coverage:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for test cases covering the new authentication methods rg -l "WithToken|WithBasicAuth|WithoutToken|FlushHeaders" --type go | grep "_test.go"Length of output: 85
Script:
#!/bin/bash # Let's try a different approach to find test files and their content # First, find the test file in the same directory fd "test_request_test.go" testing/ # If not found, let's check the content of test files in the testing directory fd ".*_test.go" testing/ --exec cat {}Length of output: 20540
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
mocks/testing/TestRequest.go
is excluded by!mocks/**
📒 Files selected for processing (3)
contracts/testing/test_request.go
(1 hunks)testing/assertable_json.go
(1 hunks)testing/test_request.go
(3 hunks)
🔇 Additional comments (6)
contracts/testing/test_request.go (2)
9-9
: LGTM! Well-designed header management methods.
The header management methods follow a consistent pattern and provide a fluent interface. The addition of FlushHeaders()
is particularly useful for test isolation.
Also applies to: 13-15
9-18
: Verify integration with JSON assertion capabilities.
Let's ensure that the new header and authentication methods work seamlessly with the JSON assertion enhancements mentioned in the PR.
✅ Verification successful
Authentication methods integrate well with JSON assertions
The verification shows that:
- The TestRequest interface methods for authentication (WithToken, WithBasicAuth) are properly implemented and mocked
- The JSON assertion capabilities (AssertFluentJson) are well-integrated into the TestResponse interface
- Test cases demonstrate the fluent API design allowing chaining of assertions
- The AssertableJSON interface provides comprehensive JSON validation methods (Has, Where, Missing, etc.)
The authentication methods and JSON assertions can work together seamlessly in test cases, where responses from authenticated requests can be validated using the fluent JSON assertion API.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for test files that demonstrate the integration of these methods with JSON assertions
# Look for test files that use both the new methods and JSON assertions
rg -l "WithToken|WithBasicAuth|FlushHeaders" | xargs rg "json"
# Check for examples or documentation
fd -g "*.md" | xargs rg -A 5 "WithToken|WithBasicAuth|FlushHeaders"
Length of output: 132
Script:
#!/bin/bash
# Let's try a different approach to verify the integration
# First, let's find all files containing TestRequest interface or implementation
rg -l "TestRequest"
# Then, let's look for test files that might demonstrate usage
fd -e test.go | xargs rg -A 5 "WithToken|WithBasicAuth|FlushHeaders"
# Also check for any JSON-related assertions in the testing package
rg -g "test*.go" "json|JSON" contracts/testing/
Length of output: 10795
Script:
#!/bin/bash
# Let's examine the implementation and usage of AssertFluentJson to understand the integration
# Check the AssertFluentJson implementation
ast-grep --pattern 'AssertFluentJson($$$)'
# Look for actual test cases using both authentication methods and JSON assertions
rg -B 5 -A 5 "AssertFluentJson" testing/
# Check the AssertableJSON interface/type definition
rg -A 5 "type AssertableJSON"
Length of output: 5966
testing/assertable_json.go (2)
1-12
: LGTM! Clean package structure and imports.
The package organization follows Go conventions, and the imports are well-structured with appropriate dependencies for testing functionality.
38-43
:
Add type checking in Count method.
As mentioned in the past review, the Count method needs type checking to prevent panics with unsupported types.
func (r *AssertableJson) Count(key string, length int) contractstesting.AssertableJSON {
actual := maps.Get(r.getDecoded(), key)
+ if actual == nil {
+ assert.Fail(r.t, fmt.Sprintf("Property [%s] does not exist.", key))
+ return r
+ }
+ switch v := actual.(type) {
+ case []interface{}, map[string]interface{}, string:
+ assert.Len(r.t, v, length, fmt.Sprintf("Property [%s] does not have the expected size.", key))
+ default:
+ assert.Fail(r.t, fmt.Sprintf("Property [%s] is not a countable type.", key))
+ }
- assert.Len(r.t, actual, length, fmt.Sprintf("Property [%s] does not have the expected size.", key))
return r
}
testing/test_request.go (2)
5-6
: LGTM: Required imports added for new functionality
The new imports are appropriate for the added authentication features.
82-84
: LGTM: Clean implementation of token removal
The method correctly reuses existing functionality and follows the DRY principle.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
🧹 Outside diff range and nitpick comments (6)
contracts/testing/assertable_json.go (3)
5-15
: Add method grouping comments for better documentation.The interface methods would benefit from grouping comments to improve readability and documentation. Consider adding descriptive comments for each group of related methods.
type AssertableJSON interface { Json() map[string]any + // Count assertions Count(key string, value int) AssertableJSON + // Existence assertions Has(key string) AssertableJSON HasAll(keys []string) AssertableJSON HasAny(keys []string) AssertableJSON Missing(key string) AssertableJSON MissingAll(keys []string) AssertableJSON + // Value comparison assertions Where(key string, value any) AssertableJSON WhereNot(key string, value any) AssertableJSON + // Iteration and scoped assertions Each(key string, callback func(AssertableJSON)) AssertableJSON First(key string, callback func(AssertableJSON)) AssertableJSON HasWithScope(key string, length int, callback func(AssertableJSON)) AssertableJSON
11-12
: Consider using generics for type-safe value comparisons.The
Where
andWhereNot
methods useany
type which could lead to runtime errors. Consider using generics to provide compile-time type safety.- Where(key string, value any) AssertableJSON - WhereNot(key string, value any) AssertableJSON + Where[T comparable](key string, value T) AssertableJSON + WhereNot[T comparable](key string, value T) AssertableJSON
13-15
: Document callback usage patterns.The callback-based methods (
Each
,First
,HasWithScope
) are powerful but might be unclear to users. Consider adding godoc comments explaining their usage patterns and providing examples.+ // Each iterates over array elements at the given key, applying the callback to each element + // Example: Each("users", func(json AssertableJSON) { json.Has("id").Has("name") }) Each(key string, callback func(AssertableJSON)) AssertableJSON + // First applies the callback to the first element of an array at the given key + // Example: First("users", func(json AssertableJSON) { json.Has("id").Has("name") }) First(key string, callback func(AssertableJSON)) AssertableJSON + // HasWithScope verifies array length and applies validation callback to each element + // Example: HasWithScope("users", 2, func(json AssertableJSON) { json.Has("id") }) HasWithScope(key string, length int, callback func(AssertableJSON)) AssertableJSONtesting/assertable_json_test.go (2)
11-22
: Consider adding more edge cases to strengthen the test coverage.While the current test cases cover the basic scenarios well, consider adding tests for:
- Empty JSON object/array:
{}
and[]
- Null JSON:
null
- JSON with special characters:
{"key\n": "value\u0022"}
1-177
: General improvements needed for the test suite.While the test coverage is good, consider these overall improvements:
Test Organization:
- Group related test cases using subtests (
t.Run
)- Consider table-driven tests for similar cases
- Add test constants for commonly used JSON strings
Documentation:
- Add package documentation
- Document test purposes and scenarios
- Add examples in documentation
Error Handling:
- Standardize error checking across tests
- Add helper functions for common assertions
- Implement consistent panic recovery
Example of using subtests:
+// TestAssertableJSON groups all JSON assertion tests +func TestAssertableJSON(t *testing.T) { + tests := []struct { + name string + json string + testFunc func(*testing.T, contractstesting.AssertableJSON) + }{ + { + name: "count assertions", + json: `{"items": [1, 2, 3]}`, + testFunc: func(t *testing.T, aj contractstesting.AssertableJSON) { + aj.Count("items", 3) + }, + }, + // Add more test cases + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assertable, err := NewAssertableJSON(t, tt.json) + assert.NoError(t, err) + tt.testFunc(t, assertable) + }) + } +}testing/assertable_json.go (1)
98-184
: Refactor to reduce code duplication and improve error handling.
- The array validation logic is duplicated across First, HasWithScope, and Each methods.
- The Each method silently continues on errors, which might hide issues.
Consider these improvements:
- Extract common array validation:
func (r *AssertableJson) validateArrayField(key string) ([]any, bool) { value, exists := r.getDecoded()[key] if !assert.True(r.t, exists, fmt.Sprintf("Property [%s] does not exist.", key)) { return nil, false } array, ok := value.([]any) if !assert.True(r.t, ok, fmt.Sprintf("Expected key [%s] to hold an array, but got %T", key, value)) { return nil, false } return array, true }
- Add error handling option in Each:
-func (r *AssertableJson) Each(key string, callback func(contractstesting.AssertableJSON)) contractstesting.AssertableJSON { +func (r *AssertableJson) Each(key string, callback func(contractstesting.AssertableJSON), continueOnError bool) contractstesting.AssertableJSON { // ... for _, item := range array { itemJson, err := json.Marshal(item) - if !assert.NoError(r.t, err) { + if err != nil { + if !continueOnError { + assert.NoError(r.t, err) + return r + } continue }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
mocks/testing/AssertableJSON.go
is excluded by!mocks/**
📒 Files selected for processing (3)
contracts/testing/assertable_json.go
(1 hunks)testing/assertable_json.go
(1 hunks)testing/assertable_json_test.go
(1 hunks)
🔇 Additional comments (4)
contracts/testing/assertable_json.go (1)
5-15
: Well-designed interface with comprehensive assertion capabilities.
The interface provides a robust set of methods for JSON assertions with:
- Consistent method chaining pattern
- Clear, single-responsibility methods
- Flexible validation capabilities
- Good coverage of common testing scenarios
This enhancement significantly improves the HTTP testing process by providing a more expressive and maintainable way to assert JSON responses.
testing/assertable_json_test.go (1)
1-9
: LGTM!
The package structure and imports are clean and appropriate for the testing context.
testing/assertable_json.go (2)
1-12
: LGTM! Package structure and imports are well-organized.
The package name and imports are appropriate for the testing utilities being implemented.
186-188
: LGTM! Helper method is appropriately implemented.
The getter method is simple and serves its purpose well.
return r.WithHeader("Authorization", fmt.Sprintf("%s %s", tt, token)) | ||
} | ||
|
||
func (r *TestRequest) WithBasicAuth(username, password string) contractstesting.TestRequest { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How to use it in Framework?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Usage of this be added in next PR. Some time api expect authorization. User can use these methods to test
Each(key string, callback func(AssertableJSON)) AssertableJSON | ||
First(key string, callback func(AssertableJSON)) AssertableJSON | ||
HasWithScope(key string, length int, callback func(AssertableJSON)) AssertableJSON |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sort them and add test cases for them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have already added test cases. Plz find in assertable_json_test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great
📑 Description
RelatedTo goravel/goravel#441
Summary by CodeRabbit
New Features
Bug Fixes
Tests
✅ Checks