From 73696aee3cbb63ed1b47e8cad3be78a2a5edf3bb Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Mon, 29 Apr 2024 15:18:24 +0000 Subject: [PATCH 1/7] test: add test for options env --- options_test.go | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 options_test.go diff --git a/options_test.go b/options_test.go new file mode 100644 index 00000000..e089d483 --- /dev/null +++ b/options_test.go @@ -0,0 +1,40 @@ +package envbuilder_test + +import ( + "testing" + + "github.com/coder/envbuilder" + "github.com/coder/serpent" + "github.com/stretchr/testify/require" +) + +type testEnvCase struct { + Env string + EnvValue string + Expected interface{} + Actual func() interface{} +} + +func TestEnvOptions(t *testing.T) { + t.Setenv("SETUP_SCRIPT", "setup.sh") + t.Setenv("CACHE_TTL_DAYS", "7") + t.Setenv("IGNORE_PATHS", "/var,/tmp") + t.Setenv("SKIP_REBUILD", "true") + t.Setenv("GIT_CLONE_SINGLE_BRANCH", "") + + var o = envbuilder.Options{} + cmd := serpent.Command{ + Options: o.CLI(), + Handler: func(inv *serpent.Invocation) error { + return nil + }, + } + err := cmd.Invoke().WithOS().Run() + require.NoError(t, err) + + require.Equal(t, o.SetupScript, "setup.sh") + require.Equal(t, o.CacheTTLDays, int64(7)) + require.Equal(t, o.IgnorePaths, []string{"/var", "/tmp"}) + require.True(t, o.SkipRebuild) + require.False(t, o.GitCloneSingleBranch) +} From deab071eeab1939535672563f3a77a0c239da609 Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Mon, 29 Apr 2024 14:16:22 -0300 Subject: [PATCH 2/7] Apply suggestions from Danny's code review Co-authored-by: Danny Kopping --- options_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/options_test.go b/options_test.go index e089d483..5eb10195 100644 --- a/options_test.go +++ b/options_test.go @@ -15,14 +15,15 @@ type testEnvCase struct { Actual func() interface{} } -func TestEnvOptions(t *testing.T) { +// TestEnvOptionParsing tests that given environment variables of different types are handled as expected. +func TestEnvOptionParsing(t *testing.T) { t.Setenv("SETUP_SCRIPT", "setup.sh") t.Setenv("CACHE_TTL_DAYS", "7") t.Setenv("IGNORE_PATHS", "/var,/tmp") t.Setenv("SKIP_REBUILD", "true") t.Setenv("GIT_CLONE_SINGLE_BRANCH", "") - var o = envbuilder.Options{} + var o envbuilder.Options cmd := serpent.Command{ Options: o.CLI(), Handler: func(inv *serpent.Invocation) error { From b091ca7861244aa815b12c5f4cb644a87013ca4f Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Mon, 29 Apr 2024 17:17:51 +0000 Subject: [PATCH 3/7] Remove unecessary type --- options_test.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/options_test.go b/options_test.go index 5eb10195..fa7843ca 100644 --- a/options_test.go +++ b/options_test.go @@ -8,13 +8,6 @@ import ( "github.com/stretchr/testify/require" ) -type testEnvCase struct { - Env string - EnvValue string - Expected interface{} - Actual func() interface{} -} - // TestEnvOptionParsing tests that given environment variables of different types are handled as expected. func TestEnvOptionParsing(t *testing.T) { t.Setenv("SETUP_SCRIPT", "setup.sh") From b76a4c0495084ea8bb2b9cd145418552318e5529 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Mon, 29 Apr 2024 17:33:42 +0000 Subject: [PATCH 4/7] Split tests by type and improve boolean tests --- options_test.go | 60 ++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 50 insertions(+), 10 deletions(-) diff --git a/options_test.go b/options_test.go index fa7843ca..628b7678 100644 --- a/options_test.go +++ b/options_test.go @@ -8,15 +8,61 @@ import ( "github.com/stretchr/testify/require" ) -// TestEnvOptionParsing tests that given environment variables of different types are handled as expected. -func TestEnvOptionParsing(t *testing.T) { +// TestStrEnvOptions tests that string env variables can be handled as expected. +func TestStrEnvOptions(t *testing.T) { t.Setenv("SETUP_SCRIPT", "setup.sh") + t.Setenv("INIT_COMMAND", "sleep infinity") + + var o envbuilder.Options + err := runCLI(&o) + require.NoError(t, err) + + require.Equal(t, o.SetupScript, "setup.sh") + require.Equal(t, o.InitCommand, "sleep infinity") +} + +// TestIntEnvOptions tests that numeric env variables can be handled as expected. +func TestIntEnvOptions(t *testing.T) { + t.Setenv("CACHE_TTL_DAYS", "7") + + var o envbuilder.Options + err := runCLI(&o) + require.NoError(t, err) + + require.Equal(t, o.CacheTTLDays, 7) +} + +// TestMultipleStrEnvOptions tests that numeric env variables can be handled as expected. +func TestMultipleStrEnvOptions(t *testing.T) { t.Setenv("CACHE_TTL_DAYS", "7") - t.Setenv("IGNORE_PATHS", "/var,/tmp") + + var o envbuilder.Options + err := runCLI(&o) + require.NoError(t, err) + + require.Equal(t, o.CacheTTLDays, 7) +} + +// TestBoolEnvOptions tests that boolean env variables can be handled as expected. +func TestBoolEnvOptions(t *testing.T) { t.Setenv("SKIP_REBUILD", "true") t.Setenv("GIT_CLONE_SINGLE_BRANCH", "") + t.Setenv("EXIT_ON_BUILD_FAILURE", "false") + t.Setenv("FORCE_SAFE", "TRUE") + t.Setenv("INSECURE", "FALSE") var o envbuilder.Options + err := runCLI(&o) + require.NoError(t, err) + + require.True(t, o.SkipRebuild) + require.False(t, o.GitCloneSingleBranch) + require.False(t, o.ExitOnBuildFailure) + require.True(t, o.ForceSafe) + require.False(t, o.Insecure) +} + +func runCLI(o *envbuilder.Options) error { cmd := serpent.Command{ Options: o.CLI(), Handler: func(inv *serpent.Invocation) error { @@ -24,11 +70,5 @@ func TestEnvOptionParsing(t *testing.T) { }, } err := cmd.Invoke().WithOS().Run() - require.NoError(t, err) - - require.Equal(t, o.SetupScript, "setup.sh") - require.Equal(t, o.CacheTTLDays, int64(7)) - require.Equal(t, o.IgnorePaths, []string{"/var", "/tmp"}) - require.True(t, o.SkipRebuild) - require.False(t, o.GitCloneSingleBranch) + return err } From 5d2dd65964ac29d16ebb55b18a05100641cba988 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Mon, 29 Apr 2024 19:38:47 +0000 Subject: [PATCH 5/7] Fix tests and group them --- options_test.go | 104 ++++++++++++++++++++++++++++-------------------- 1 file changed, 60 insertions(+), 44 deletions(-) diff --git a/options_test.go b/options_test.go index 628b7678..4e944d00 100644 --- a/options_test.go +++ b/options_test.go @@ -1,6 +1,7 @@ package envbuilder_test import ( + "bytes" "testing" "github.com/coder/envbuilder" @@ -8,58 +9,57 @@ import ( "github.com/stretchr/testify/require" ) -// TestStrEnvOptions tests that string env variables can be handled as expected. -func TestStrEnvOptions(t *testing.T) { - t.Setenv("SETUP_SCRIPT", "setup.sh") - t.Setenv("INIT_COMMAND", "sleep infinity") +// TestEnvOptionParsing tests that given environment variables of different types are handled as expected. +func TestTestEnvOptionParsing(t *testing.T) { + t.Run("string", func(t *testing.T) { + t.Setenv("SETUP_SCRIPT", "setup.sh") + t.Setenv("INIT_COMMAND", "sleep infinity") - var o envbuilder.Options - err := runCLI(&o) - require.NoError(t, err) + var o envbuilder.Options + err := runCLI(&o) + require.NoError(t, err) - require.Equal(t, o.SetupScript, "setup.sh") - require.Equal(t, o.InitCommand, "sleep infinity") -} + require.Equal(t, o.SetupScript, "setup.sh") + require.Equal(t, o.InitCommand, "sleep infinity") + }) -// TestIntEnvOptions tests that numeric env variables can be handled as expected. -func TestIntEnvOptions(t *testing.T) { - t.Setenv("CACHE_TTL_DAYS", "7") + t.Run("int", func(t *testing.T) { + t.Setenv("CACHE_TTL_DAYS", "7") - var o envbuilder.Options - err := runCLI(&o) - require.NoError(t, err) + var o envbuilder.Options + err := runCLI(&o) + require.NoError(t, err) - require.Equal(t, o.CacheTTLDays, 7) -} + require.Equal(t, o.CacheTTLDays, int64(7)) + }) -// TestMultipleStrEnvOptions tests that numeric env variables can be handled as expected. -func TestMultipleStrEnvOptions(t *testing.T) { - t.Setenv("CACHE_TTL_DAYS", "7") + t.Run("string array", func(t *testing.T) { + t.Setenv("IGNORE_PATHS", "/var,/temp") - var o envbuilder.Options - err := runCLI(&o) - require.NoError(t, err) + var o envbuilder.Options + err := runCLI(&o) + require.NoError(t, err) - require.Equal(t, o.CacheTTLDays, 7) -} + require.Equal(t, o.IgnorePaths, []string{"/var", "/temp"}) + }) + + t.Run("bool", func(t *testing.T) { + t.Setenv("SKIP_REBUILD", "true") + t.Setenv("GIT_CLONE_SINGLE_BRANCH", "false") + t.Setenv("EXIT_ON_BUILD_FAILURE", "true") + t.Setenv("FORCE_SAFE", "false") + t.Setenv("INSECURE", "true") + + var o envbuilder.Options + err := runCLI(&o) + require.NoError(t, err) -// TestBoolEnvOptions tests that boolean env variables can be handled as expected. -func TestBoolEnvOptions(t *testing.T) { - t.Setenv("SKIP_REBUILD", "true") - t.Setenv("GIT_CLONE_SINGLE_BRANCH", "") - t.Setenv("EXIT_ON_BUILD_FAILURE", "false") - t.Setenv("FORCE_SAFE", "TRUE") - t.Setenv("INSECURE", "FALSE") - - var o envbuilder.Options - err := runCLI(&o) - require.NoError(t, err) - - require.True(t, o.SkipRebuild) - require.False(t, o.GitCloneSingleBranch) - require.False(t, o.ExitOnBuildFailure) - require.True(t, o.ForceSafe) - require.False(t, o.Insecure) + require.True(t, o.SkipRebuild) + require.False(t, o.GitCloneSingleBranch) + require.True(t, o.ExitOnBuildFailure) + require.False(t, o.ForceSafe) + require.True(t, o.Insecure) + }) } func runCLI(o *envbuilder.Options) error { @@ -69,6 +69,22 @@ func runCLI(o *envbuilder.Options) error { return nil }, } - err := cmd.Invoke().WithOS().Run() + i := cmd.Invoke().WithOS() + fakeIO(i) + err := i.Run() return err } + +type ioBufs struct { + Stdin bytes.Buffer + Stdout bytes.Buffer + Stderr bytes.Buffer +} + +func fakeIO(i *serpent.Invocation) *ioBufs { + var b ioBufs + i.Stdout = &b.Stdout + i.Stderr = &b.Stderr + i.Stdin = &b.Stdin + return &b +} From ffe9331c8e5a9d85f2240f08a61807b36b0ca2e0 Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Tue, 30 Apr 2024 10:46:35 -0300 Subject: [PATCH 6/7] Expresse the test intent more concisely Co-authored-by: Danny Kopping --- options_test.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/options_test.go b/options_test.go index 4e944d00..0e32c6c0 100644 --- a/options_test.go +++ b/options_test.go @@ -12,15 +12,14 @@ import ( // TestEnvOptionParsing tests that given environment variables of different types are handled as expected. func TestTestEnvOptionParsing(t *testing.T) { t.Run("string", func(t *testing.T) { - t.Setenv("SETUP_SCRIPT", "setup.sh") - t.Setenv("INIT_COMMAND", "sleep infinity") + const val = "setup.sh" + t.Setenv("SETUP_SCRIPT", val) var o envbuilder.Options err := runCLI(&o) require.NoError(t, err) - require.Equal(t, o.SetupScript, "setup.sh") - require.Equal(t, o.InitCommand, "sleep infinity") + require.Equal(t, o.SetupScript, val) }) t.Run("int", func(t *testing.T) { From 3a7e2e5b8028dacbc65342500cf154d1606670b0 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Tue, 30 Apr 2024 13:57:57 +0000 Subject: [PATCH 7/7] Improve readability --- options_test.go | 77 +++++++++++++++++++++++++++---------------------- 1 file changed, 43 insertions(+), 34 deletions(-) diff --git a/options_test.go b/options_test.go index 0e32c6c0..7cfea2db 100644 --- a/options_test.go +++ b/options_test.go @@ -10,68 +10,77 @@ import ( ) // TestEnvOptionParsing tests that given environment variables of different types are handled as expected. -func TestTestEnvOptionParsing(t *testing.T) { +func TestEnvOptionParsing(t *testing.T) { t.Run("string", func(t *testing.T) { - const val = "setup.sh" + const val = "setup.sh" t.Setenv("SETUP_SCRIPT", val) - - var o envbuilder.Options - err := runCLI(&o) - require.NoError(t, err) - + o := runCLI() require.Equal(t, o.SetupScript, val) }) t.Run("int", func(t *testing.T) { t.Setenv("CACHE_TTL_DAYS", "7") - - var o envbuilder.Options - err := runCLI(&o) - require.NoError(t, err) - + o := runCLI() require.Equal(t, o.CacheTTLDays, int64(7)) }) t.Run("string array", func(t *testing.T) { t.Setenv("IGNORE_PATHS", "/var,/temp") - - var o envbuilder.Options - err := runCLI(&o) - require.NoError(t, err) - + o := runCLI() require.Equal(t, o.IgnorePaths, []string{"/var", "/temp"}) }) t.Run("bool", func(t *testing.T) { - t.Setenv("SKIP_REBUILD", "true") - t.Setenv("GIT_CLONE_SINGLE_BRANCH", "false") - t.Setenv("EXIT_ON_BUILD_FAILURE", "true") - t.Setenv("FORCE_SAFE", "false") - t.Setenv("INSECURE", "true") - - var o envbuilder.Options - err := runCLI(&o) - require.NoError(t, err) - - require.True(t, o.SkipRebuild) - require.False(t, o.GitCloneSingleBranch) - require.True(t, o.ExitOnBuildFailure) - require.False(t, o.ForceSafe) - require.True(t, o.Insecure) + t.Run("lowercase", func(t *testing.T) { + t.Setenv("SKIP_REBUILD", "true") + t.Setenv("GIT_CLONE_SINGLE_BRANCH", "false") + o := runCLI() + require.True(t, o.SkipRebuild) + require.False(t, o.GitCloneSingleBranch) + }) + + t.Run("uppercase", func(t *testing.T) { + t.Setenv("SKIP_REBUILD", "TRUE") + t.Setenv("GIT_CLONE_SINGLE_BRANCH", "FALSE") + o := runCLI() + require.True(t, o.SkipRebuild) + require.False(t, o.GitCloneSingleBranch) + }) + + t.Run("numeric", func(t *testing.T) { + t.Setenv("SKIP_REBUILD", "1") + t.Setenv("GIT_CLONE_SINGLE_BRANCH", "0") + o := runCLI() + require.True(t, o.SkipRebuild) + require.False(t, o.GitCloneSingleBranch) + }) + + t.Run("empty", func(t *testing.T) { + t.Setenv("GIT_CLONE_SINGLE_BRANCH", "") + o := runCLI() + require.False(t, o.GitCloneSingleBranch) + }) }) } -func runCLI(o *envbuilder.Options) error { +func runCLI() envbuilder.Options { + var o envbuilder.Options cmd := serpent.Command{ Options: o.CLI(), Handler: func(inv *serpent.Invocation) error { return nil }, } + i := cmd.Invoke().WithOS() fakeIO(i) err := i.Run() - return err + + if err != nil { + panic("failed to run CLI: " + err.Error()) + } + + return o } type ioBufs struct {