Add test coverage for using levels in a FlagSet (#257)

* Add test coverage for using levels in a FlagSet

Levels implement `flag.Value`, so they should be usable in the standard lib's
flag sets. However, this wasn't covered by tests.

In the final push for docs, we'll need examples for these use cases.

* Updates from CR

flag.go

@@ -29,6 +29,8 @@ import (
 // LevelFlag defines a Level flag with specified name, default value and
 // usage string. The return value is the address of a Level value that stores
 // the value of the flag.
+//
+// Note that you can also use any non-nil *Level as a flag.Value.
 func LevelFlag(name string, defaultLevel zapcore.Level, usage string) *zapcore.Level {
 	lvl := defaultLevel
 	flag.Var(&lvl, name, usage)

flag_test.go

@@ -30,12 +30,42 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
+type flagTestCase struct {
+	args      []string
+	wantLevel zapcore.Level
+	wantErr   bool
+}
+
+func (tc flagTestCase) runImplicitSet(t testing.TB) {
+	origCommandLine := flag.CommandLine
+	flag.CommandLine = flag.NewFlagSet("test", flag.ContinueOnError)
+	flag.CommandLine.SetOutput(ioutil.Discard)
+	defer func() { flag.CommandLine = origCommandLine }()
+
+	level := LevelFlag("level", InfoLevel, "")
+	tc.run(t, flag.CommandLine, level)
+}
+
+func (tc flagTestCase) runExplicitSet(t testing.TB) {
+	var lvl zapcore.Level
+	set := flag.NewFlagSet("test", flag.ContinueOnError)
+	set.Var(&lvl, "level", "minimum enabled logging level")
+	tc.run(t, set, &lvl)
+}
+
+func (tc flagTestCase) run(t testing.TB, set *flag.FlagSet, actual *zapcore.Level) {
+	err := set.Parse(tc.args)
+	if tc.wantErr {
+		assert.Error(t, err, "Parse(%v) should fail.", tc.args)
+		return
+	}
+	if assert.NoError(t, err, "Parse(%v) should succeed.", tc.args) {
+		assert.Equal(t, tc.wantLevel, *actual, "Level mismatch.")
+	}
+}
+
 func TestLevelFlag(t *testing.T) {
-	tests := []struct {
-		args      []string
-		wantLevel zapcore.Level
-		wantErr   bool
-	}{
+	tests := []flagTestCase{
 		{
 			args:      nil,
 			wantLevel: zapcore.InfoLevel,
@@ -50,22 +80,23 @@ func TestLevelFlag(t *testing.T) {
 		},
 	}
 
+	for _, tt := range tests {
+		tt.runExplicitSet(t)
+		tt.runImplicitSet(t)
+	}
+}
+
+func TestLevelFlagsAreIndependent(t *testing.T) {
 	origCommandLine := flag.CommandLine
+	flag.CommandLine = flag.NewFlagSet("test", flag.ContinueOnError)
+	flag.CommandLine.SetOutput(ioutil.Discard)
 	defer func() { flag.CommandLine = origCommandLine }()
 
-	for _, tt := range tests {
-		flag.CommandLine = flag.NewFlagSet("test", flag.ContinueOnError)
-		flag.CommandLine.SetOutput(ioutil.Discard)
-		level := LevelFlag("level", InfoLevel, "")
+	// Make sure that these two flags are independent.
+	fileLevel := LevelFlag("file-level", InfoLevel, "")
+	consoleLevel := LevelFlag("console-level", InfoLevel, "")
 
-		err := flag.CommandLine.Parse(tt.args)
-		if tt.wantErr {
-			assert.Error(t, err, "Parse(%v) should fail", tt.args)
-			continue
-		}
-
-		if assert.NoError(t, err, "Parse(%v) shouldn't fail", tt.args) {
-			assert.Equal(t, tt.wantLevel, *level, "Level mismatch")
-		}
-	}
+	assert.NoError(t, flag.CommandLine.Parse([]string{"-file-level", "debug"}), "Unexpected flag-parsing error.")
+	assert.Equal(t, InfoLevel, *consoleLevel, "Expected file logging level to remain unchanged.")
+	assert.Equal(t, DebugLevel, *fileLevel, "Expected console logging level to have changed.")
 }