tests: Fix CLI unit tests #1607
Conversation
michaljurecko
changed the title
michaljurecko
force-pushed
the
michaljurecko-PSGO-447
branch
from
17455a9 to
5703f74
Compare
michaljurecko
force-pushed
the
michaljurecko-PSGO-447
branch
from
5703f74 to
40e4598
Compare
michaljurecko
force-pushed
the
michaljurecko-PSGO-447
branch
from
e82bb11 to
e57edcc
Compare
michaljurecko
changed the title
| func NewValueWithOrigin[T any](v T, setBy SetBy) Value[T] { | ||
| return Value[T]{Value: v, SetBy: setBy} | ||
| } | ||
|
|
There was a problem hiding this comment.
See update tests.
| @@ -20,8 +20,7 @@ import ( | |||
| ) | |||
|
|
|||
| const ( | |||
| SetByUnknown SetBy = iota | |||
There was a problem hiding this comment.
Removed unused SetByUnknown, so uninitialized Value has set SetByDefault, and IsSet() == false, by default.
| func DefaultFlags() *Flags { | ||
| return &Flags{ | ||
| func DefaultFlags() Flags { | ||
| return Flags{ | ||
| CI: configmap.NewValue(true), |
There was a problem hiding this comment.
There is no reason to return pointer here.
| if v := reflect.ValueOf(target); v.Kind() != reflect.Pointer && v.Type().Elem().Kind() != reflect.Pointer { | ||
| if v := reflect.ValueOf(target); v.Kind() != reflect.Pointer || v.Type().Elem().Kind() != reflect.Struct { | ||
| return errors.Errorf(`cannot bind to type "%s": expected a pointer to a struct`, v.Type().String()) | ||
| } |
There was a problem hiding this comment.
Fixed check.+
| f := syncInit.Flags{ | ||
| Branches: configmap.NewValue("main"), | ||
| CI: configmap.NewValue(false), | ||
| } | ||
| f := syncInit.DefaultFlags() | ||
| f.Branches = configmap.NewValueWithOrigin("main", configmap.SetByFlag) | ||
| f.CI = configmap.NewValueWithOrigin(false, configmap.SetByFlag) |
There was a problem hiding this comment.
I don't think this test could ever work 🤔 , you have to simulate that the value has been set by the flag.
There was a problem hiding this comment.
Yes, I found this out in several tests
| f := DefaultFlags() | ||
| // bind flags to struct | ||
| if err := configmap.Bind(utils.GetBindConfig(cmd.Flags(), args), f); err != nil { | ||
| // Bind flags to struct | ||
| f := Flags{} | ||
| if err := configmap.Bind(utils.GetBindConfig(cmd.Flags(), args), &f); err != nil { |
There was a problem hiding this comment.
Please, select f := DefaultFlags() approach or f := Flags{} approach and use it in all places.
| go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.46.0 | ||
| go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.48.0 | ||
| go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.44.0 |
There was a problem hiding this comment.
Updated lib, to fix data race, but maybe we have to wait for 0.48.1:
open-telemetry/opentelemetry-go-contrib#4895
| func (f *testFile) Close() error { | ||
| err := f.file.Close() | ||
| if f.CloseError != nil { | ||
| return f.CloseError | ||
| } | ||
| return f.file.Close() | ||
| return err | ||
| } |
There was a problem hiding this comment.
Fix one more test on Windows, always close the underlying file.
Jira: https://keboola.atlassian.net/browse/PSGO-447
Changes: