Skip to content

Commit

Permalink
Port more tests for Use and squash a few bugs
Browse files Browse the repository at this point in the history
  • Loading branch information
@tomasaschan
tomasaschan committed May 26, 2024
1 parent 4f6aa89 commit 8cebb21
Show file tree
Hide file tree
Showing 3 changed files with 112 additions and 30 deletions.
14 changes: 6 additions & 8 deletions pkg/envtest/setup/use/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,18 +56,16 @@ func WithEnvOptions(opts ...env.Option) Option {
func VerifySum(verify bool) Option { return func(c *config) { c.verifySum = !verify } }

func configure(options ...Option) *config {
cfg := &config{}
cfg := &config{
platform: versions.Platform{
OS: runtime.GOOS,
Arch: runtime.GOARCH,
},
}

for _, opt := range options {
opt(cfg)
}

if cfg.platform == (versions.Platform{}) {
cfg.platform = versions.Platform{
OS: runtime.GOOS,
Arch: runtime.GOARCH,
}
}

return cfg
}
35 changes: 25 additions & 10 deletions pkg/envtest/setup/use/use.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package use

import (
"context"
"errors"
"fmt"

"sigs.k8s.io/controller-runtime/pkg/envtest/setup/env"
Expand All @@ -17,6 +18,11 @@ type Result struct {
Path string
}

// ErrNoMatchingVersion is returned when the spec matches no available
// version; available is defined both by versions being published at all,
// but also by other options such as NoDownload.
var ErrNoMatchingVersion = errors.New("no matching version found")

// Use selects an appropriate version based on the user's spec, downloads it if needed,
// and returns the path to the binary asset directory.
func Use(ctx context.Context, version versions.Spec, options ...Option) (Result, error) {
Expand All @@ -37,27 +43,36 @@ func Use(ctx context.Context, version versions.Spec, options ...Option) (Result,
}
}

if !cfg.forceDownload && !version.CheckLatest {
if selected, err := env.SelectLocalVersion(ctx, version, cfg.platform); err != nil {
return Result{}, err
} else if selected != (store.Item{}) {
selectedLocal, err := env.SelectLocalVersion(ctx, version, cfg.platform)
if err != nil {
return Result{}, err
}

if !cfg.forceDownload && !version.CheckLatest || cfg.noDownload {
if selectedLocal != (store.Item{}) {
return Result{
Path: env.PathTo(&selected.Version, selected.Platform),
Version: versions.Spec{Selector: selected.Version},
Platform: selected.Platform,
Path: env.PathTo(&selectedLocal.Version, selectedLocal.Platform),
Version: versions.Spec{Selector: selectedLocal.Version},
Platform: selectedLocal.Platform,
}, nil
}
}

if cfg.noDownload {
return Result{}, fmt.Errorf("no local version matching %s found, but you specified NoDownload()", version)
return Result{}, fmt.Errorf("%w: no local version matching %s found, but you specified NoDownload()", ErrNoMatchingVersion, version)
}

selectedVersion, selectedPlatform, err := env.SelectRemoteVersion(ctx, version, cfg.platform)
if err != nil {
return Result{}, err
}

if selectedLocal != (store.Item{}) && !selectedVersion.NewerThan(selectedLocal.Version) {
return Result{
Path: env.PathTo(&selectedLocal.Version, selectedLocal.Platform),
Version: versions.Spec{Selector: selectedLocal.Version},
Platform: selectedLocal.Platform,
}, nil
}

if err := env.FetchRemoteVersion(ctx, selectedVersion, selectedPlatform, cfg.verifySum); err != nil {
return Result{}, err
}
Expand Down
93 changes: 81 additions & 12 deletions pkg/envtest/setup/use/use_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ func TestEnv(t *testing.T) {

var _ = Describe("Use", func() {
var (
envOpts []env.Option
version = versions.Spec{
defaultEnvOpts []env.Option
version = versions.Spec{
Selector: versions.Concrete{Major: 1, Minor: 16, Patch: 0},
}
)
Expand All @@ -43,8 +43,7 @@ var _ = Describe("Use", func() {

s := testhelpers.NewMockStore()

envOpts = append(
envOpts,
defaultEnvOpts = []env.Option{
env.WithClient(&remote.Client{
Log: testLog.WithName("test-remote-client"),
Bucket: "kubebuilder-tools-test",
Expand All @@ -53,7 +52,7 @@ var _ = Describe("Use", func() {
}),
env.WithStore(s),
env.WithFS(afero.NewIOFS(s.Root)),
)
}
})

Context("when useEnv is set", func() {
Expand All @@ -63,7 +62,7 @@ var _ = Describe("Use", func() {
version,
use.WithAssetsFromEnv(true),
use.WithPlatform("*", "*"),
use.WithEnvOptions(envOpts...),
use.WithEnvOptions(defaultEnvOpts...),
)

Expect(err).NotTo(HaveOccurred())
Expand All @@ -79,7 +78,7 @@ var _ = Describe("Use", func() {
use.WithAssetsFromEnv(true),
use.WithAssetsAt(".test-binaries/missing-binaries"),
use.WithPlatform("*", "*"),
use.WithEnvOptions(envOpts...),
use.WithEnvOptions(defaultEnvOpts...),
)

Expect(err).NotTo(HaveOccurred())
Expand All @@ -95,7 +94,7 @@ var _ = Describe("Use", func() {
use.WithAssetsFromEnv(true),
use.WithAssetsAt("a/good/version"),
use.WithPlatform("*", "*"),
use.WithEnvOptions(envOpts...),
use.WithEnvOptions(defaultEnvOpts...),
)

Expect(err).NotTo(HaveOccurred())
Expand All @@ -111,7 +110,7 @@ var _ = Describe("Use", func() {
use.WithAssetsFromEnv(true),
use.WithAssetsAt("wrong/version"),
use.WithPlatform("*", "*"),
use.WithEnvOptions(envOpts...),
use.WithEnvOptions(defaultEnvOpts...),
)

Expect(err).NotTo(HaveOccurred())
Expand All @@ -121,15 +120,13 @@ var _ = Describe("Use", func() {
})

It("should not need to contact the network", func() {
envOpts = append(envOpts, env.WithClient(nil)) // ensure tests fail if we try to contact remote

result, err := use.Use(
ctx,
version,
use.WithAssetsFromEnv(true),
use.WithAssetsAt("a/good/version"),
use.WithPlatform("*", "*"),
use.WithEnvOptions(envOpts...),
use.WithEnvOptions(append(defaultEnvOpts, env.WithClient(nil))...),
)

Expect(err).NotTo(HaveOccurred())
Expand All @@ -138,4 +135,76 @@ var _ = Describe("Use", func() {
Expect(result.Path).To(HaveSuffix("/good/version"))
})
})

Context("when downloads are disabled", func() {
It("should error if no matches are found locally", func() {
_, err := use.Use(
ctx,
versions.Spec{Selector: versions.Concrete{Major: 9001}},
use.NoDownload(true),
use.WithPlatform("*", "*"),
// ensures tests panic if we try to connect to the network
use.WithEnvOptions(append(defaultEnvOpts, env.WithClient(nil))...),
)
Expect(err).To(HaveOccurred())
Expect(err).To(MatchError(use.ErrNoMatchingVersion))
})

It("should settle for the latest local match if latest is requested", func() {
result, err := use.Use(
ctx,
versions.Spec{
CheckLatest: true,
Selector: versions.PatchSelector{
Major: 1,
Minor: 16,
Patch: versions.AnyPoint,
},
},
use.WithPlatform("*", "*"),
use.NoDownload(true),
use.WithEnvOptions(append(defaultEnvOpts, env.WithClient(nil))...),
)
Expect(err).NotTo(HaveOccurred())
Expect(result.Version).To(Equal(versions.Spec{Selector: versions.Concrete{Major: 1, Minor: 16, Patch: 2}}))
})
})

Context("if latest is requested", func() {
It("should contact the network to see if there's anything newer", func() {
result, err := use.Use(
ctx,
versions.Spec{
CheckLatest: true,
Selector: versions.PatchSelector{
Major: 1,
Minor: 16,
Patch: versions.AnyPoint,
},
},
use.WithPlatform("*", "*"),
use.WithEnvOptions(defaultEnvOpts...),
)
Expect(err).NotTo(HaveOccurred())
Expect(result.Version).To(Equal(versions.Spec{Selector: versions.Concrete{Major: 1, Minor: 16, Patch: 4}}))
})

It("should still use the latest local if the network doesn't have anything newer", func() {
result, err := use.Use(
ctx,
versions.Spec{
CheckLatest: true,
Selector: versions.PatchSelector{
Major: 1,
Minor: 14,
Patch: versions.AnyPoint,
},
},
use.WithPlatform("*", "*"),
use.WithEnvOptions(defaultEnvOpts...),
)
Expect(err).NotTo(HaveOccurred())
Expect(result.Version).To(Equal(versions.Spec{Selector: versions.Concrete{Major: 1, Minor: 14, Patch: 26}}))
})
})
})

0 comments on commit 8cebb21

Please sign in to comment.