feat(group): add 'madb group add' command
This is the first pass of implementing the 'madb group' commands, and
it adds 'madb group add <group_name> <members...>' command. The group
definition is stored in the config file.
Change-Id: I55d76c3f5912136ce38b13f498b7dd2f9c1d12d1
Ref: #2
diff --git a/doc.go b/doc.go
index c8e6186..76df444 100644
--- a/doc.go
+++ b/doc.go
@@ -19,6 +19,7 @@
exec Run the provided adb command on all devices and emulators
concurrently
extern Run the provided external command for all devices
+ group Manage device groups
install Install your app on all devices
name Manage device nicknames
shell Run the provided adb shell command on all devices and emulators
@@ -220,6 +221,39 @@
-seq=false
Run the command sequentially, instead of running it in parallel.
+Madb group - Manage device groups
+
+Manages device groups, each of which can have one or more device members. The
+device groups can be used for specifying the target devices of other madb
+commands.
+
+Usage:
+ madb group [flags] <command>
+
+The madb group commands are:
+ add Add devices to a device group
+
+Madb group add - Add devices to a device group
+
+Adds devices 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.
+
+When creating a new device group with this command, the provided name must not
+conflict with an existing device nickname (see: madb help name set).
+
+A group can contain another device group, in which case all the members of the
+other group will also be considered as members of the current group.
+
+Usage:
+ madb group add [flags] <group_name> <member1> [<member2> ...]
+
+<group_name> is an alpha-numeric string with no special characters or spaces.
+This name must not be an existing device nickname.
+
+<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
new file mode 100644
index 0000000..6224be5
--- /dev/null
+++ b/group.go
@@ -0,0 +1,127 @@
+// Copyright 2016 The Vanadium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package main
+
+import (
+ "fmt"
+ "strconv"
+ "strings"
+
+ "v.io/x/lib/cmdline"
+)
+
+// 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
+// - clear-all: delete all the existing device groups
+
+// TODO(youngseokyoon): use the groups for filtering devices.
+
+var cmdMadbGroup = &cmdline.Command{
+ Children: []*cmdline.Command{cmdMadbGroupAdd},
+ Name: "group",
+ DontInheritFlags: true,
+ Short: "Manage device groups",
+ Long: `
+Manages device groups, each of which can have one or more device members. The
+device groups can be used for specifying the target devices of other madb
+commands.
+`,
+}
+
+var cmdMadbGroupAdd = &cmdline.Command{
+ Runner: subCommandRunnerWithFilepath{runMadbGroupAdd, getDefaultConfigFilePath},
+ Name: "add",
+ Short: "Add devices to a device group",
+ Long: `
+Adds devices 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.
+
+When creating a new device group with this command, the provided name must not
+conflict with an existing device nickname (see: madb help name set).
+
+A group can contain another device group, in which case all the members of the
+other group will also be considered as members of the current group.
+`,
+ ArgsName: "<group_name> <member1> [<member2> ...]",
+ ArgsLong: `
+<group_name> is an alpha-numeric string with no special characters or spaces.
+This name must not be an existing device nickname.
+
+<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 runMadbGroupAdd(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 isDeviceNickname(groupName, cfg) {
+ return fmt.Errorf("The group name %q conflicts with a device nickname.", groupName)
+ }
+
+ members := removeDuplicates(args[1:])
+ for _, member := range members {
+ if err := isValidMember(member, cfg); err != nil {
+ return fmt.Errorf("Invalid member %q: %v", member, err)
+ }
+ }
+
+ oldMembers, ok := cfg.Groups[groupName]
+ if !ok {
+ oldMembers = []string{}
+ }
+
+ cfg.Groups[groupName] = removeDuplicates(append(oldMembers, members...))
+ 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.
+// TODO(youngseokyoon): reuse this function in madb.go.
+func isValidMember(member string, cfg *config) error {
+ if strings.HasPrefix(member, "@") {
+ index, err := strconv.Atoi(member[1:])
+ if err != nil || index <= 0 {
+ return fmt.Errorf("Invalid device specifier %q. '@' sign must be followed by a numeric device index starting from 1.", member)
+ }
+ return nil
+ } else if !isValidSerial(member) && !isValidName(member) {
+ return fmt.Errorf("Invalid device specifier %q. Not a valid serial or a nickname.", member)
+ }
+
+ return nil
+}
+
+// removeDuplicates takes a string slice and removes all the duplicates.
+func removeDuplicates(s []string) []string {
+ result := make([]string, 0, len(s))
+
+ used := map[string]bool{}
+ for _, elem := range s {
+ if !used[elem] {
+ result = append(result, elem)
+ used[elem] = true
+ }
+ }
+
+ return result
+}
diff --git a/group_test.go b/group_test.go
new file mode 100644
index 0000000..49dd946
--- /dev/null
+++ b/group_test.go
@@ -0,0 +1,105 @@
+// Copyright 2016 The Vanadium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package main
+
+import (
+ "os"
+ "reflect"
+ "testing"
+)
+
+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,
+ },
+ },
+ }
+
+ for i, testSuite := range tests {
+ filename := tempFilename(t)
+ defer os.Remove(filename)
+
+ for j, test := range testSuite {
+ err := runMadbGroupAdd(nil, test.input, filename)
+ if test.errExpected != (err != nil) {
+ t.Fatalf("error expected for tests[%v][%v]: %v, got: %v", i, j, test.errExpected, err)
+ }
+
+ cfg, err := readConfig(filename)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if got, want := cfg.Groups, test.want; !reflect.DeepEqual(got, want) {
+ t.Fatalf("unmatched results for tests[%v][%v]: got %v, want %v", i, j, got, want)
+ }
+ }
+ }
+}
+
+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)
+ }
+
+ err = runMadbGroupAdd(nil, []string{"NICKNAME1", "SERIAL2"}, filename)
+ if err == nil {
+ t.Fatalf("error expected but got: %v", err)
+ }
+}
diff --git a/madb.go b/madb.go
index 5ccbb5b..562436f 100644
--- a/madb.go
+++ b/madb.go
@@ -85,6 +85,7 @@
cmdMadbClearData,
cmdMadbExec,
cmdMadbExtern,
+ cmdMadbGroup,
cmdMadbInstall,
cmdMadbName,
cmdMadbShell,
@@ -336,6 +337,10 @@
Version string
// Names keeps the mapping between device nicknames and their serials.
Names map[string]string
+ // Groups keeps the device group definitions. A group can contain multiple
+ // devices, each of which is denoted by its name, serial, or index. A group
+ // can also include other groups.
+ Groups map[string][]string
// UserIDs keeps the mapping between device serials and their default user
// IDs.
UserIDs map[string]string
@@ -375,6 +380,7 @@
// The file may not exist or be empty when there are no stored data.
if stat, err := os.Stat(filename); os.IsNotExist(err) || (err == nil && stat.Size() == 0) {
result.Names = make(map[string]string)
+ result.Groups = make(map[string][]string)
result.UserIDs = make(map[string]string)
return result, nil
}
@@ -401,6 +407,9 @@
if result.Names == nil {
result.Names = make(map[string]string)
}
+ if result.Groups == nil {
+ result.Groups = make(map[string][]string)
+ }
if result.UserIDs == nil {
result.UserIDs = make(map[string]string)
}
@@ -422,6 +431,30 @@
return encoder.Encode(*cfg)
}
+func isNameInUse(name string, cfg *config) bool {
+ return isDeviceNickname(name, cfg) || isGroupName(name, cfg)
+}
+
+func isDeviceNickname(name string, cfg *config) bool {
+ _, ok := cfg.Names[name]
+ return ok
+}
+
+func isGroupName(name string, cfg *config) bool {
+ _, ok := cfg.Groups[name]
+ return ok
+}
+
+func isValidSerial(serial string) bool {
+ r := regexp.MustCompile(`^([A-Za-z0-9:\-\._]+|@\d+)$`)
+ return r.MatchString(serial)
+}
+
+func isValidName(name string) bool {
+ r := regexp.MustCompile(`^\w+$`)
+ return r.MatchString(name)
+}
+
type subCommandRunner struct {
// init is an optional function that does some initial work that should only
// be performed once, before directing the command to all the devices.
@@ -441,8 +474,9 @@
// Invokes the sub command on all the devices in parallel.
func (r subCommandRunner) Run(env *cmdline.Env, args []string) error {
prefixFlag = strings.ToLower(prefixFlag)
- if prefixFlag != "name" && prefixFlag != "serial" && prefixFlag != "none" {
- return fmt.Errorf(`The -prefix flag value must be one of "name", "serial", or "none".`)
+ allowed := []string{"name", "serial", "none"}
+ if !isStringInSlice(prefixFlag, allowed) {
+ return fmt.Errorf("The -prefix flag value must be one of %v", strings.Join(allowed, ", "))
}
if err := startAdbServer(); err != nil {
@@ -757,6 +791,17 @@
return result
}
+// isStringInSlice determines whether the given string appears in the slice.
+func isStringInSlice(str string, slice []string) bool {
+ for _, elem := range slice {
+ if str == elem {
+ return true
+ }
+ }
+
+ return false
+}
+
type pathProvider func() (string, error)
// subCommandRunnerWithFilepath is an adapter that turns the madb name/user subcommand functions into cmdline.Runners.
diff --git a/madb_test.go b/madb_test.go
index 395c1d5..6731496 100644
--- a/madb_test.go
+++ b/madb_test.go
@@ -514,3 +514,55 @@
}
}
}
+
+func TestIsValidSerial(t *testing.T) {
+ tests := []struct {
+ input string
+ want bool
+ }{
+ // The following strings should be accepted
+ {"HT4BVWV00023", true},
+ {"01023f5e2fd2accf", true},
+ {"usb:3-3.4.2", true},
+ {"product:volantisg", true},
+ {"model:Nexus_9", true},
+ {"device:flounder_lte", true},
+ {"@1", true},
+ {"@2", true},
+ // The following strings should not be accepted
+ {"have spaces", false},
+ {"@abcd", false},
+ {"#not_allowed_chars~", false},
+ }
+
+ for _, test := range tests {
+ if got := isValidSerial(test.input); got != test.want {
+ t.Fatalf("unmatched results for serial '%v': got %v, want %v", test.input, got, test.want)
+ }
+ }
+}
+
+func TestIsValidName(t *testing.T) {
+ tests := []struct {
+ input string
+ want bool
+ }{
+ // The following strings should be accepted
+ {"Nexus5X", true},
+ {"Nexus9", true},
+ {"P1", true},
+ {"P2", true},
+ {"Tablet", true},
+ // The following strings should not be accepted
+ {"have spaces", false},
+ {"@1", false},
+ {"@abcd", false},
+ {"#not_allowed_chars~", false},
+ }
+
+ for _, test := range tests {
+ if got := isValidName(test.input); got != test.want {
+ t.Fatalf("unmatched results for nickname '%v': got %v, want %v", test.input, got, test.want)
+ }
+ }
+}
diff --git a/name.go b/name.go
index aca6661..d73d855 100644
--- a/name.go
+++ b/name.go
@@ -6,7 +6,6 @@
import (
"fmt"
- "regexp"
"v.io/x/lib/cmdline"
)
@@ -65,11 +64,11 @@
}
serial, nickname := args[0], args[1]
- if !isValidDeviceSerial(serial) {
+ if !isValidSerial(serial) {
return env.UsageErrorf("Not a valid device serial: %v", serial)
}
- if !isValidNickname(nickname) {
+ if !isValidName(nickname) {
return env.UsageErrorf("Not a valid nickname: %v", nickname)
}
@@ -79,14 +78,14 @@
}
// If the nickname is already in use, don't allow it at all.
- if _, present := cfg.Names[nickname]; present {
+ if isNameInUse(nickname, cfg) {
return fmt.Errorf("The provided nickname %q is already in use.", nickname)
}
// If the serial number already has an assigned nickname, delete it first.
// Need to do this check, because the nickname-serial map should be a one-to-one mapping.
- if nickname, present := reverseMap(cfg.Names)[serial]; present {
- delete(cfg.Names, nickname)
+ if name, present := reverseMap(cfg.Names)[serial]; present {
+ delete(cfg.Names, name)
}
// Add the nickname serial mapping.
@@ -116,7 +115,7 @@
}
name := args[0]
- if !isValidDeviceSerial(name) && !isValidNickname(name) {
+ if !isValidSerial(name) && !isValidName(name) {
return env.UsageErrorf("Not a valid device serial or name: %v", name)
}
@@ -186,16 +185,6 @@
return writeConfig(cfg, filename)
}
-func isValidDeviceSerial(serial string) bool {
- r := regexp.MustCompile(`^([A-Za-z0-9:\-\._]+|@\d+)$`)
- return r.MatchString(serial)
-}
-
-func isValidNickname(nickname string) bool {
- r := regexp.MustCompile(`^\w+$`)
- return r.MatchString(nickname)
-}
-
// reverseMap returns a new map which contains reversed key, value pairs in the original map.
// The source map is assumed to be a one-to-one mapping between keys and values.
func reverseMap(source map[string]string) map[string]string {
diff --git a/name_test.go b/name_test.go
index a8fc1cf..d6aeed4 100644
--- a/name_test.go
+++ b/name_test.go
@@ -57,6 +57,17 @@
if err = runMadbNameSet(nil, []string{"SERIAL3", "NN1"}, filename); err == nil {
t.Fatalf("expected an error but succeeded.")
}
+
+ // Try an existing group name and see if it fails.
+ err = runMadbGroupAdd(nil, []string{"GROUP1", "SERIAL1"}, filename)
+ if err != nil {
+ t.Fatal(err)
+ }
+
+ err = runMadbNameSet(nil, []string{"SERIAL4", "GROUP1"}, filename)
+ if err == nil {
+ t.Fatalf("error expected but got: %v", err)
+ }
}
func TestMadbNameUnset(t *testing.T) {
@@ -135,55 +146,3 @@
t.Fatalf("unmatched results: got %v, want %v", got, want)
}
}
-
-func TestIsValidDeviceSerial(t *testing.T) {
- tests := []struct {
- input string
- want bool
- }{
- // The following strings should be accepted
- {"HT4BVWV00023", true},
- {"01023f5e2fd2accf", true},
- {"usb:3-3.4.2", true},
- {"product:volantisg", true},
- {"model:Nexus_9", true},
- {"device:flounder_lte", true},
- {"@1", true},
- {"@2", true},
- // The following strings should not be accepted
- {"have spaces", false},
- {"@abcd", false},
- {"#not_allowed_chars~", false},
- }
-
- for _, test := range tests {
- if got := isValidDeviceSerial(test.input); got != test.want {
- t.Fatalf("unmatched results for serial '%v': got %v, want %v", test.input, got, test.want)
- }
- }
-}
-
-func TestIsValidNickname(t *testing.T) {
- tests := []struct {
- input string
- want bool
- }{
- // The following strings should be accepted
- {"Nexus5X", true},
- {"Nexus9", true},
- {"P1", true},
- {"P2", true},
- {"Tablet", true},
- // The following strings should not be accepted
- {"have spaces", false},
- {"@1", false},
- {"@abcd", false},
- {"#not_allowed_chars~", false},
- }
-
- for _, test := range tests {
- if got := isValidNickname(test.input); got != test.want {
- t.Fatalf("unmatched results for nickname '%v': got %v, want %v", test.input, got, test.want)
- }
- }
-}
diff --git a/user.go b/user.go
index 75c87f4..6bab71d 100644
--- a/user.go
+++ b/user.go
@@ -92,7 +92,7 @@
// TODO(youngseokyoon): make it possible to specify the device using its nickname or index.
// Validate the device serial
serial := args[0]
- if !isValidDeviceSerial(serial) {
+ if !isValidSerial(serial) {
return fmt.Errorf("Not a valid device serial: %v", serial)
}
@@ -137,7 +137,7 @@
// TODO(youngseokyoon): make it possible to specify the device using its nickname or index.
// Validate the device serial
serial := args[0]
- if !isValidDeviceSerial(serial) {
+ if !isValidSerial(serial) {
return fmt.Errorf("Not a valid device serial: %v", serial)
}
