-
Notifications
You must be signed in to change notification settings - Fork 2
Support for uint #1
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
base: main
Are you sure you want to change the base?
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -101,9 +101,9 @@ Bob,bob@example.com,25 | |
|
|
||
| func TestComplexTypes(t *testing.T) { | ||
| // CSV data with various types | ||
| csvData := `name,created_at,active,score,count,rate,tags | ||
| John,2023-01-02T15:04:05Z,true,98.6,42,3.14,test;debug | ||
| Jane,2023-06-15T09:30:00Z,false,75.2,100,2.718,prod;live` | ||
| csvData := `name,created_at,active,score,count,rate,tags,rank | ||
| John,2023-01-02T15:04:05Z,true,98.6,42,3.14,test;debug,1 | ||
| Jane,2023-06-15T09:30:00Z,false,75.2,100,2.718,prod;live,42` | ||
|
|
||
| reader := strings.NewReader(csvData) | ||
|
|
||
|
|
@@ -125,6 +125,7 @@ Jane,2023-06-15T09:30:00Z,false,75.2,100,2.718,prod;live` | |
| Count: 42, | ||
| Rate: 3.14, | ||
| Tags: "test;debug", | ||
| Rank: 1, | ||
| }, | ||
| { | ||
| Name: "Jane", | ||
|
|
@@ -134,6 +135,7 @@ Jane,2023-06-15T09:30:00Z,false,75.2,100,2.718,prod;live` | |
| Count: 100, | ||
| Rate: 2.718, | ||
| Tags: "prod;live", | ||
| Rank: 42, | ||
| }, | ||
| } | ||
|
|
||
|
|
@@ -166,3 +168,48 @@ func TestCustomUnmarshaler(t *testing.T) { | |
| t.Errorf("Parsed results do not match expected.\nExpected: %+v\nGot: %+v", expected, results) | ||
| } | ||
| } | ||
|
|
||
| func TestInvalidData(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| panickedValue := func(f func()) (value any) { | ||
| defer func() { | ||
| value = recover() | ||
| }() | ||
|
|
||
| f() | ||
| return | ||
| } | ||
|
|
||
| test := func(t *testing.T, input string, expectedMessage string) { | ||
| reader := strings.NewReader(input) | ||
|
roshats marked this conversation as resolved.
|
||
|
|
||
| rb, err := rowboat.NewReader[ComplexRecord](reader) | ||
| if err != nil { | ||
| t.Fatalf("Failed to create RowBoat: %v", err) | ||
| } | ||
|
|
||
| actualValue := panickedValue(func() { | ||
| slices.Collect(rb.All()) | ||
| }) | ||
|
|
||
| if actualValue == nil { | ||
| t.Errorf("Expected a panic for input '%s', but it was parsed without error", input) | ||
| return | ||
| } | ||
|
|
||
| actualError, ok := actualValue.(error) | ||
| if !ok { | ||
| t.Errorf("Expected a panic for input '%s', but received a non-error value: %v", input, actualValue) | ||
|
roshats marked this conversation as resolved.
Outdated
|
||
| return | ||
| } | ||
|
|
||
| actualMessage := actualError.Error() | ||
| if expectedMessage != actualMessage { | ||
| t.Errorf("Expected a panic for input '%s' with error message '%s', but got '%s'", input, expectedMessage, actualMessage) | ||
|
roshats marked this conversation as resolved.
Outdated
|
||
| } | ||
| } | ||
|
|
||
| test(t, "score\nnot-float", `error setting field Score: strconv.ParseFloat: parsing "not-float": invalid syntax`) | ||
| test(t, "rank\n-1", `error setting field Rank: strconv.ParseUint: parsing "-1": invalid syntax`) | ||
|
Comment on lines
+208
to
+216
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here the test are too dependent from the implementation. You can see the error message is about strconv failing to parse. I'm doing this comment here, but I mean it's interesting to change the implementation, and so the tests There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Never mind ... The whole lib is like that... |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,6 +21,7 @@ type ComplexRecord struct { | |
| Count int `csv:"count"` | ||
| Rate float32 `csv:"rate"` | ||
| Tags string `csv:"tags"` | ||
| Rank uint `csv:"rank"` | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please use a uint8 so you could also use 1000 as an invalid value, and simulate an issue an int64 might have with a huge number
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've added overflow check for both uint and int. |
||
| } | ||
|
|
||
| type Point struct { | ||
|
|
||
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.
Could you add a test about sending a negative number via a CSV into a uint field ?
I feel like it could be useful to validate how the library behaves in such use case.
Also, it would help to avoid regression
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.
Thanks for the comment. I've added a test for invalid values.