Skip to content

Commit

Permalink
feat: use main func ctx for cobra and use ctx in tests
Browse files Browse the repository at this point in the history
Explicitly create the context and set it on the CLI, instead of depending on
NewDockerCli() to instance a default context.

Co-authored-by: Sebastiaan van Stijn <[email protected]>
Co-authored-by: Alano Terblanche <[email protected]>

Signed-off-by: Alano Terblanche <[email protected]>
  • Loading branch information
thaJeztah authored and Benehiko committed Apr 24, 2024
1 parent 8651906 commit 709df33
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 12 deletions.
8 changes: 1 addition & 7 deletions cli-plugins/plugin/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,7 @@ func RunPlugin(dockerCli *command.DockerCli, plugin *cobra.Command, meta manager
PersistentPreRunE = func(cmd *cobra.Command, _ []string) error {
var err error
persistentPreRunOnce.Do(func() {
cmdContext := cmd.Context()
// TODO: revisit and make sure this check makes sense
// see: https://github.com/docker/cli/pull/4599#discussion_r1422487271
if cmdContext == nil {
cmdContext = context.TODO()
}
ctx, cancel := context.WithCancel(cmdContext)
ctx, cancel := context.WithCancel(cmd.Context())
cmd.SetContext(ctx)
// Set up the context to cancel based on signalling via CLI socket.
socket.ConnectAndWait(cancel)
Expand Down
18 changes: 18 additions & 0 deletions cmd/docker/builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ import (
var pluginFilename = "docker-buildx"

func TestBuildWithBuilder(t *testing.T) {
ctx, cancel := context.WithCancel(context.TODO())
defer cancel()

testcases := []struct {
name string
context string
Expand Down Expand Up @@ -64,12 +67,16 @@ echo '{"SchemaVersion":"0.1.0","Vendor":"Docker Inc.","Version":"v0.6.3","ShortD
for _, tt := range testcases {
tt := tt
t.Run(tt.name, func(t *testing.T) {
ctx2, cancel2 := context.WithCancel(ctx)
defer cancel2()

if tt.builder != "" {
t.Setenv("BUILDX_BUILDER", tt.builder)
}

var b bytes.Buffer
dockerCli, err := command.NewDockerCli(
command.WithBaseContext(ctx2),
command.WithAPIClient(&fakeClient{}),
command.WithInputStream(discard),
command.WithCombinedStreams(&b),
Expand Down Expand Up @@ -126,6 +133,9 @@ func (c *fakeClient) Ping(_ context.Context) (types.Ping, error) {
}

func TestBuildkitDisabled(t *testing.T) {
ctx, cancel := context.WithCancel(context.TODO())
defer cancel()

t.Setenv("DOCKER_BUILDKIT", "0")

dir := fs.NewDir(t, t.Name(),
Expand All @@ -136,6 +146,7 @@ func TestBuildkitDisabled(t *testing.T) {
b := bytes.NewBuffer(nil)

dockerCli, err := command.NewDockerCli(
command.WithBaseContext(ctx),
command.WithAPIClient(&fakeClient{}),
command.WithInputStream(discard),
command.WithCombinedStreams(b),
Expand Down Expand Up @@ -163,6 +174,9 @@ func TestBuildkitDisabled(t *testing.T) {
}

func TestBuilderBroken(t *testing.T) {
ctx, cancel := context.WithCancel(context.TODO())
defer cancel()

dir := fs.NewDir(t, t.Name(),
fs.WithFile(pluginFilename, `#!/bin/sh exit 1`, fs.WithMode(0o777)),
)
Expand All @@ -171,6 +185,7 @@ func TestBuilderBroken(t *testing.T) {
b := bytes.NewBuffer(nil)

dockerCli, err := command.NewDockerCli(
command.WithBaseContext(ctx),
command.WithAPIClient(&fakeClient{}),
command.WithInputStream(discard),
command.WithCombinedStreams(b),
Expand Down Expand Up @@ -199,6 +214,8 @@ func TestBuilderBroken(t *testing.T) {

func TestBuilderBrokenEnforced(t *testing.T) {
t.Setenv("DOCKER_BUILDKIT", "1")
ctx, cancel := context.WithCancel(context.TODO())
defer cancel()

dir := fs.NewDir(t, t.Name(),
fs.WithFile(pluginFilename, `#!/bin/sh exit 1`, fs.WithMode(0o777)),
Expand All @@ -208,6 +225,7 @@ func TestBuilderBrokenEnforced(t *testing.T) {
b := bytes.NewBuffer(nil)

dockerCli, err := command.NewDockerCli(
command.WithBaseContext(ctx),
command.WithAPIClient(&fakeClient{}),
command.WithInputStream(discard),
command.WithCombinedStreams(b),
Expand Down
3 changes: 2 additions & 1 deletion cmd/docker/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (

func main() {
ctx := context.Background()

dockerCli, err := command.NewDockerCli(command.WithBaseContext(ctx))
if err != nil {
fmt.Fprintln(os.Stderr, err)
Expand Down Expand Up @@ -352,7 +353,7 @@ func runDocker(ctx context.Context, dockerCli *command.DockerCli) error {
// We've parsed global args already, so reset args to those
// which remain.
cmd.SetArgs(args)
err = cmd.Execute()
err = cmd.ExecuteContext(ctx)

// If the command is being executed in an interactive terminal
// and hook are enabled, run the plugin hooks.
Expand Down
13 changes: 11 additions & 2 deletions cmd/docker/docker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package main

import (
"bytes"
"context"
"io"
"os"
"testing"
Expand All @@ -15,8 +16,10 @@ import (

func TestClientDebugEnabled(t *testing.T) {
defer debug.Disable()
ctx, cancel := context.WithCancel(context.TODO())
defer cancel()

cli, err := command.NewDockerCli()
cli, err := command.NewDockerCli(command.WithBaseContext(ctx))
assert.NilError(t, err)
tcmd := newDockerCommand(cli)
tcmd.SetFlag("debug", "true")
Expand All @@ -39,7 +42,13 @@ func runCliCommand(t *testing.T, r io.ReadCloser, w io.Writer, args ...string) e
if w == nil {
w = io.Discard
}
cli, err := command.NewDockerCli(command.WithInputStream(r), command.WithCombinedStreams(w))
ctx, cancel := context.WithCancel(context.TODO())
defer cancel()

cli, err := command.NewDockerCli(
command.WithBaseContext(ctx),
command.WithInputStream(r),
command.WithCombinedStreams(w))
assert.NilError(t, err)
tcmd := newDockerCommand(cli)

Expand Down
1 change: 1 addition & 0 deletions docs/generate/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ func gen(opts *options) error {
Use: "docker [OPTIONS] COMMAND [ARG...]",
Short: "The base command for the Docker CLI.",
}

clientOpts, _ := cli.SetupRootCommand(cmd)
if err := dockerCLI.Initialize(clientOpts); err != nil {
return err
Expand Down
3 changes: 1 addition & 2 deletions e2e/cli-plugins/plugins/nopersistentprerun/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (

func main() {
plugin.Run(func(dockerCli command.Cli) *cobra.Command {
cmd := &cobra.Command{
return &cobra.Command{
Use: "nopersistentprerun",
Short: "Testing without PersistentPreRun hooks",
// PersistentPreRunE: Not specified, we need to test that it works in the absence of an explicit call
Expand All @@ -25,7 +25,6 @@ func main() {
return nil
},
}
return cmd
},
manager.Metadata{
SchemaVersion: "0.1.0",
Expand Down

0 comments on commit 709df33

Please sign in to comment.