-
Notifications
You must be signed in to change notification settings - Fork 4
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
tests: Fix CLI unit tests #1607
Changes from 7 commits
6e515b7
0f69950
0641703
c17887c
abaa875
62b35e8
e57edcc
65ebc34
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 |
---|---|---|
|
@@ -19,8 +19,8 @@ type Flags struct { | |
CIPush configmap.Value[bool] `configKey:"ci-push" configUsage:"create workflow to push change in main branch to the project"` | ||
} | ||
|
||
func DefaultFlags() *Flags { | ||
return &Flags{ | ||
func DefaultFlags() Flags { | ||
return Flags{ | ||
CI: configmap.NewValue(true), | ||
CIValidate: configmap.NewValue(true), | ||
CIPush: configmap.NewValue(true), | ||
|
@@ -35,9 +35,9 @@ func WorkflowsCommand(p dependencies.Provider) *cobra.Command { | |
Short: helpmsg.Read(`ci/workflows/short`), | ||
Long: helpmsg.Read(`ci/workflows/long`), | ||
RunE: func(cmd *cobra.Command, args []string) (err error) { | ||
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 { | ||
Comment on lines
-38
to
+40
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, select |
||
return err | ||
} | ||
|
||
|
@@ -52,7 +52,7 @@ func WorkflowsCommand(p dependencies.Provider) *cobra.Command { | |
} | ||
|
||
// Ask options | ||
options := AskWorkflowsOptions(*f, d.Dialogs()) | ||
options := AskWorkflowsOptions(f, d.Dialogs()) | ||
|
||
// Generate workflows | ||
return workflowsGen.Run(cmd.Context(), prj.Fs(), options, d) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,8 +22,8 @@ type Flags struct { | |
CIMainBranch configmap.Value[string] `configKey:"ci-main-branch" configUsage:"name of the main branch for push/pull workflows"` | ||
} | ||
|
||
func DefaultFlags() *Flags { | ||
return &Flags{ | ||
func DefaultFlags() Flags { | ||
return Flags{ | ||
CI: configmap.NewValue(true), | ||
Comment on lines
-25
to
27
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. There is no reason to return pointer here. |
||
CIValidate: configmap.NewValue(true), | ||
CIPull: configmap.NewValue(true), | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -67,7 +67,7 @@ func TestDialogs_AskInitOptions(t *testing.T) { | |
}() | ||
|
||
// Run | ||
opts, err := syncInit.AskInitOptions(context.Background(), dialog, d, *syncInit.DefaultFlags()) | ||
opts, err := syncInit.AskInitOptions(context.Background(), dialog, d, syncInit.DefaultFlags()) | ||
assert.NoError(t, err) | ||
assert.NoError(t, console.Tty().Close()) | ||
wg.Wait() | ||
|
@@ -109,10 +109,9 @@ func TestDialogs_AskInitOptions_No_CI(t *testing.T) { | |
o.Set("ci", "false") | ||
o.Set("branches", "main") | ||
|
||
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) | ||
Comment on lines
-112
to
+114
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 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
||
// Run | ||
opts, err := syncInit.AskInitOptions(context.Background(), dialog, d, f) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,8 +20,7 @@ import ( | |
) | ||
|
||
const ( | ||
SetByUnknown SetBy = iota | ||
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. Removed unused |
||
SetByDefault | ||
SetByDefault SetBy = iota | ||
SetByConfig | ||
SetByFlag | ||
SetByEnv | ||
|
@@ -192,7 +191,7 @@ func Bind(inputs BindConfig, targets ...any) error { | |
errs := errors.NewMultiError() | ||
for _, target := range targets { | ||
// Validate type | ||
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()) | ||
} | ||
Comment on lines
-195
to
196
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. Fixed check.+ |
||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,6 +30,10 @@ func NewValue[T any](v T) Value[T] { | |
return Value[T]{Value: v, SetBy: SetByDefault} | ||
} | ||
|
||
func NewValueWithOrigin[T any](v T, setBy SetBy) Value[T] { | ||
return Value[T]{Value: v, SetBy: setBy} | ||
} | ||
|
||
Comment on lines
+33
to
+36
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. See update tests. |
||
func (v fieldValue) IsSet() bool { | ||
return v.SetBy != SetByDefault | ||
} | ||
|
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.
Updated lib, to fix data race, but maybe we have to wait for
0.48.1
:open-telemetry/opentelemetry-go-contrib#4895