feat(group): add 'madb group remove' command
Change-Id: Idbeb00d7d092c87aa02db849660a81450a516e6e
Ref: #2
diff --git a/doc.go b/doc.go
index 76df444..31b6310 100644
--- a/doc.go
+++ b/doc.go
@@ -231,11 +231,12 @@
madb group [flags] <command>
The madb group commands are:
- add Add devices to a device group
+ add Add members to a device group
+ remove Remove members from a device group
-Madb group add - Add devices to a device group
+Madb group add - Add members to a device group
-Adds devices to a device group. This command also creates the group, if the
+Adds members to a device group. This command also creates the group, if the
group does not exist yet. The device group can be used when specifying devices
in any madb commands.
@@ -254,6 +255,20 @@
<member> is a member specifier, which can be one of device serial, qualifier,
device index (e.g., '@1', '@2'), device nickname, or another device group.
+Madb group remove - Remove members from a device group
+
+Removes members from an existing device group. If there are no remaining members
+after that, the group gets deleted.
+
+Usage:
+ madb group remove [flags] <group_name> <member1> [<member2> ...]
+
+<group_name> is an alpha-numeric string with no special characters or spaces.
+This name must be an existing device group name.
+
+<member> is a member specifier, which can be one of device serial, qualifier,
+device index (e.g., '@1', '@2'), device nickname, or another device group.
+
Madb install - Install your app on all devices
Installs your app on all devices.
diff --git a/group.go b/group.go
index 6224be5..dc1e563 100644
--- a/group.go
+++ b/group.go
@@ -13,7 +13,6 @@
)
// TODO(youngseokyoon): implement the following sub-commands.
-// - remove: remove members from an existing group
// - rename: rename a group
// - delete: delete a group
// - list: list all the groups and their members
@@ -22,7 +21,7 @@
// TODO(youngseokyoon): use the groups for filtering devices.
var cmdMadbGroup = &cmdline.Command{
- Children: []*cmdline.Command{cmdMadbGroupAdd},
+ Children: []*cmdline.Command{cmdMadbGroupAdd, cmdMadbGroupRemove},
Name: "group",
DontInheritFlags: true,
Short: "Manage device groups",
@@ -36,9 +35,9 @@
var cmdMadbGroupAdd = &cmdline.Command{
Runner: subCommandRunnerWithFilepath{runMadbGroupAdd, getDefaultConfigFilePath},
Name: "add",
- Short: "Add devices to a device group",
+ Short: "Add members to a device group",
Long: `
-Adds devices to a device group. This command also creates the group, if the
+Adds members to a device group. This command also creates the group, if the
group does not exist yet. The device group can be used when specifying devices
in any madb commands.
@@ -93,6 +92,54 @@
return writeConfig(cfg, filename)
}
+var cmdMadbGroupRemove = &cmdline.Command{
+ Runner: subCommandRunnerWithFilepath{runMadbGroupRemove, getDefaultConfigFilePath},
+ Name: "remove",
+ Short: "Remove members from a device group",
+ Long: `
+Removes members from an existing device group. If there are no remaining members
+after that, the group gets deleted.
+`,
+ ArgsName: "<group_name> <member1> [<member2> ...]",
+ ArgsLong: `
+<group_name> is an alpha-numeric string with no special characters or spaces.
+This name must be an existing device group name.
+
+<member> is a member specifier, which can be one of device serial, qualifier,
+device index (e.g., '@1', '@2'), device nickname, or another device group.
+`,
+}
+
+func runMadbGroupRemove(env *cmdline.Env, args []string, filename string) error {
+ // Check if the arguments are valid.
+ if len(args) < 2 {
+ return env.UsageErrorf("There must be at least two arguments.")
+ }
+
+ groupName := args[0]
+ if !isValidName(groupName) {
+ return fmt.Errorf("Not a valid group name: %q", groupName)
+ }
+
+ cfg, err := readConfig(filename)
+ if err != nil {
+ return err
+ }
+ if !isGroupName(groupName, cfg) {
+ return fmt.Errorf("Not an existing group name: %q", groupName)
+ }
+
+ members := removeDuplicates(args[1:])
+ oldMembers := cfg.Groups[groupName]
+ cfg.Groups[groupName] = subtractSlices(oldMembers, members)
+
+ if len(cfg.Groups[groupName]) == 0 {
+ delete(cfg.Groups, groupName)
+ }
+
+ return writeConfig(cfg, filename)
+}
+
// isValidMember takes a member string given as an argument, and returns nil
// when the member string is valid. Otherwise, an error is returned inicating
// the reason why the given member string is not valid.
@@ -125,3 +172,22 @@
return result
}
+
+// subtractSlices takes two slices and returns a new slice formed by removing
+// all the elements in s2 from s1.
+func subtractSlices(s1, s2 []string) []string {
+ result := make([]string, 0, len(s1))
+
+ m := map[string]bool{}
+ for _, e2 := range s2 {
+ m[e2] = true
+ }
+
+ for _, e1 := range s1 {
+ if !m[e1] {
+ result = append(result, e1)
+ }
+ }
+
+ return result
+}
diff --git a/group_test.go b/group_test.go
index 49dd946..a364714 100644
--- a/group_test.go
+++ b/group_test.go
@@ -8,72 +8,26 @@
"os"
"reflect"
"testing"
+
+ "v.io/x/lib/cmdline"
)
-func TestMadbGroupAdd(t *testing.T) {
- tests := [][]struct {
- input []string
- want map[string][]string
- errExpected bool
- }{
- {
- {
- []string{"GROUP1", "SERIAL1", "NICKNAME1", "NICKNAME2"},
- map[string][]string{"GROUP1": []string{"SERIAL1", "NICKNAME1", "NICKNAME2"}},
- false,
- },
- {
- []string{"GROUP1", "SERIAL2", "NICKNAME3", "NICKNAME1"},
- map[string][]string{"GROUP1": []string{"SERIAL1", "NICKNAME1", "NICKNAME2", "SERIAL2", "NICKNAME3"}},
- false,
- },
- {
- []string{"GROUP2", "SERIAL1", "SERIAL4", "NICKNAME1"},
- map[string][]string{
- "GROUP1": []string{"SERIAL1", "NICKNAME1", "NICKNAME2", "SERIAL2", "NICKNAME3"},
- "GROUP2": []string{"SERIAL1", "SERIAL4", "NICKNAME1"},
- },
- false,
- },
- {
- []string{"GROUP2", "GROUP1"},
- map[string][]string{
- "GROUP1": []string{"SERIAL1", "NICKNAME1", "NICKNAME2", "SERIAL2", "NICKNAME3"},
- "GROUP2": []string{"SERIAL1", "SERIAL4", "NICKNAME1", "GROUP1"},
- },
- false,
- },
- },
- {
- // Duplicate members should be removed even within the arguments.
- {
- []string{"GROUP1", "SERIAL1", "NICKNAME1", "NICKNAME1"},
- map[string][]string{"GROUP1": []string{"SERIAL1", "NICKNAME1"}},
- false,
- },
- },
- {
- // Invalid mamber name
- {
- []string{"GROUP1", "#INVALID_NAME#"},
- map[string][]string{},
- true,
- },
- // Invalid member index
- {
- []string{"GROUP1", "@ABC"},
- map[string][]string{},
- true,
- },
- },
- }
+type testCase struct {
+ runnerFunc func(*cmdline.Env, []string, string) error
+ input []string
+ want map[string][]string
+ errExpected bool
+}
+type testSequence []testCase
+
+func runGroupTests(t *testing.T, tests []testSequence) {
for i, testSuite := range tests {
filename := tempFilename(t)
defer os.Remove(filename)
for j, test := range testSuite {
- err := runMadbGroupAdd(nil, test.input, filename)
+ err := test.runnerFunc(cmdline.EnvFromOS(), test.input, filename)
if test.errExpected != (err != nil) {
t.Fatalf("error expected for tests[%v][%v]: %v, got: %v", i, j, test.errExpected, err)
}
@@ -89,17 +43,184 @@
}
}
+func TestMadbGroupAdd(t *testing.T) {
+ tests := []testSequence{
+ {
+ {
+ runMadbGroupAdd,
+ []string{},
+ map[string][]string{},
+ true,
+ },
+ },
+ {
+ {
+ runMadbGroupAdd,
+ []string{"GROUP1"},
+ map[string][]string{},
+ true,
+ },
+ },
+ {
+ {
+ runMadbGroupAdd,
+ []string{"GROUP1", "SERIAL1", "NICKNAME1", "NICKNAME2"},
+ map[string][]string{"GROUP1": []string{"SERIAL1", "NICKNAME1", "NICKNAME2"}},
+ false,
+ },
+ {
+ runMadbGroupAdd,
+ []string{"GROUP1", "SERIAL2", "NICKNAME3", "NICKNAME1"},
+ map[string][]string{"GROUP1": []string{"SERIAL1", "NICKNAME1", "NICKNAME2", "SERIAL2", "NICKNAME3"}},
+ false,
+ },
+ {
+ runMadbGroupAdd,
+ []string{"GROUP2", "SERIAL1", "SERIAL4", "NICKNAME1"},
+ map[string][]string{
+ "GROUP1": []string{"SERIAL1", "NICKNAME1", "NICKNAME2", "SERIAL2", "NICKNAME3"},
+ "GROUP2": []string{"SERIAL1", "SERIAL4", "NICKNAME1"},
+ },
+ false,
+ },
+ {
+ runMadbGroupAdd,
+ []string{"GROUP2", "GROUP1"},
+ map[string][]string{
+ "GROUP1": []string{"SERIAL1", "NICKNAME1", "NICKNAME2", "SERIAL2", "NICKNAME3"},
+ "GROUP2": []string{"SERIAL1", "SERIAL4", "NICKNAME1", "GROUP1"},
+ },
+ false,
+ },
+ },
+ {
+ // Duplicate members should be removed even within the arguments.
+ {
+ runMadbGroupAdd,
+ []string{"GROUP1", "SERIAL1", "NICKNAME1", "NICKNAME1"},
+ map[string][]string{"GROUP1": []string{"SERIAL1", "NICKNAME1"}},
+ false,
+ },
+ },
+ {
+ // Invalid mamber name
+ {
+ runMadbGroupAdd,
+ []string{"GROUP1", "#INVALID_NAME#"},
+ map[string][]string{},
+ true,
+ },
+ // Invalid member index
+ {
+ runMadbGroupAdd,
+ []string{"GROUP1", "@ABC"},
+ map[string][]string{},
+ true,
+ },
+ },
+ }
+
+ runGroupTests(t, tests)
+}
+
func TestMadbGroupAddNameConflict(t *testing.T) {
- filename := tempFilename(t)
- defer os.Remove(filename)
-
- err := runMadbNameSet(nil, []string{"SERIAL1", "NICKNAME1"}, filename)
- if err != nil {
- t.Fatal(err)
+ tests := []testSequence{
+ {
+ {
+ runMadbNameSet,
+ []string{"SERIAL1", "NICKNAME1"},
+ map[string][]string{},
+ false,
+ },
+ {
+ runMadbGroupAdd,
+ []string{"NICKNAME1", "SERIAL2"},
+ map[string][]string{},
+ true,
+ },
+ },
}
- err = runMadbGroupAdd(nil, []string{"NICKNAME1", "SERIAL2"}, filename)
- if err == nil {
- t.Fatalf("error expected but got: %v", err)
+ runGroupTests(t, tests)
+}
+
+func TestMadbGroupRemove(t *testing.T) {
+ tests := []testSequence{
+ {
+ {
+ runMadbGroupRemove,
+ []string{},
+ map[string][]string{},
+ true,
+ },
+ },
+ {
+ {
+ runMadbGroupRemove,
+ []string{"GROUP1"},
+ map[string][]string{},
+ true,
+ },
+ },
+ {
+ {
+ runMadbGroupAdd,
+ []string{"GROUP1", "SERIAL1", "NICKNAME1", "NICKNAME2", "SERIAL2", "NICKNAME3"},
+ map[string][]string{"GROUP1": []string{"SERIAL1", "NICKNAME1", "NICKNAME2", "SERIAL2", "NICKNAME3"}},
+ false,
+ },
+ {
+ runMadbGroupAdd,
+ []string{"GROUP2", "SERIAL3", "NICKNAME4"},
+ map[string][]string{
+ "GROUP1": []string{"SERIAL1", "NICKNAME1", "NICKNAME2", "SERIAL2", "NICKNAME3"},
+ "GROUP2": []string{"SERIAL3", "NICKNAME4"},
+ },
+ false,
+ },
+ {
+ runMadbGroupRemove,
+ []string{"GROUP1", "SERIAL1", "NICKNAME3"},
+ map[string][]string{
+ "GROUP1": []string{"NICKNAME1", "NICKNAME2", "SERIAL2"},
+ "GROUP2": []string{"SERIAL3", "NICKNAME4"},
+ },
+ false,
+ },
+ {
+ runMadbGroupRemove,
+ []string{"GROUP2", "SERIAL3"},
+ map[string][]string{
+ "GROUP1": []string{"NICKNAME1", "NICKNAME2", "SERIAL2"},
+ "GROUP2": []string{"NICKNAME4"},
+ },
+ false,
+ },
+ {
+ runMadbGroupRemove,
+ []string{"GROUP3", "SERIAL3"},
+ map[string][]string{
+ "GROUP1": []string{"NICKNAME1", "NICKNAME2", "SERIAL2"},
+ "GROUP2": []string{"NICKNAME4"},
+ },
+ true,
+ },
+ {
+ runMadbGroupRemove,
+ []string{"GROUP2", "NICKNAME4"},
+ map[string][]string{
+ "GROUP1": []string{"NICKNAME1", "NICKNAME2", "SERIAL2"},
+ },
+ false,
+ },
+ {
+ runMadbGroupRemove,
+ []string{"GROUP1", "NICKNAME2", "SERIAL2", "NICKNAME1"},
+ map[string][]string{},
+ false,
+ },
+ },
}
+
+ runGroupTests(t, tests)
}
