From ea364b2f417eee12f3bf22c145b7d8594ad623ef Mon Sep 17 00:00:00 2001 From: Dan Duvall Date: Thu, 5 Apr 2018 15:04:23 -0700 Subject: Refactor validation tests without YAML parsing Summary: Since `config.Validate` was changed to take any interface as an argument, many of the validation tests can be refactored in a way that avoids having to parse the full config context in YAML and instead validates each specific config struct directly. The new test pattern is simpler and less prone to future breakage should unrelated parts of the overall config change. Tests that rely on root config context were left unchanged. Test Plan: Run `go test ./...`. Reviewers: thcipriani, demon, hashar, mmodell, #release-engineering-team Reviewed By: thcipriani, #release-engineering-team Tags: #release-engineering-team Differential Revision: https://phabricator.wikimedia.org/D1023 --- config/apt_test.go | 42 ++++++++-------- config/artifacts_test.go | 2 +- config/common_test.go | 23 ++++----- config/config_test.go | 26 +++++----- config/flag_test.go | 2 +- config/lives_test.go | 92 ++++++----------------------------- config/node_test.go | 20 +++----- config/python_test.go | 6 +-- config/reader_test.go | 4 +- config/runs_test.go | 124 ++++++++++++----------------------------------- config/user_test.go | 63 ++++++++++++++++++++++++ config/variant_test.go | 2 +- config/version_test.go | 4 +- 13 files changed, 172 insertions(+), 238 deletions(-) create mode 100644 config/user_test.go diff --git a/config/apt_test.go b/config/apt_test.go index 480c1b7..751bfcd 100644 --- a/config/apt_test.go +++ b/config/apt_test.go @@ -10,7 +10,7 @@ import ( "phabricator.wikimedia.org/source/blubber/config" ) -func TestAptConfig(t *testing.T) { +func TestAptConfigYAML(t *testing.T) { cfg, err := config.ReadConfig([]byte(`--- version: v1 apt: @@ -66,32 +66,30 @@ func TestAptConfigInstructions(t *testing.T) { func TestAptConfigValidation(t *testing.T) { t.Run("packages", func(t *testing.T) { t.Run("ok", func(t *testing.T) { - _, err := config.ReadConfig([]byte(`--- - version: v1 - apt: - packages: - - f1 - - foo-fighter - - bar+b.az - - bar+b.az=0:0.1~foo1-1 - - bar+b.az/stable - - bar+b.az/jessie-wikimedia - variants: {}`)) + err := config.Validate(config.AptConfig{ + Packages: []string{ + "f1", + "foo-fighter", + "bar+b.az", + "bar+b.az=0:0.1~foo1-1", + "bar+b.az/stable", + "bar+b.az/jessie-wikimedia", + }, + }) assert.False(t, config.IsValidationError(err)) }) t.Run("bad", func(t *testing.T) { - _, err := config.ReadConfig([]byte(`--- - version: v1 - apt: - packages: - - foo - - foo fighter - - bar_baz - - 'bar=0.1*bad version' - - bar/0bad_release - variants: {}`)) + err := config.Validate(config.AptConfig{ + Packages: []string{ + "f1", + "foo fighter", + "bar_baz", + "bar=0.1*bad version", + "bar/0bad_release", + }, + }) if assert.True(t, config.IsValidationError(err)) { msg := config.HumanizeValidationError(err) diff --git a/config/artifacts_test.go b/config/artifacts_test.go index 3092572..f538165 100644 --- a/config/artifacts_test.go +++ b/config/artifacts_test.go @@ -9,7 +9,7 @@ import ( "phabricator.wikimedia.org/source/blubber/config" ) -func TestArtifactsConfig(t *testing.T) { +func TestArtifactsConfigYAML(t *testing.T) { cfg, err := config.ReadConfig([]byte(`--- version: v1 base: foo diff --git a/config/common_test.go b/config/common_test.go index 537c025..315630d 100644 --- a/config/common_test.go +++ b/config/common_test.go @@ -8,7 +8,7 @@ import ( "phabricator.wikimedia.org/source/blubber/config" ) -func TestCommonConfig(t *testing.T) { +func TestCommonConfigYAML(t *testing.T) { cfg, err := config.ReadConfig([]byte(`--- version: v1 base: fooimage @@ -33,28 +33,25 @@ func TestCommonConfig(t *testing.T) { func TestCommonConfigValidation(t *testing.T) { t.Run("base", func(t *testing.T) { t.Run("ok", func(t *testing.T) { - _, err := config.ReadConfig([]byte(`--- - version: v1 - base: foo - variants: {}`)) + err := config.Validate(config.CommonConfig{ + Base: "foo", + }) assert.Nil(t, err) }) t.Run("optional", func(t *testing.T) { - _, err := config.ReadConfig([]byte(`--- - version: v1 - base: - variants: {}`)) + err := config.Validate(config.CommonConfig{ + Base: "", + }) assert.False(t, config.IsValidationError(err)) }) t.Run("bad", func(t *testing.T) { - _, err := config.ReadConfig([]byte(`--- - version: v1 - base: foo fighter - variants: {}`)) + err := config.Validate(config.CommonConfig{ + Base: "foo fighter", + }) if assert.True(t, config.IsValidationError(err)) { msg := config.HumanizeValidationError(err) diff --git a/config/config_test.go b/config/config_test.go index f6b6c1e..98e91a7 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -8,7 +8,7 @@ import ( "phabricator.wikimedia.org/source/blubber/config" ) -func TestConfig(t *testing.T) { +func TestConfigYAML(t *testing.T) { cfg, err := config.ReadConfig([]byte(`--- version: v1 variants: @@ -24,21 +24,25 @@ func TestConfig(t *testing.T) { func TestConfigValidation(t *testing.T) { t.Run("variants", func(t *testing.T) { t.Run("ok", func(t *testing.T) { - _, err := config.ReadConfig([]byte(`--- - version: v1 - variants: - build: {} - foo: {}`)) + err := config.Validate(config.Config{ + VersionConfig: config.VersionConfig{Version: "v1"}, + Variants: map[string]config.VariantConfig{ + "build": config.VariantConfig{}, + "foo": config.VariantConfig{}, + }, + }) assert.False(t, config.IsValidationError(err)) }) t.Run("bad", func(t *testing.T) { - _, err := config.ReadConfig([]byte(`--- - version: v1 - variants: - build foo: {} - foo bar: {}`)) + err := config.Validate(config.Config{ + VersionConfig: config.VersionConfig{Version: "v1"}, + Variants: map[string]config.VariantConfig{ + "build foo": config.VariantConfig{}, + "foo bar": config.VariantConfig{}, + }, + }) if assert.True(t, config.IsValidationError(err)) { msg := config.HumanizeValidationError(err) diff --git a/config/flag_test.go b/config/flag_test.go index e7481a6..ef6fbf4 100644 --- a/config/flag_test.go +++ b/config/flag_test.go @@ -8,7 +8,7 @@ import ( "phabricator.wikimedia.org/source/blubber/config" ) -func TestFlagOverwrite(t *testing.T) { +func TestFlagMerge(t *testing.T) { cfg, err := config.ReadConfig([]byte(`--- version: v1 base: foo diff --git a/config/lives_test.go b/config/lives_test.go index d6097b5..06da8f0 100644 --- a/config/lives_test.go +++ b/config/lives_test.go @@ -9,7 +9,7 @@ import ( "phabricator.wikimedia.org/source/blubber/config" ) -func TestLivesConfig(t *testing.T) { +func TestLivesConfigYAML(t *testing.T) { cfg, err := config.ReadConfig([]byte(`--- version: v1 base: foo @@ -95,27 +95,23 @@ func TestLivesConfigInstructions(t *testing.T) { func TestLivesConfigValidation(t *testing.T) { t.Run("in", func(t *testing.T) { t.Run("ok", func(t *testing.T) { - _, err := config.ReadConfig([]byte(`--- - version: v1 - lives: - in: /foo`)) + err := config.Validate(config.LivesConfig{ + In: "/foo", + }) assert.False(t, config.IsValidationError(err)) }) t.Run("optional", func(t *testing.T) { - _, err := config.ReadConfig([]byte(`--- - version: v1 - lives: {}`)) + err := config.Validate(config.LivesConfig{}) assert.False(t, config.IsValidationError(err)) }) t.Run("non-root", func(t *testing.T) { - _, err := config.ReadConfig([]byte(`--- - version: v1 - lives: - in: /`)) + err := config.Validate(config.LivesConfig{ + In: "/", + }) if assert.True(t, config.IsValidationError(err)) { msg := config.HumanizeValidationError(err) @@ -125,10 +121,9 @@ func TestLivesConfigValidation(t *testing.T) { }) t.Run("non-root tricky", func(t *testing.T) { - _, err := config.ReadConfig([]byte(`--- - version: v1 - lives: - in: /foo/..`)) + err := config.Validate(config.LivesConfig{ + In: "/foo/..", + }) if assert.True(t, config.IsValidationError(err)) { msg := config.HumanizeValidationError(err) @@ -138,10 +133,9 @@ func TestLivesConfigValidation(t *testing.T) { }) t.Run("absolute", func(t *testing.T) { - _, err := config.ReadConfig([]byte(`--- - version: v1 - lives: - in: foo/bar`)) + err := config.Validate(config.LivesConfig{ + In: "foo/bar", + }) if assert.True(t, config.IsValidationError(err)) { msg := config.HumanizeValidationError(err) @@ -150,62 +144,4 @@ func TestLivesConfigValidation(t *testing.T) { } }) }) - - t.Run("as", func(t *testing.T) { - t.Run("ok", func(t *testing.T) { - _, err := config.ReadConfig([]byte(`--- - version: v1 - lives: - as: foo-bar.baz`)) - - assert.False(t, config.IsValidationError(err)) - }) - - t.Run("optional", func(t *testing.T) { - _, err := config.ReadConfig([]byte(`--- - version: v1 - lives: {}`)) - - assert.False(t, config.IsValidationError(err)) - }) - - t.Run("no spaces", func(t *testing.T) { - _, err := config.ReadConfig([]byte(`--- - version: v1 - lives: - as: foo bar`)) - - if assert.True(t, config.IsValidationError(err)) { - msg := config.HumanizeValidationError(err) - - assert.Equal(t, `as: "foo bar" is not a valid user name`, msg) - } - }) - - t.Run("long enough", func(t *testing.T) { - _, err := config.ReadConfig([]byte(`--- - version: v1 - lives: - as: fo`)) - - if assert.True(t, config.IsValidationError(err)) { - msg := config.HumanizeValidationError(err) - - assert.Equal(t, `as: "fo" is not a valid user name`, msg) - } - }) - - t.Run("not root", func(t *testing.T) { - _, err := config.ReadConfig([]byte(`--- - version: v1 - lives: - as: root`)) - - if assert.True(t, config.IsValidationError(err)) { - msg := config.HumanizeValidationError(err) - - assert.Equal(t, `as: "root" is not a valid user name`, msg) - } - }) - }) } diff --git a/config/node_test.go b/config/node_test.go index 6b7519a..5d5a39e 100644 --- a/config/node_test.go +++ b/config/node_test.go @@ -9,7 +9,7 @@ import ( "phabricator.wikimedia.org/source/blubber/config" ) -func TestNodeConfig(t *testing.T) { +func TestNodeConfigYAML(t *testing.T) { cfg, err := config.ReadConfig([]byte(`--- version: v1 base: foo @@ -162,27 +162,23 @@ func TestNodeConfigInstructionsEnvironmentOnly(t *testing.T) { func TestNodeConfigValidation(t *testing.T) { t.Run("env", func(t *testing.T) { t.Run("ok", func(t *testing.T) { - _, err := config.ReadConfig([]byte(`--- - version: v1 - node: - env: production`)) + err := config.Validate(config.NodeConfig{ + Env: "production", + }) assert.False(t, config.IsValidationError(err)) }) t.Run("optional", func(t *testing.T) { - _, err := config.ReadConfig([]byte(`--- - version: v1 - node: {}`)) + err := config.Validate(config.NodeConfig{}) assert.False(t, config.IsValidationError(err)) }) t.Run("bad", func(t *testing.T) { - _, err := config.ReadConfig([]byte(`--- - version: v1 - node: - env: foo bar`)) + err := config.Validate(config.NodeConfig{ + Env: "foo bar", + }) if assert.True(t, config.IsValidationError(err)) { msg := config.HumanizeValidationError(err) diff --git a/config/python_test.go b/config/python_test.go index 55e49c7..5c07644 100644 --- a/config/python_test.go +++ b/config/python_test.go @@ -9,7 +9,7 @@ import ( "phabricator.wikimedia.org/source/blubber/config" ) -func TestPythonConfigUnmarshalMerge(t *testing.T) { +func TestPythonConfigYAMLMerge(t *testing.T) { cfg, err := config.ReadConfig([]byte(`--- version: v1 base: foo @@ -35,7 +35,7 @@ func TestPythonConfigUnmarshalMerge(t *testing.T) { } } -func TestPythonConfigMergeEmpty(t *testing.T) { +func TestPythonConfigYAMLMergeEmpty(t *testing.T) { cfg, err := config.ReadConfig([]byte(`--- version: v1 base: foo @@ -57,7 +57,7 @@ func TestPythonConfigMergeEmpty(t *testing.T) { } } -func TestPythonConfigDoNotMergeNil(t *testing.T) { +func TestPythonConfigYAMLDoNotMergeNil(t *testing.T) { cfg, err := config.ReadConfig([]byte(`--- version: v1 base: foo diff --git a/config/reader_test.go b/config/reader_test.go index 51269ea..933e388 100644 --- a/config/reader_test.go +++ b/config/reader_test.go @@ -27,7 +27,7 @@ func ExampleResolveIncludes() { // Output: [varF varD varE varB varC varA] } -func TestReadConfig_ErrorsOnUnknownYAML(t *testing.T) { +func TestReadConfigErrorsOnUnknownYAML(t *testing.T) { _, err := config.ReadConfig([]byte(`--- version: v1 newphone: whodis @@ -41,7 +41,7 @@ func TestReadConfig_ErrorsOnUnknownYAML(t *testing.T) { ) } -func TestReadConfig_ValidateVersionBeforeStrictUnmarshal(t *testing.T) { +func TestReadConfigValidateVersionBeforeStrictUnmarshal(t *testing.T) { _, err := config.ReadConfig([]byte(`--- version: foo newphone: whodis diff --git a/config/runs_test.go b/config/runs_test.go index 8ae87fc..2719f6f 100644 --- a/config/runs_test.go +++ b/config/runs_test.go @@ -9,7 +9,7 @@ import ( "phabricator.wikimedia.org/source/blubber/config" ) -func TestRunsConfig(t *testing.T) { +func TestRunsConfigYAML(t *testing.T) { cfg, err := config.ReadConfig([]byte(`--- version: v1 base: foo @@ -83,95 +83,35 @@ func TestRunsConfigInstructions(t *testing.T) { } func TestRunsConfigValidation(t *testing.T) { - t.Run("as", func(t *testing.T) { - t.Run("ok", func(t *testing.T) { - _, err := config.ReadConfig([]byte(`--- - version: v1 - runs: - as: foo-bar.baz`)) - - assert.False(t, config.IsValidationError(err)) - }) - - t.Run("optional", func(t *testing.T) { - _, err := config.ReadConfig([]byte(`--- - version: v1 - runs: {}`)) - - assert.False(t, config.IsValidationError(err)) - }) - - t.Run("no spaces", func(t *testing.T) { - _, err := config.ReadConfig([]byte(`--- - version: v1 - runs: - as: foo bar`)) - - if assert.True(t, config.IsValidationError(err)) { - msg := config.HumanizeValidationError(err) - - assert.Equal(t, `as: "foo bar" is not a valid user name`, msg) - } - }) - - t.Run("long enough", func(t *testing.T) { - _, err := config.ReadConfig([]byte(`--- - version: v1 - runs: - as: fo`)) - - if assert.True(t, config.IsValidationError(err)) { - msg := config.HumanizeValidationError(err) - - assert.Equal(t, `as: "fo" is not a valid user name`, msg) - } - }) - - t.Run("not root", func(t *testing.T) { - _, err := config.ReadConfig([]byte(`--- - version: v1 - runs: - as: root`)) - - if assert.True(t, config.IsValidationError(err)) { - msg := config.HumanizeValidationError(err) - - assert.Equal(t, `as: "root" is not a valid user name`, msg) - } - }) - }) - t.Run("environment", func(t *testing.T) { t.Run("ok", func(t *testing.T) { - _, err := config.ReadConfig([]byte(`--- - version: v1 - runs: - environment: - foo: bar - f1oo: bar - FOO: bar - foo_fighter: bar - FOO_FIGHTER: bar - _FOO_FIGHTER: bar`)) + err := config.Validate(config.RunsConfig{ + Environment: map[string]string{ + "foo": "bar", + "f1oo": "bar", + "FOO": "bar", + "foo_fighter": "bar", + "FOO_FIGHTER": "bar", + "_FOO_FIGHTER": "bar", + }, + }) assert.False(t, config.IsValidationError(err)) }) t.Run("optional", func(t *testing.T) { - _, err := config.ReadConfig([]byte(`--- - version: v1 - runs: {}`)) + err := config.Validate(config.RunsConfig{}) assert.False(t, config.IsValidationError(err)) }) t.Run("bad", func(t *testing.T) { t.Run("spaces", func(t *testing.T) { - _, err := config.ReadConfig([]byte(`--- - version: v1 - runs: - environment: - foo fighter: bar`)) + err := config.Validate(config.RunsConfig{ + Environment: map[string]string{ + "foo fighter": "bar", + }, + }) if assert.True(t, config.IsValidationError(err)) { msg := config.HumanizeValidationError(err) @@ -181,11 +121,11 @@ func TestRunsConfigValidation(t *testing.T) { }) t.Run("dashes", func(t *testing.T) { - _, err := config.ReadConfig([]byte(`--- - version: v1 - runs: - environment: - foo-fighter: bar`)) + err := config.Validate(config.RunsConfig{ + Environment: map[string]string{ + "foo-fighter": "bar", + }, + }) if assert.True(t, config.IsValidationError(err)) { msg := config.HumanizeValidationError(err) @@ -195,11 +135,11 @@ func TestRunsConfigValidation(t *testing.T) { }) t.Run("dots", func(t *testing.T) { - _, err := config.ReadConfig([]byte(`--- - version: v1 - runs: - environment: - foo.fighter: bar`)) + err := config.Validate(config.RunsConfig{ + Environment: map[string]string{ + "foo.fighter": "bar", + }, + }) if assert.True(t, config.IsValidationError(err)) { msg := config.HumanizeValidationError(err) @@ -209,11 +149,11 @@ func TestRunsConfigValidation(t *testing.T) { }) t.Run("starts with number", func(t *testing.T) { - _, err := config.ReadConfig([]byte(`--- - version: v1 - runs: - environment: - 1foo: bar`)) + err := config.Validate(config.RunsConfig{ + Environment: map[string]string{ + "1foo": "bar", + }, + }) if assert.True(t, config.IsValidationError(err)) { msg := config.HumanizeValidationError(err) diff --git a/config/user_test.go b/config/user_test.go new file mode 100644 index 0000000..bb19296 --- /dev/null +++ b/config/user_test.go @@ -0,0 +1,63 @@ +package config_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "phabricator.wikimedia.org/source/blubber/config" +) + +func TestUserConfigValidation(t *testing.T) { + t.Run("as", func(t *testing.T) { + t.Run("ok", func(t *testing.T) { + err := config.Validate(config.UserConfig{ + As: "foo-bar.baz", + }) + + assert.False(t, config.IsValidationError(err)) + }) + + t.Run("optional", func(t *testing.T) { + err := config.Validate(config.UserConfig{}) + + assert.False(t, config.IsValidationError(err)) + }) + + t.Run("no spaces", func(t *testing.T) { + err := config.Validate(config.UserConfig{ + As: "foo bar", + }) + + if assert.True(t, config.IsValidationError(err)) { + msg := config.HumanizeValidationError(err) + + assert.Equal(t, `as: "foo bar" is not a valid user name`, msg) + } + }) + + t.Run("long enough", func(t *testing.T) { + err := config.Validate(config.UserConfig{ + As: "fo", + }) + + if assert.True(t, config.IsValidationError(err)) { + msg := config.HumanizeValidationError(err) + + assert.Equal(t, `as: "fo" is not a valid user name`, msg) + } + }) + + t.Run("not root", func(t *testing.T) { + err := config.Validate(config.UserConfig{ + As: "root", + }) + + if assert.True(t, config.IsValidationError(err)) { + msg := config.HumanizeValidationError(err) + + assert.Equal(t, `as: "root" is not a valid user name`, msg) + } + }) + }) +} diff --git a/config/variant_test.go b/config/variant_test.go index f5216f6..cd51143 100644 --- a/config/variant_test.go +++ b/config/variant_test.go @@ -10,7 +10,7 @@ import ( "phabricator.wikimedia.org/source/blubber/config" ) -func TestVariantConfig(t *testing.T) { +func TestVariantConfigYAML(t *testing.T) { cfg, err := config.ReadConfig([]byte(`--- version: v1 base: foo diff --git a/config/version_test.go b/config/version_test.go index 07df93e..628b31a 100644 --- a/config/version_test.go +++ b/config/version_test.go @@ -8,7 +8,7 @@ import ( "phabricator.wikimedia.org/source/blubber/config" ) -func TestVersionConfig_YAML(t *testing.T) { +func TestVersionConfigYAML(t *testing.T) { cfg, err := config.ReadConfig([]byte(`--- version: v1 variants: @@ -21,7 +21,7 @@ func TestVersionConfig_YAML(t *testing.T) { } } -func TestVersionConfig_Validation(t *testing.T) { +func TestVersionConfigValidation(t *testing.T) { t.Run("supported version", func(t *testing.T) { err := config.Validate(config.VersionConfig{ Version: "v1", -- cgit v1.2.1