Skip to content
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

chore: remove boolean without value in args #767

Merged
merged 5 commits into from
Mar 12, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 18 additions & 3 deletions internal/args/args.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package args

import (
"fmt"
"reflect"
"regexp"
"strings"
Expand Down Expand Up @@ -37,15 +38,15 @@ var (
// ["arg1=1", "arg2=2", "arg3"] => {"arg1": "1", "arg2": "2", "arg3":"" }
func SplitRawMap(rawArgs []string) map[string]struct{} {
argsMap := map[string]struct{}{}
for _, arg := range SplitRaw(rawArgs) {
for _, arg := range SplitRawNoError(rawArgs) {
argsMap[arg[0]] = struct{}{}
}
return argsMap
}

// SplitRaw creates a slice that maps arg names to their values.
// SplitRawNoError creates a slice that maps arg names to their values.
// ["arg1=1", "arg2=2", "arg3"] => { {"arg1", "1"}, {"arg2", "2"}, {"arg3",""} }
func SplitRaw(rawArgs []string) [][2]string {
func SplitRawNoError(rawArgs []string) [][2]string {
keyValue := [][2]string{}
for _, arg := range rawArgs {
tmp := strings.SplitN(arg, "=", 2)
Expand All @@ -57,6 +58,20 @@ func SplitRaw(rawArgs []string) [][2]string {
return keyValue
}

// SplitRaw creates a slice that maps arg names to their values.
// ["arg1=1", "arg2=2", "arg3"] => { {"arg1", "1"}, {"arg2", "2"}, {"arg3",""} }
func SplitRaw(rawArgs []string) ([][2]string, error) {
keyValue := [][2]string{}
for _, arg := range rawArgs {
tmp := strings.SplitN(arg, "=", 2)
if len(tmp) < 2 || (len(tmp) == 2 && strings.HasSuffix(arg, "=")) {
return nil, fmt.Errorf("arg '%v' must have a value", tmp[0])
}
keyValue = append(keyValue, [2]string{tmp[0], tmp[1]})
}
return keyValue, nil
}

func getInterfaceFromReflectValue(reflectValue reflect.Value) interface{} {
i := reflectValue.Interface()
if reflectValue.CanAddr() {
Expand Down
5 changes: 4 additions & 1 deletion internal/args/unmarshal.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,10 @@ func UnmarshalStruct(args []string, data interface{}) error {

// Map arg names to their values.
// ["arg1=1", "arg2=2", "arg3"] => [ ["arg1","1"], ["arg2","2"], ["arg3",""] ]
argsSlice := SplitRaw(args)
argsSlice, err := SplitRaw(args)
if err != nil {
return err
}

processedArgNames := make(map[string]bool)

Expand Down
52 changes: 51 additions & 1 deletion internal/args/unmarshal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func TestUnmarshalStruct(t *testing.T) {
"float32=3.2",
"float64=6.4",
"string-ptr=test",
"bool",
"bool=true",
},
expected: &Basic{
String: "test",
Expand Down Expand Up @@ -430,6 +430,56 @@ func TestUnmarshalStruct(t *testing.T) {
IPv6: "ipv6",
},
}))

t.Run("bool-without-equal", run(TestCase{
args: []string{
"bool",
},
data: &Basic{},
error: "arg 'bool' must have a value",
}))

t.Run("bool-without-value", run(TestCase{
args: []string{
"bool=",
},
data: &Basic{},
error: "arg 'bool' must have a value",
}))

t.Run("string-without-equal", run(TestCase{
args: []string{
"string",
},
data: &Basic{},
error: "arg 'string' must have a value",
}))

t.Run("string-without-value", run(TestCase{
args: []string{
"string=",
},
data: &Basic{},
error: "arg 'string' must have a value",
}))

t.Run("strings-without-equal", run(TestCase{
args: []string{
"strings",
},
data: &Slice{},
error: "arg 'strings' must have a value",
}))

// TODO: decide if we want to fix it
// According to specs, this should not trigger an error
t.Run("strings-without-value", run(TestCase{
args: []string{
"strings=",
},
data: &Slice{},
error: "arg 'strings' must have a value",
}))
}
func TestIsUmarshalableValue(t *testing.T) {
type TestCase struct {
Expand Down
14 changes: 14 additions & 0 deletions internal/core/cobra_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,20 @@ func cobraRun(ctx context.Context, cmd *Command) func(*cobra.Command, []string)
// Apply default values on missing args.
rawArgs = ApplyDefaultValues(cmd.ArgSpecs, rawArgs)

// Check args exist valid
argsSlice := args.SplitRawNoError(rawArgs)
for _, arguments := range argsSlice {
// TODO: handle args such as tags.index
if cmd.ArgSpecs.GetByName(arguments[0]) == nil &&
cmd.ArgSpecs.GetByName(arguments[0]+".{index}") == nil &&
!strings.Contains(arguments[0], ".") {
return handleUnmarshalErrors(cmd, &args.UnmarshalArgError{
Err: &args.UnknownArgError{},
ArgName: arguments[0],
})
}
}

// Unmarshal args.
// After that we are done working with rawArgs
// and will be working with cmdArgs.
Expand Down
8 changes: 3 additions & 5 deletions internal/namespaces/instance/v1/args_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package instance

import (
"fmt"
"testing"

"github.com/scaleway/scaleway-cli/internal/args"
Expand Down Expand Up @@ -55,12 +56,9 @@ func TestNullableStringValueUnmarshal(t *testing.T) {
},
data: &NullableStringValueRequest{},
expectedStruct: &NullableStringValueRequest{
Reverse: &instance.NullableStringValue{
Null: true,
Value: "",
},
Reverse: (*instance.NullableStringValue)(nil),
},
expectedError: nil,
expectedError: fmt.Errorf("arg 'reverse' must have a value"),
}))
}

Expand Down
32 changes: 16 additions & 16 deletions internal/namespaces/instance/v1/custom_server_create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (

// deleteServerAfterFunc deletes the created server and its attached volumes and IPs.
func deleteServerAfterFunc(ctx *core.AfterFuncCtx) error {
ctx.ExecuteCmd("scw instance server delete with-volumes=all with-ip force-shutdown server-id=" + ctx.CmdResult.(*instance.Server).ID)
ctx.ExecuteCmd("scw instance server delete with-volumes=all with-ip=true force-shutdown=true server-id=" + ctx.CmdResult.(*instance.Server).ID)
return nil
}

Expand All @@ -24,7 +24,7 @@ func Test_CreateServer(t *testing.T) {
t.Run("Simple", func(t *testing.T) {
t.Run("Default", core.Test(&core.TestConfig{
Commands: GetCommands(),
Cmd: "scw instance server create image=ubuntu_bionic stopped",
Cmd: "scw instance server create image=ubuntu_bionic stopped=true",
Check: core.TestCheckCombine(
core.TestCheckGolden(),
func(t *testing.T, ctx *core.CheckFuncCtx) {
Expand All @@ -37,7 +37,7 @@ func Test_CreateServer(t *testing.T) {

t.Run("GP1-XS", core.Test(&core.TestConfig{
Commands: GetCommands(),
Cmd: "scw instance server create type=GP1-XS image=ubuntu_bionic stopped",
Cmd: "scw instance server create type=GP1-XS image=ubuntu_bionic stopped=true",
Check: core.TestCheckCombine(
func(t *testing.T, ctx *core.CheckFuncCtx) {
assert.Equal(t, "GP1-XS", ctx.Result.(*instance.Server).CommercialType)
Expand All @@ -49,7 +49,7 @@ func Test_CreateServer(t *testing.T) {

t.Run("With name", core.Test(&core.TestConfig{
Commands: GetCommands(),
Cmd: "scw instance server create image=ubuntu_bionic name=yo stopped",
Cmd: "scw instance server create image=ubuntu_bionic name=yo stopped=true",
Check: core.TestCheckCombine(
func(t *testing.T, ctx *core.CheckFuncCtx) {
assert.Equal(t, "yo", ctx.Result.(*instance.Server).Name)
Expand All @@ -73,7 +73,7 @@ func Test_CreateServer(t *testing.T) {

t.Run("With bootscript", core.Test(&core.TestConfig{
Commands: GetCommands(),
Cmd: "scw instance server create image=ubuntu_bionic bootscript-id=eb760e3c-30d8-49a3-b3ad-ad10c3aa440b stopped",
Cmd: "scw instance server create image=ubuntu_bionic bootscript-id=eb760e3c-30d8-49a3-b3ad-ad10c3aa440b stopped=true",
Check: core.TestCheckCombine(
func(t *testing.T, ctx *core.CheckFuncCtx) {
assert.Equal(t, "eb760e3c-30d8-49a3-b3ad-ad10c3aa440b", ctx.Result.(*instance.Server).Bootscript.ID)
Expand All @@ -85,7 +85,7 @@ func Test_CreateServer(t *testing.T) {

t.Run("Image UUID", core.Test(&core.TestConfig{
Commands: GetCommands(),
Cmd: "scw instance server create image=f974feac-abae-4365-b988-8ec7d1cec10d stopped",
Cmd: "scw instance server create image=f974feac-abae-4365-b988-8ec7d1cec10d stopped=true",
Check: core.TestCheckCombine(
func(t *testing.T, ctx *core.CheckFuncCtx) {
assert.Equal(t, "Ubuntu Bionic Beaver", ctx.Result.(*instance.Server).Image.Name)
Expand All @@ -97,7 +97,7 @@ func Test_CreateServer(t *testing.T) {

t.Run("Tags", core.Test(&core.TestConfig{
Commands: GetCommands(),
Cmd: "scw instance server create image=ubuntu_bionic tags.0=prod tags.1=blue stopped",
Cmd: "scw instance server create image=ubuntu_bionic tags.0=prod tags.1=blue stopped=true",
Check: core.TestCheckCombine(
func(t *testing.T, ctx *core.CheckFuncCtx) {
assert.Equal(t, "prod", ctx.Result.(*instance.Server).Tags[0])
Expand All @@ -116,7 +116,7 @@ func Test_CreateServer(t *testing.T) {

t.Run("valid single local volume", core.Test(&core.TestConfig{
Commands: GetCommands(),
Cmd: "scw instance server create image=ubuntu_bionic root-volume=local:20GB stopped",
Cmd: "scw instance server create image=ubuntu_bionic root-volume=local:20GB stopped=true",
Check: core.TestCheckCombine(
func(t *testing.T, ctx *core.CheckFuncCtx) {
assert.Equal(t, 20*scw.GB, ctx.Result.(*instance.Server).Volumes["0"].Size)
Expand All @@ -128,7 +128,7 @@ func Test_CreateServer(t *testing.T) {

t.Run("valid double local volumes", core.Test(&core.TestConfig{
Commands: GetCommands(),
Cmd: "scw instance server create image=ubuntu_bionic root-volume=local:10GB additional-volumes.0=l:10G stopped",
Cmd: "scw instance server create image=ubuntu_bionic root-volume=local:10GB additional-volumes.0=l:10G stopped=true",
Check: core.TestCheckCombine(
func(t *testing.T, ctx *core.CheckFuncCtx) {
assert.Equal(t, 10*scw.GB, ctx.Result.(*instance.Server).Volumes["0"].Size)
Expand All @@ -141,7 +141,7 @@ func Test_CreateServer(t *testing.T) {

t.Run("valid additional block volumes", core.Test(&core.TestConfig{
Commands: GetCommands(),
Cmd: "scw instance server create image=ubuntu_bionic additional-volumes.0=b:1G additional-volumes.1=b:5G additional-volumes.2=b:10G stopped",
Cmd: "scw instance server create image=ubuntu_bionic additional-volumes.0=b:1G additional-volumes.1=b:5G additional-volumes.2=b:10G stopped=true",
Check: core.TestCheckCombine(
func(t *testing.T, ctx *core.CheckFuncCtx) {
assert.Equal(t, 1*scw.GB, ctx.Result.(*instance.Server).Volumes["1"].Size)
Expand All @@ -161,7 +161,7 @@ func Test_CreateServer(t *testing.T) {

t.Run("explicit new IP", core.Test(&core.TestConfig{
Commands: GetCommands(),
Cmd: "scw instance server create image=ubuntu_bionic ip=new stopped",
Cmd: "scw instance server create image=ubuntu_bionic ip=new stopped=true",
Check: core.TestCheckCombine(
func(t *testing.T, ctx *core.CheckFuncCtx) {
assert.NotEmpty(t, ctx.Result.(*instance.Server).PublicIP.Address)
Expand Down Expand Up @@ -189,7 +189,7 @@ func Test_CreateServer(t *testing.T) {
t.Run("existing IP", core.Test(&core.TestConfig{
Commands: GetCommands(),
BeforeFunc: createIP("IP"),
Cmd: "scw instance server create image=ubuntu_bionic ip={{ .IP.Address }} stopped",
Cmd: "scw instance server create image=ubuntu_bionic ip={{ .IP.Address }} stopped=true",
Check: core.TestCheckCombine(
func(t *testing.T, ctx *core.CheckFuncCtx) {
assert.NotEmpty(t, ctx.Result.(*instance.Server).PublicIP.Address)
Expand All @@ -203,7 +203,7 @@ func Test_CreateServer(t *testing.T) {
t.Run("existing IP ID", core.Test(&core.TestConfig{
Commands: GetCommands(),
BeforeFunc: createIP("IP"),
Cmd: "scw instance server create image=ubuntu_bionic ip={{ .IP.ID }} stopped",
Cmd: "scw instance server create image=ubuntu_bionic ip={{ .IP.ID }} stopped=true",
Check: core.TestCheckCombine(
func(t *testing.T, ctx *core.CheckFuncCtx) {
assert.NotEmpty(t, ctx.Result.(*instance.Server).PublicIP.Address)
Expand All @@ -216,7 +216,7 @@ func Test_CreateServer(t *testing.T) {

t.Run("with ipv6", core.Test(&core.TestConfig{
Commands: GetCommands(),
Cmd: "scw instance server create image=ubuntu_bionic ipv6 -w", // IPv6 is created at runtime
Cmd: "scw instance server create image=ubuntu_bionic ipv6=true -w", // IPv6 is created at runtime
Check: core.TestCheckCombine(
func(t *testing.T, ctx *core.CheckFuncCtx) {
assert.NotEmpty(t, ctx.Result.(*instance.Server).IPv6.Address)
Expand Down Expand Up @@ -363,8 +363,8 @@ func Test_CreateServerErrors(t *testing.T) {

t.Run("Error: already attached additional volume ID", core.Test(&core.TestConfig{
Commands: GetCommands(),
BeforeFunc: core.ExecStoreBeforeCmd("Server", "scw instance server create name=cli-test image=ubuntu_bionic root-volume=l:10G additional-volumes.0=l:10G stopped"),
Cmd: `scw instance server create image=ubuntu_bionic root-volume=l:10G additional-volumes.0={{ (index .Server.Volumes "1").ID }} stopped`,
BeforeFunc: core.ExecStoreBeforeCmd("Server", "scw instance server create name=cli-test image=ubuntu_bionic root-volume=l:10G additional-volumes.0=l:10G stopped=true"),
Cmd: `scw instance server create image=ubuntu_bionic root-volume=l:10G additional-volumes.0={{ (index .Server.Volumes "1").ID }} stopped=true`,
Check: core.TestCheckCombine(
core.TestCheckGolden(),
core.TestCheckExitCode(1),
Expand Down
22 changes: 4 additions & 18 deletions internal/namespaces/instance/v1/instance_cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,21 +86,7 @@ func Test_ServerUpdate(t *testing.T) {
t.Run("No initial placement group & placement-group-id=none", core.Test(&core.TestConfig{
Commands: GetCommands(),
BeforeFunc: createServer("Server"),
Cmd: "scw instance server update server-id={{ .Server.ID }} placement-group=none",
Check: core.TestCheckCombine(
func(t *testing.T, ctx *core.CheckFuncCtx) {
require.NoError(t, ctx.Err)
assert.Nil(t, ctx.Result.(*instance.UpdateServerResponse).Server.PlacementGroup)
},
core.TestCheckExitCode(0),
),
AfterFunc: deleteServer("Server"),
}))

t.Run(`No initial placement group & placement-group-id=`, core.Test(&core.TestConfig{
Commands: GetCommands(),
BeforeFunc: createServer("Server"),
Cmd: `scw instance server update server-id={{ .Server.ID }} placement-group=`,
Cmd: "scw instance server update server-id={{ .Server.ID }} placement-group-id=none",
Check: core.TestCheckCombine(
func(t *testing.T, ctx *core.CheckFuncCtx) {
require.NoError(t, ctx.Err)
Expand Down Expand Up @@ -160,7 +146,7 @@ func Test_ServerUpdate(t *testing.T) {
Commands: GetCommands(),
BeforeFunc: core.BeforeFuncCombine(
createPlacementGroup("PlacementGroup"),
core.ExecStoreBeforeCmd("Server", "scw instance server create image=ubuntu-bionic placement-group-id={{ .PlacementGroup.ID }} stopped"),
core.ExecStoreBeforeCmd("Server", "scw instance server create image=ubuntu-bionic placement-group-id={{ .PlacementGroup.ID }} stopped=true"),
),
Cmd: `scw instance server update server-id={{ .Server.ID }} placement-group-id=none`,
Check: core.TestCheckCombine(
Expand All @@ -180,7 +166,7 @@ func Test_ServerUpdate(t *testing.T) {
Commands: GetCommands(),
BeforeFunc: core.BeforeFuncCombine(
createPlacementGroup("PlacementGroup"),
core.ExecStoreBeforeCmd("Server", "scw instance server create image=ubuntu-bionic placement-group-id={{ .PlacementGroup.ID }} stopped"),
core.ExecStoreBeforeCmd("Server", "scw instance server create image=ubuntu-bionic placement-group-id={{ .PlacementGroup.ID }} stopped=true"),
),
Cmd: `scw instance server update server-id={{ .Server.ID }} placement-group-id={{ .PlacementGroup.ID }}`,
Check: core.TestCheckCombine(
Expand All @@ -204,7 +190,7 @@ func Test_ServerUpdate(t *testing.T) {
BeforeFunc: core.BeforeFuncCombine(
createPlacementGroup("PlacementGroup1"),
createPlacementGroup("PlacementGroup2"),
core.ExecStoreBeforeCmd("Server", "scw instance server create image=ubuntu-bionic placement-group-id={{ .PlacementGroup1.ID }} stopped"),
core.ExecStoreBeforeCmd("Server", "scw instance server create image=ubuntu-bionic placement-group-id={{ .PlacementGroup1.ID }} stopped=true"),
),
Cmd: `scw instance server update server-id={{ .Server.ID }} placement-group-id={{ .PlacementGroup2.ID }}`,
Check: core.TestCheckCombine(
Expand Down