Skip to content

Commit

Permalink
flags: Move cache flag constants and tests to enums
Browse files Browse the repository at this point in the history
- Gate acceptance tests to work only when pack supports `--cache`
- string constants for cache type and cache format has been moved to enums
- Fix test cases, update acceptance test helper methods

Signed-off-by: Nitish Gupta <imnitish.ng@gmail.com>
  • Loading branch information
imnitishng committed Jul 6, 2022
1 parent 7d267bc commit 44f55dc
Show file tree
Hide file tree
Showing 7 changed files with 70 additions and 24 deletions.
1 change: 1 addition & 0 deletions acceptance/acceptance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1759,6 +1759,7 @@ func testAcceptance(
when("--cache with options for build cache as image", func() {
var cacheImageName, cacheFlags string
it.Before(func() {
h.SkipIf(t, !pack.SupportsFeature(invoke.Cache), "")
cacheImageName = fmt.Sprintf("%s-cache", repoName)
cacheFlags = fmt.Sprintf("type=build;format=image;name=%s", cacheImageName)
})
Expand Down
12 changes: 10 additions & 2 deletions acceptance/invoke/pack.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package invoke

import (
"bytes"
"fmt"
"io/ioutil"
"os"
"os/exec"
Expand Down Expand Up @@ -210,20 +211,27 @@ func (i *PackInvoker) Supports(command string) bool {
search = command
}

re := regexp.MustCompile(fmt.Sprint(`\b%s\b`, search))
output, err := i.baseCmd(cmdParts...).CombinedOutput()
i.assert.Nil(err)

return strings.Contains(string(output), search) && !strings.Contains(string(output), "Unknown help topic")
return re.MatchString(string(output)) && !strings.Contains(string(output), "Unknown help topic")
}

type Feature int

const CreationTime = iota
const (
CreationTime = iota
Cache
)

var featureTests = map[Feature]func(i *PackInvoker) bool{
CreationTime: func(i *PackInvoker) bool {
return i.Supports("build --creation-time")
},
Cache: func(i *PackInvoker) bool {
return i.Supports("build --cache")
},
}

func (i *PackInvoker) SupportsFeature(f Feature) bool {
Expand Down
2 changes: 1 addition & 1 deletion internal/build/lifecycle_execution.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ func (l *LifecycleExecution) PrevImageName() string {
func (l *LifecycleExecution) Run(ctx context.Context, phaseFactoryCreator PhaseFactoryCreator) error {
phaseFactory := phaseFactoryCreator(l)
var buildCache Cache
if l.opts.CacheImage != "" || (l.opts.Cache.CacheType == "build" && l.opts.Cache.Format == "image") {
if l.opts.CacheImage != "" || (l.opts.Cache.CacheType == cache.Build && l.opts.Cache.Format == cache.CacheImage) {
cacheImageName := l.opts.CacheImage
if cacheImageName == "" {
cacheImageName = l.opts.Cache.Source
Expand Down
8 changes: 4 additions & 4 deletions internal/build/lifecycle_execution_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ func testLifecycleExecution(t *testing.T, when spec.G, it spec.S) {
return fakePhaseFactory
})

h.AssertError(t, err, fmt.Sprintf("invalid cache image name: %s", "could not parse reference: %%!(NOVERB)"))
h.AssertError(t, err, fmt.Sprintf("invalid cache image name: %s", "could not parse reference: %%"))
})

it("fails for cache flags", func() {
Expand All @@ -365,8 +365,8 @@ func testLifecycleExecution(t *testing.T, when spec.G, it spec.S) {
TrustBuilder: false,
UseCreator: false,
Cache: cache.CacheOpts{
CacheType: "build",
Format: "image",
CacheType: cache.Build,
Format: cache.CacheImage,
Source: "%%%",
},
Termui: fakeTermui,
Expand All @@ -379,7 +379,7 @@ func testLifecycleExecution(t *testing.T, when spec.G, it spec.S) {
return fakePhaseFactory
})

h.AssertError(t, err, fmt.Sprintf("invalid cache image name: %s", "could not parse reference: %%!(NOVERB)"))
h.AssertError(t, err, fmt.Sprintf("invalid cache image name: %s", "could not parse reference: %%"))
})
})
})
Expand Down
61 changes: 49 additions & 12 deletions internal/cache/cache_opts.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,43 @@ import (
"github.com/pkg/errors"
)

type Cache int
type Format int
type CacheOpts struct {
CacheType string
Format string
CacheType Cache
Format Format
Source string
}

const (
Build Cache = iota
Launch
)
const (
CacheVolume Format = iota
CacheImage
)

func (c Cache) String() string {
switch c {
case Build:
return "build"
case Launch:
return "launch"
}
return ""
}

func (f Format) String() string {
switch f {
case CacheImage:
return "image"
case CacheVolume:
return "volume"
}
return ""
}

func (c *CacheOpts) Set(value string) error {
csvReader := csv.NewReader(strings.NewReader(value))
csvReader.Comma = ';'
Expand All @@ -30,15 +61,21 @@ func (c *CacheOpts) Set(value string) error {
value := strings.ToLower(parts[1])
switch key {
case "type":
if value != "build" && value != "launch" {
switch value {
case "build":
c.CacheType = Build
case "launch":
c.CacheType = Launch
default:
return errors.Errorf("invalid cache type '%s'", value)
}
c.CacheType = value
case "format":
if value != "image" {
switch value {
case "image":
c.Format = CacheImage
default:
return errors.Errorf("invalid cache format '%s'", value)
}
c.Format = value
case "name":
c.Source = value
}
Expand All @@ -58,10 +95,10 @@ func (c *CacheOpts) Set(value string) error {

func (c *CacheOpts) String() string {
var cacheFlag string
if c.CacheType != "" {
if c.CacheType.String() != "" {
cacheFlag += fmt.Sprintf("type=%s;", c.CacheType)
}
if c.Format != "" {
if c.Format.String() != "" {
cacheFlag += fmt.Sprintf("format=%s;", c.Format)
}
if c.Source != "" {
Expand All @@ -75,11 +112,11 @@ func (c *CacheOpts) Type() string {
}

func populateMissing(c *CacheOpts) error {
if c.CacheType == "" {
c.CacheType = "build"
if c.CacheType.String() == "" {
c.CacheType = Build
}
if c.Format == "" {
c.Format = "volume"
if c.Format.String() == "" {
c.Format = CacheVolume
}
if c.Source == "" {
return errors.Errorf("cache 'name' is required")
Expand Down
8 changes: 4 additions & 4 deletions internal/commands/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -241,15 +241,15 @@ func validateBuildFlags(flags *BuildFlags, cfg config.Config, packClient PackCli
return client.NewExperimentError("Support for buildpack registries is currently experimental.")
}

if flags.Cache.CacheType == "launch" && flags.Cache.Format == "image" {
if flags.Cache.CacheType == cache.Launch && flags.Cache.Format == cache.CacheImage {
logger.Warn("cache definition: 'launch' cache in format 'image' is not supported.")
}

if flags.Cache.String() != "" && flags.CacheImage != "" {
return errors.New("'cache' flag cannot be used with 'cache-image' flag.")
if flags.Cache.Format == cache.CacheImage && flags.CacheImage != "" {
return errors.New("'cache' flag with 'image' format cannot be used with 'cache-image' flag.")
}

if flags.Cache.Format == "image" && !flags.Publish {
if flags.Cache.Format == cache.CacheImage && !flags.Publish {
return errors.New("image cache format requires the 'publish' flag")
}

Expand Down
2 changes: 1 addition & 1 deletion internal/commands/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ func testBuildCommand(t *testing.T, when spec.G, it spec.S) {
it("errors", func() {
command.SetArgs([]string{"--builder", "my-builder", "image", "--cache-image", "some-cache-image", "--cache", "type=build;format=image;name=myorg/myimage:cache"})
err := command.Execute()
h.AssertError(t, err, "'cache' flag cannot be used with 'cache-image' flag.")
h.AssertError(t, err, "'cache' flag with 'image' format cannot be used with 'cache-image' flag")
})
})
})
Expand Down

0 comments on commit 44f55dc

Please sign in to comment.