From 919496fa6840f5dae26b7e377f66fcefcf976073 Mon Sep 17 00:00:00 2001 From: Supratik Das Date: Thu, 26 Mar 2020 19:53:02 +0530 Subject: [PATCH] Fix failing tests and tidy up code Signed-off-by: Supratik Das --- acceptance/acceptance_test.go | 13 +++++++++---- builder/config_reader.go | 17 ++++++----------- builder/config_reader_test.go | 8 +++----- 3 files changed, 18 insertions(+), 20 deletions(-) diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index 3646c063c..59779d4af 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -283,8 +283,17 @@ func testAcceptance(t *testing.T, when spec.G, it spec.S, packFixturesDir, packP builderName = value }) + it.After(func() { + h.AssertNil(t, os.RemoveAll(tmpDir)) + }) + when("builder.toml is invalid", func() { it("displays an error", func() { + packVer, err := packVersion(packPath) + h.AssertNil(t, err) + packSemver := semver.MustParse(strings.TrimPrefix(strings.Split(packVer, " ")[0], "v")) + + h.SkipIf(t, packSemver.GreaterThan(semver.MustParse("0.9.0")) || packSemver.Equal(semver.MustParse("0.0.0")), "unexpected behaviour in current pack version") h.CopyFile(t, filepath.Join(packFixturesDir, "invalid_builder.toml"), filepath.Join(tmpDir, "invalid_builder.toml")) _, err := h.RunE(subjectPack("create-builder", "some-builder:build", "--builder-config", filepath.Join(tmpDir, "invalid_builder.toml"))) @@ -292,10 +301,6 @@ func testAcceptance(t *testing.T, when spec.G, it spec.S, packFixturesDir, packP }) }) - it.After(func() { - h.AssertNil(t, os.RemoveAll(tmpDir)) - }) - when("build", func() { var repo, repoName string diff --git a/builder/config_reader.go b/builder/config_reader.go index da864dbe0..45921eacc 100644 --- a/builder/config_reader.go +++ b/builder/config_reader.go @@ -76,7 +76,7 @@ func ReadConfig(path string) (config Config, warnings []string, err error) { } defer file.Close() - warnings, obsoleteConfigs, err := getWarningsForObsoleteFields(file) + warnings, err := getWarningsForObsoleteFields(file) if err != nil { return Config{}, nil, errors.Wrapf(err, "check warnings for file '%s'", path) } @@ -84,7 +84,7 @@ func ReadConfig(path string) (config Config, warnings []string, err error) { return Config{}, nil, errors.Wrap(err, "reset config file pointer") } - config, err = parseConfig(file, builderDir, path, obsoleteConfigs) + config, err = parseConfig(file, builderDir, path) if err != nil { return Config{}, nil, errors.Wrapf(err, "parse contents of '%s'", path) } @@ -96,7 +96,7 @@ func ReadConfig(path string) (config Config, warnings []string, err error) { return config, warnings, nil } -func getWarningsForObsoleteFields(reader io.Reader) ([]string, int, error) { +func getWarningsForObsoleteFields(reader io.Reader) ([]string, error) { var warnings []string var obsoleteConfig = struct { @@ -104,20 +104,19 @@ func getWarningsForObsoleteFields(reader io.Reader) ([]string, int, error) { }{} if _, err := toml.DecodeReader(reader, &obsoleteConfig); err != nil { - return nil, 0, err + return nil, err } if len(obsoleteConfig.Groups) > 0 { warnings = append(warnings, fmt.Sprintf("%s field is obsolete in favor of %s", style.Symbol("groups"), style.Symbol("order"))) } - return warnings, len(obsoleteConfig.Groups), nil + return warnings, nil } // parseConfig reads a builder configuration from reader and resolves relative buildpack paths using `relativeToDir` -func parseConfig(reader io.Reader, relativeToDir, path string, obsoleteConfigs int) (Config, error) { +func parseConfig(reader io.Reader, relativeToDir, path string) (Config, error) { builderConfig := Config{} - fmt.Println(obsoleteConfigs) tomlMetadata, err := toml.DecodeReader(reader, &builderConfig) if err != nil { return Config{}, errors.Wrap(err, "decoding toml contents") @@ -132,10 +131,6 @@ func parseConfig(reader io.Reader, relativeToDir, path string, obsoleteConfigs i pluralizedElement += "s" } - if obsoleteConfigs > 0 { - return builderConfig, nil - } - return Config{}, errors.Errorf("unknown configuration %s %s in %s", pluralizedElement, errorKeys, diff --git a/builder/config_reader_test.go b/builder/config_reader_test.go index 965873748..606b6fca7 100644 --- a/builder/config_reader_test.go +++ b/builder/config_reader_test.go @@ -84,12 +84,10 @@ func testConfig(t *testing.T, when spec.G, it spec.S) { `), 0666)) }) - it("returns warnings", func() { + it("returns error", func() { _, warns, err := builder.ReadConfig(builderConfigPath) - // h.AssertError(t, err, "parse contents of") - h.AssertNil(t, err) - - h.AssertSliceContainsOnly(t, warns, "'groups' field is obsolete in favor of 'order'") + h.AssertError(t, err, "parse contents of") + h.AssertEq(t, len(warns), 0) }) })