lib: Add support for cmdline.Env.Vars
This adds envrionment variable support to cmdline.Env, and also
updates some programs to use this support. The main benefit of
using cmdline.Env.Vars is that it makes testing easier, since you
can just pass in your own map.
We represent environment variables as map[string]string, which is
a nice simple form for lookups and mutations. Originally I was
planning on using *envvar.Vars, but the delta-tracking support is
probably unnecessary, and the native Go map syntax is nicer.
A subsequent change will update the v23.Init and test.InitForTest
mechanisms to support explicit passing of args and envvars, and
will take advantage of this support.
MultiPart: 1/2
Change-Id: I6044cf72a54254d9abd585908a1b712847b97913
diff --git a/cmdline/.api b/cmdline/.api
index 2e4b2ae..9067aa0 100644
--- a/cmdline/.api
+++ b/cmdline/.api
@@ -23,6 +23,7 @@
pkg cmdline, type Env struct, Stdin io.Reader
pkg cmdline, type Env struct, Stdout io.Writer
pkg cmdline, type Env struct, Usage func(io.Writer)
+pkg cmdline, type Env struct, Vars map[string]string
pkg cmdline, type ErrExitCode int
pkg cmdline, type Runner interface { Run }
pkg cmdline, type Runner interface, Run(*Env, []string) error
diff --git a/cmdline/cmdline_test.go b/cmdline/cmdline_test.go
index d367a96..ecdf043 100644
--- a/cmdline/cmdline_test.go
+++ b/cmdline/cmdline_test.go
@@ -9,15 +9,12 @@
"errors"
"flag"
"fmt"
- "os"
"regexp"
"strings"
"testing"
-)
-func init() {
- os.Setenv("CMDLINE_WIDTH", "80") // make sure the formatting stays the same.
-}
+ "v.io/x/lib/envvar"
+)
var (
errEchoStr = "echo error"
@@ -62,7 +59,7 @@
type testCase struct {
Args []string
- Envs map[string]string
+ Vars map[string]string
Err string
Stdout string
Stderr string
@@ -84,22 +81,18 @@
return fmt.Sprint(err)
}
+var baseVars = map[string]string{
+ "CMDLINE_WIDTH": "80", // make sure formatting stays the same.
+}
+
func runTestCases(t *testing.T, cmd *Command, tests []testCase) {
for _, test := range tests {
// Reset global variables before running each test case.
var stdout, stderr bytes.Buffer
flagExtra, flagTopLevelExtra, optNoNewline = false, false, false
- origEnvs := make(map[string]string)
- // Set a fresh flag.CommandLine and fresh envvars for each run.
- flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ContinueOnError)
- for k, v := range test.Envs {
- origEnvs[k] = os.Getenv(k)
- if err := os.Setenv(k, v); err != nil {
- t.Fatalf("os.Setenv(%v, %v) failed: %v", k, v, err)
- }
- }
-
+ // Start with a fresh flag.CommandLine for each run.
+ flag.CommandLine = flag.NewFlagSet("test", flag.ContinueOnError)
var globalFlag1 string
flag.StringVar(&globalFlag1, "global1", "", "global test flag 1")
globalFlag2 := flag.Int64("global2", 0, "global test flag 2")
@@ -109,20 +102,24 @@
// Parse and run the command and check against expected results.
parseOK := false
- env := &Env{Stdout: &stdout, Stderr: &stderr}
+ env := &Env{
+ Stdout: &stdout,
+ Stderr: &stderr,
+ Vars: envvar.MergeMaps(baseVars, test.Vars),
+ }
runner, args, err := Parse(cmd, env, test.Args)
if err == nil {
err = runner.Run(env, args)
parseOK = true
}
if got, want := errString(err), test.Err; got != want {
- t.Errorf("Ran with args %q envs %q\n GOT error:\n%q\nWANT error:\n%q", test.Args, test.Envs, got, want)
+ t.Errorf("Ran with args %q vars %q\n GOT error:\n%q\nWANT error:\n%q", test.Args, test.Vars, got, want)
}
if got, want := stripTestFlags(stdout.String()), test.Stdout; got != want {
- t.Errorf("Ran with args %q envs %q\n GOT stdout:\n%q\nWANT stdout:\n%q", test.Args, test.Envs, got, want)
+ t.Errorf("Ran with args %q vars %q\n GOT stdout:\n%q\nWANT stdout:\n%q", test.Args, test.Vars, got, want)
}
if got, want := stripTestFlags(stderr.String()), test.Stderr; got != want {
- t.Errorf("Ran with args %q envs %q\n GOT stderr:\n%q\nWANT stderr:\n%q", test.Args, test.Envs, got, want)
+ t.Errorf("Ran with args %q vars %q\n GOT stderr:\n%q\nWANT stderr:\n%q", test.Args, test.Vars, got, want)
}
if got, want := globalFlag1, test.GlobalFlag1; got != want {
t.Errorf("global1 flag got %q, want %q", got, want)
@@ -134,11 +131,6 @@
if parseOK && !flag.CommandLine.Parsed() {
t.Errorf("flag.CommandLine should be parsed by now")
}
- for k, v := range origEnvs {
- if err := os.Setenv(k, v); err != nil {
- t.Fatalf("os.Setenv(%v, %v) failed: %v", k, v, err)
- }
- }
}
}
@@ -2372,7 +2364,7 @@
},
{
Args: []string{"-help"},
- Envs: map[string]string{"CMDLINE_STYLE": "full"},
+ Vars: map[string]string{"CMDLINE_STYLE": "full"},
Stdout: `Test hiding global flags, root no children.
Usage:
diff --git a/cmdline/env.go b/cmdline/env.go
index cbe8e5d..df06fa5 100644
--- a/cmdline/env.go
+++ b/cmdline/env.go
@@ -10,12 +10,18 @@
"os"
"strconv"
+ "v.io/x/lib/envvar"
"v.io/x/lib/textutil"
)
// NewEnv returns a new environment with defaults based on the operating system.
func NewEnv() *Env {
- return &Env{Stdin: os.Stdin, Stdout: os.Stdout, Stderr: os.Stderr}
+ return &Env{
+ Stdin: os.Stdin,
+ Stdout: os.Stdout,
+ Stderr: os.Stderr,
+ Vars: envvar.SliceToMap(os.Environ()),
+ }
}
// Env represents the environment for command parsing and running. Typically
@@ -25,8 +31,7 @@
Stdin io.Reader
Stdout io.Writer
Stderr io.Writer
- // TODO(toddw): Add env vars, using a new "v.io/x/lib/envvar" package.
- // Vars envvar.Vars
+ Vars map[string]string // Environment variables
// Usage is a function that prints usage information to w. Typically set by
// calls to Main or Parse to print usage of the leaf command.
@@ -56,8 +61,7 @@
const defaultWidth = 80
func (e *Env) width() int {
- // TODO(toddw): Replace os.Getenv with lookup in env.Vars.
- if width, err := strconv.Atoi(os.Getenv("CMDLINE_WIDTH")); err == nil && width != 0 {
+ if width, err := strconv.Atoi(e.Vars["CMDLINE_WIDTH"]); err == nil && width != 0 {
return width
}
if _, width, err := textutil.TerminalSize(); err == nil && width != 0 {
@@ -67,9 +71,8 @@
}
func (e *Env) style() style {
- // TODO(toddw): Replace os.Getenv with lookup in env.Vars.
style := styleCompact
- style.Set(os.Getenv("CMDLINE_STYLE"))
+ style.Set(e.Vars["CMDLINE_STYLE"])
return style
}
diff --git a/cmdline/env_test.go b/cmdline/env_test.go
index 963343a..86bc819 100644
--- a/cmdline/env_test.go
+++ b/cmdline/env_test.go
@@ -42,8 +42,8 @@
func TestEnvWidth(t *testing.T) {
tests := []struct {
- env string
- want int
+ value string
+ want int
}{
{"123", 123},
{"-1", -1},
@@ -52,20 +52,25 @@
{"foobar", defaultWidth},
}
for _, test := range tests {
- if err := os.Setenv("CMDLINE_WIDTH", test.env); err != nil {
- t.Errorf("Setenv(%q) failed: %v", test.env, err)
- continue
+ // Test using a fake environment.
+ env := &Env{Vars: map[string]string{"CMDLINE_WIDTH": test.value}}
+ if got, want := env.width(), test.want; got != want {
+ t.Errorf("%q got %v, want %v", test.value, got, want)
}
- if got, want := NewEnv().width(), test.want; got != want {
- t.Errorf("%q got %v, want %v", test.env, got, want)
+ // Test using the OS environment.
+ if err := os.Setenv("CMDLINE_WIDTH", test.value); err != nil {
+ t.Errorf("Setenv(%q) failed: %v", test.value, err)
+ } else if got, want := NewEnv().width(), test.want; got != want {
+ t.Errorf("%q got %v, want %v", test.value, got, want)
}
}
+ os.Unsetenv("CMDLINE_WIDTH")
}
func TestEnvStyle(t *testing.T) {
tests := []struct {
- env string
- want style
+ value string
+ want style
}{
{"compact", styleCompact},
{"full", styleFull},
@@ -75,12 +80,17 @@
{"foobar", styleCompact},
}
for _, test := range tests {
- if err := os.Setenv("CMDLINE_STYLE", test.env); err != nil {
- t.Errorf("Setenv(%q) failed: %v", test.env, err)
- continue
+ // Test using a fake environment.
+ env := &Env{Vars: map[string]string{"CMDLINE_STYLE": test.value}}
+ if got, want := env.style(), test.want; got != want {
+ t.Errorf("%q got %v, want %v", test.value, got, want)
}
- if got, want := NewEnv().style(), test.want; got != want {
- t.Errorf("%q got %v, want %v", test.env, got, want)
+ // Test using the OS environment.
+ if err := os.Setenv("CMDLINE_STYLE", test.value); err != nil {
+ t.Errorf("Setenv(%q) failed: %v", test.value, err)
+ } else if got, want := NewEnv().style(), test.want; got != want {
+ t.Errorf("%q got %v, want %v", test.value, got, want)
}
}
+ os.Unsetenv("CMDLINE_STYLE")
}
diff --git a/envvar/envvar_test.go b/envvar/envvar_test.go
index 74d8a02..d991184 100644
--- a/envvar/envvar_test.go
+++ b/envvar/envvar_test.go
@@ -424,7 +424,7 @@
if err := os.Setenv(testKey, testValue); err != nil {
t.Fatalf("Setenv(%q, %q) failed: %v", testKey, testValue, err)
}
- defer os.Setenv(testKey, "")
+ defer os.Unsetenv(testKey)
vars := VarsFromOS()
if got, want := vars.Contains(testKey), true; got != want {
t.Errorf(`Contains(%q) got %v, want %v`, testKey, got, want)
