Skip to content

Commit 86c52ec

Browse files
dgageotdgageot
committed
Make commands code easier to test
Signed-off-by: David Gageot <david@gageot.net>
1 parent d855c35 commit 86c52ec

File tree

24 files changed

+222
-176
lines changed

24 files changed

+222
-176
lines changed

‎commands/active.go‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,20 +12,27 @@ import (
1212
"github.com/docker/machine/libmachine/state"
1313
)
1414

15-
func cmdActive(c *cli.Context) {
15+
var (
16+
errTooManyArguments = errors.New("Error: Too many arguments given")
17+
)
18+
19+
func cmdActive(c *cli.Context) error {
1620
if len(c.Args()) > 0 {
17-
fatal("Error: Too many arguments given.")
21+
return errTooManyArguments
1822
}
1923

2024
store := getStore(c)
25+
2126
host, err := getActiveHost(store)
2227
if err != nil {
23-
fatalf("Error getting active host: %s", err)
28+
return fmt.Errorf("Error getting active host: %s", err)
2429
}
2530

2631
if host != nil {
2732
fmt.Println(host.Name)
2833
}
34+
35+
return nil
2936
}
3037

3138
func getActiveHost(store persist.Store) (*host.Host, error) {

‎commands/commands.go‎

Lines changed: 37 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ import (
1919

2020
var (
2121
ErrUnknownShell = errors.New("Error: Unknown shell")
22-
ErrNoMachineSpecified = errors.New("Error: Expected to get one or more machine names as arguments.")
23-
ErrExpectedOneMachine = errors.New("Error: Expected one machine name as an argument.")
22+
ErrNoMachineSpecified = errors.New("Error: Expected to get one or more machine names as arguments")
23+
ErrExpectedOneMachine = errors.New("Error: Expected one machine name as an argument")
2424
)
2525

2626
func newPluginDriver(driverName string, rawContent []byte) (*rpcdriver.RpcClientDriver, error) {
@@ -32,29 +32,25 @@ func newPluginDriver(driverName string, rawContent []byte) (*rpcdriver.RpcClient
3232
return d, nil
3333
}
3434

35-
func fatal(args ...interface{}) {
36-
log.Fatal(args...)
37-
}
38-
39-
func fatalf(fmtString string, args ...interface{}) {
40-
log.Fatalf(fmtString, args...)
35+
func fatalOnError(command func(context *cli.Context) error) func(context *cli.Context) {
36+
return func(context *cli.Context) {
37+
if err := command(context); err != nil {
38+
log.Fatal(err)
39+
}
40+
}
4141
}
4242

43-
func confirmInput(msg string) bool {
43+
func confirmInput(msg string) (bool, error) {
4444
fmt.Printf("%s (y/n): ", msg)
45+
4546
var resp string
4647
_, err := fmt.Scanln(&resp)
47-
4848
if err != nil {
49-
fatal(err)
49+
return false, err
5050
}
5151

52-
if strings.Index(strings.ToLower(resp), "y") == 0 {
53-
return true
54-
55-
}
56-
57-
return false
52+
confirmed := strings.Index(strings.ToLower(resp), "y") == 0
53+
return confirmed, nil
5854
}
5955

6056
func getStore(c *cli.Context) persist.Store {
@@ -112,17 +108,16 @@ func saveHost(store persist.Store, h *host.Host) error {
112108
return nil
113109
}
114110

115-
func getFirstArgHost(c *cli.Context) *host.Host {
111+
func getFirstArgHost(c *cli.Context) (*host.Host, error) {
116112
store := getStore(c)
117113
hostName := c.Args().First()
118114

119115
exists, err := store.Exists(hostName)
120116
if err != nil {
121-
fatalf("Error checking if host %q exists: %s", hostName, err)
117+
return nil, fmt.Errorf("Error checking if host %q exists: %s", hostName, err)
122118
}
123-
124119
if !exists {
125-
fatalf("Host %q does not exist", hostName)
120+
return nil, fmt.Errorf("Host %q does not exist", hostName)
126121
}
127122

128123
h, err := loadHost(store, hostName)
@@ -131,9 +126,10 @@ func getFirstArgHost(c *cli.Context) *host.Host {
131126
// the host reliably we're definitely not going to be able to
132127
// do anything else interesting, but also this premature exit
133128
// feels wrong to me. Let's revisit it later.
134-
fatalf("Error trying to get host %q: %s", hostName, err)
129+
return nil, fmt.Errorf("Error trying to get host %q: %s", hostName, err)
135130
}
136-
return h
131+
132+
return h, nil
137133
}
138134

139135
func getHostsFromContext(c *cli.Context) ([]*host.Host, error) {
@@ -155,13 +151,13 @@ var Commands = []cli.Command{
155151
{
156152
Name: "active",
157153
Usage: "Print which machine is active",
158-
Action: cmdActive,
154+
Action: fatalOnError(cmdActive),
159155
},
160156
{
161157
Name: "config",
162158
Usage: "Print the connection config for machine",
163159
Description: "Argument is a machine name.",
164-
Action: cmdConfig,
160+
Action: fatalOnError(cmdConfig),
165161
Flags: []cli.Flag{
166162
cli.BoolFlag{
167163
Name: "swarm",
@@ -173,14 +169,14 @@ var Commands = []cli.Command{
173169
Flags: sharedCreateFlags,
174170
Name: "create",
175171
Usage: "Create a machine.\n\nSpecify a driver with --driver to include the create flags for that driver in the help text.",
176-
Action: cmdCreateOuter,
172+
Action: fatalOnError(cmdCreateOuter),
177173
SkipFlagParsing: true,
178174
},
179175
{
180176
Name: "env",
181177
Usage: "Display the commands to set up the environment for the Docker client",
182178
Description: "Argument is a machine name.",
183-
Action: cmdEnv,
179+
Action: fatalOnError(cmdEnv),
184180
Flags: []cli.Flag{
185181
cli.BoolFlag{
186182
Name: "swarm",
@@ -204,7 +200,7 @@ var Commands = []cli.Command{
204200
Name: "inspect",
205201
Usage: "Inspect information about a machine",
206202
Description: "Argument is a machine name.",
207-
Action: cmdInspect,
203+
Action: fatalOnError(cmdInspect),
208204
Flags: []cli.Flag{
209205
cli.StringFlag{
210206
Name: "format, f",
@@ -217,13 +213,13 @@ var Commands = []cli.Command{
217213
Name: "ip",
218214
Usage: "Get the IP address of a machine",
219215
Description: "Argument(s) are one or more machine names.",
220-
Action: cmdIp,
216+
Action: fatalOnError(cmdIp),
221217
},
222218
{
223219
Name: "kill",
224220
Usage: "Kill a machine",
225221
Description: "Argument(s) are one or more machine names.",
226-
Action: cmdKill,
222+
Action: fatalOnError(cmdKill),
227223
},
228224
{
229225
Flags: []cli.Flag{
@@ -239,13 +235,13 @@ var Commands = []cli.Command{
239235
},
240236
Name: "ls",
241237
Usage: "List machines",
242-
Action: cmdLs,
238+
Action: fatalOnError(cmdLs),
243239
},
244240
{
245241
Name: "regenerate-certs",
246242
Usage: "Regenerate TLS Certificates for a machine",
247243
Description: "Argument(s) are one or more machine names.",
248-
Action: cmdRegenerateCerts,
244+
Action: fatalOnError(cmdRegenerateCerts),
249245
Flags: []cli.Flag{
250246
cli.BoolFlag{
251247
Name: "force, f",
@@ -257,7 +253,7 @@ var Commands = []cli.Command{
257253
Name: "restart",
258254
Usage: "Restart a machine",
259255
Description: "Argument(s) are one or more machine names.",
260-
Action: cmdRestart,
256+
Action: fatalOnError(cmdRestart),
261257
},
262258
{
263259
Flags: []cli.Flag{
@@ -269,20 +265,20 @@ var Commands = []cli.Command{
269265
Name: "rm",
270266
Usage: "Remove a machine",
271267
Description: "Argument(s) are one or more machine names.",
272-
Action: cmdRm,
268+
Action: fatalOnError(cmdRm),
273269
},
274270
{
275271
Name: "ssh",
276272
Usage: "Log into or run a command on a machine with SSH.",
277273
Description: "Arguments are [machine-name] [command]",
278-
Action: cmdSsh,
274+
Action: fatalOnError(cmdSsh),
279275
SkipFlagParsing: true,
280276
},
281277
{
282278
Name: "scp",
283279
Usage: "Copy files between machines",
284280
Description: "Arguments are [machine:][path] [machine:][path].",
285-
Action: cmdScp,
281+
Action: fatalOnError(cmdScp),
286282
Flags: []cli.Flag{
287283
cli.BoolFlag{
288284
Name: "recursive, r",
@@ -294,31 +290,31 @@ var Commands = []cli.Command{
294290
Name: "start",
295291
Usage: "Start a machine",
296292
Description: "Argument(s) are one or more machine names.",
297-
Action: cmdStart,
293+
Action: fatalOnError(cmdStart),
298294
},
299295
{
300296
Name: "status",
301297
Usage: "Get the status of a machine",
302298
Description: "Argument is a machine name.",
303-
Action: cmdStatus,
299+
Action: fatalOnError(cmdStatus),
304300
},
305301
{
306302
Name: "stop",
307303
Usage: "Stop a machine",
308304
Description: "Argument(s) are one or more machine names.",
309-
Action: cmdStop,
305+
Action: fatalOnError(cmdStop),
310306
},
311307
{
312308
Name: "upgrade",
313309
Usage: "Upgrade a machine to the latest version of Docker",
314310
Description: "Argument(s) are one or more machine names.",
315-
Action: cmdUpgrade,
311+
Action: fatalOnError(cmdUpgrade),
316312
},
317313
{
318314
Name: "url",
319315
Usage: "Get the URL of a machine",
320316
Description: "Argument is a machine name.",
321-
Action: cmdUrl,
317+
Action: fatalOnError(cmdUrl),
322318
},
323319
}
324320

‎commands/config.go‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,26 +14,31 @@ import (
1414
"github.com/docker/machine/libmachine/state"
1515
)
1616

17-
func cmdConfig(c *cli.Context) {
17+
func cmdConfig(c *cli.Context) error {
1818
// Ensure that log messages always go to stderr when this command is
1919
// being run (it is intended to be run in a subshell)
2020
log.SetOutWriter(os.Stderr)
2121

2222
if len(c.Args()) != 1 {
23-
fatal(ErrExpectedOneMachine)
23+
return ErrExpectedOneMachine
2424
}
2525

26-
h := getFirstArgHost(c)
26+
host, err := getFirstArgHost(c)
27+
if err != nil {
28+
return err
29+
}
2730

28-
dockerHost, authOptions, err := runConnectionBoilerplate(h, c)
31+
dockerHost, authOptions, err := runConnectionBoilerplate(host, c)
2932
if err != nil {
30-
fatalf("Error running connection boilerplate: %s", err)
33+
return fmt.Errorf("Error running connection boilerplate: %s", err)
3134
}
3235

3336
log.Debug(dockerHost)
3437

3538
fmt.Printf("--tlsverify --tlscacert=%q --tlscert=%q --tlskey=%q -H=%s",
3639
authOptions.CaCertPath, authOptions.ClientCertPath, authOptions.ClientKeyPath, dockerHost)
40+
41+
return nil
3742
}
3843

3944
func runConnectionBoilerplate(h *host.Host, c *cli.Context) (string, *auth.AuthOptions, error) {
@@ -44,7 +49,7 @@ func runConnectionBoilerplate(h *host.Host, c *cli.Context) (string, *auth.AuthO
4449
return "", &auth.AuthOptions{}, fmt.Errorf("Error trying to get host state: %s", err)
4550
}
4651
if hostState != state.Running {
47-
return "", &auth.AuthOptions{}, fmt.Errorf("%s is not running. Please start it in order to use the connection settings.", h.Name)
52+
return "", &auth.AuthOptions{}, fmt.Errorf("%s is not running. Please start it in order to use the connection settings", h.Name)
4853
}
4954

5055
dockerHost, err := h.Driver.GetURL()
@@ -101,7 +106,7 @@ func parseSwarm(hostUrl string, h *host.Host) (string, error) {
101106
swarmOptions := h.HostOptions.SwarmOptions
102107

103108
if !swarmOptions.Master {
104-
return "", fmt.Errorf("Error: %s is not a swarm master. The --swarm flag is intended for use with swarm masters.", h.Name)
109+
return "", fmt.Errorf("Error: %s is not a swarm master. The --swarm flag is intended for use with swarm masters", h.Name)
105110
}
106111

107112
u, err := url.Parse(swarmOptions.Host)

0 commit comments

Comments
 (0)