fix: add function for proper bytearray to base64 encoding #3548
Conversation
Benjosh95
force-pushed
the
bf/userdata-base64-fix
branch
from
9160675 to
4502061
Compare
| @@ -37,3 +38,166 @@ func TestContainsInt(t *testing.T) { | |||
| t.Fatalf("Should not be contained") | |||
| } | |||
| } | |||
|
|
|||
| // Test struct for YAML conversion testing | |||
| type TestServer struct { | |||
There was a problem hiding this comment.
| type TestServer struct { | |
| type testServer struct { |
doesn't need to be exposed I guess
There was a problem hiding this comment.
...or just move the struct definition into the TestConvertByteArraysToBase64 func. You're only using it there if I didn't mess something up here. See the example below for reference.
| name: "normal case with byte arrays", | ||
| input: TestServer{ | ||
| Name: "test-server", | ||
| UserData: func() *[]byte { b := []byte("hello world"); return &b }(), |
There was a problem hiding this comment.
| UserData: func() *[]byte { b := []byte("hello world"); return &b }(), | |
| UserData: Ptr([]byte("hello world")) |
stackit-sdk-go/core/utils/utils.go
Lines 3 to 6 in 0b4638e
| } | ||
| } | ||
| case string: | ||
| if v != "" && v != "hello" && v != "test" && v != "nested" { |
| {"nil", nil, ""}, | ||
| {"map with []byte", map[string]interface{}{"data": []byte("hello")}, "aGVsbG8="}, | ||
| {"slice with []byte", []interface{}{[]byte("test")}, "dGVzdA=="}, | ||
| {"nested map", map[string]interface{}{"level": map[string]interface{}{"data": []byte("nested")}}, "bmVzdGVk"}, |
There was a problem hiding this comment.
Please format the input for the table test properly.
{
name: "nested map",
input: map[string]interface{}{
"level": map[string]interface{}{
"data": []byte("nested")
},
},
expected: "bmVzdGVk"
},Maybe it's just me, but I'm having a really hard time to get the goal of this PR in it's current state.
| input interface{} | ||
| expected string // check if []byte was converted to base64 string | ||
| }{ | ||
| {"nil", nil, ""}, |
There was a problem hiding this comment.
"nil"? Accident or intended?
| t.Run(tt.name, func(t *testing.T) { | ||
| convertByteArraysToBase64Recursive(tt.input) | ||
|
|
||
| if tt.expected == "" { |
There was a problem hiding this comment.
why is this skip of the further checks needed?
| name string | ||
| input interface{} | ||
| expectedFields map[string]interface{} | ||
| expectError bool |
There was a problem hiding this comment.
You have no test case in your table test where expectError is set to true. Then you don't have to have it here.
I would say either add some test cases which are expected to exit with an error or get rid of this input.
| @@ -37,3 +38,166 @@ func TestContainsInt(t *testing.T) { | |||
| t.Fatalf("Should not be contained") | |||
| } | |||
| } | |||
|
|
|||
| // Test struct for YAML conversion testing | |||
| type TestServer struct { | |||
There was a problem hiding this comment.
...or just move the struct definition into the TestConvertByteArraysToBase64 func. You're only using it there if I didn't mess something up here. See the example below for reference.
| for fieldName, actualValue := range result { | ||
| if _, expected := tt.expectedFields[fieldName]; !expected { | ||
| // Allow data field to exist even if not in expectedFields (for empty byte array case) | ||
| if fieldName == "data" && tt.name == "empty byte array case" { |
There was a problem hiding this comment.
Seems weird to me checking for test case names here. Any reason?
| t.Run(tt.name, func(t *testing.T) { | ||
| result, err := ConvertByteArraysToBase64(tt.input) | ||
|
|
||
| if tt.expectError { |
There was a problem hiding this comment.
| if tt.expectError { | |
| if (err != nil) != tt.wantErr { |
There was a problem hiding this comment.
This should handle all cases without needing 3 if conditions 😄
| if result != nil { | ||
| t.Fatalf("Expected nil result for nil input, got: %v", result) | ||
| } | ||
| return |
There was a problem hiding this comment.
Why stop the test case only because the input is nil?
Description
Jira-ticket: https://jira.schwarz/browse/STACKITSDK-139
Checklist
make fmtexamples/directory)make test(will be checked by CI)make lint(will be checked by CI)