From 12269a43977148f230445f0da83b102ee306f3e6 Mon Sep 17 00:00:00 2001 From: moogacs Date: Sun, 29 Dec 2024 13:55:13 +0100 Subject: [PATCH] address more comments --- cleanup.go | 22 +++++++++++----------- docker.go | 16 +--------------- docker_test.go | 24 ++++++++++++------------ 3 files changed, 24 insertions(+), 38 deletions(-) diff --git a/cleanup.go b/cleanup.go index 7a6564fc15..74d275910c 100644 --- a/cleanup.go +++ b/cleanup.go @@ -11,20 +11,20 @@ import ( // DefaultTimeout for termination var DefaultTimeout = 10 * time.Second -// terminateOptions is a type that holds the options for terminating a container. -type terminateOptions struct { +// TerminateOptions is a type that holds the options for terminating a container. +type TerminateOptions struct { ctx context.Context stopTimeout *time.Duration volumes []string } // TerminateOption is a type that represents an option for terminating a container. -type TerminateOption func(*terminateOptions) +type TerminateOption func(*TerminateOptions) // NewTerminateOptions returns a fully initialised TerminateOptions. // Defaults: StopTimeout: 10 seconds. -func NewTerminateOptions(ctx context.Context, opts ...TerminateOption) *terminateOptions { - options := &terminateOptions{ +func NewTerminateOptions(ctx context.Context, opts ...TerminateOption) *TerminateOptions { + options := &TerminateOptions{ stopTimeout: &DefaultTimeout, ctx: ctx, } @@ -35,17 +35,17 @@ func NewTerminateOptions(ctx context.Context, opts ...TerminateOption) *terminat } // Context returns the context to use duration a Terminate. -func (o *terminateOptions) Context() context.Context { +func (o *TerminateOptions) Context() context.Context { return o.ctx } // StopTimeout returns the stop timeout to use duration a Terminate. -func (o *terminateOptions) StopTimeout() *time.Duration { +func (o *TerminateOptions) StopTimeout() *time.Duration { return o.stopTimeout } // Cleanup performs any clean up needed -func (o *terminateOptions) Cleanup() error { +func (o *TerminateOptions) Cleanup() error { // TODO: simplify this when when perform the client refactor. if len(o.volumes) == 0 { return nil @@ -68,7 +68,7 @@ func (o *terminateOptions) Cleanup() error { // StopContext returns a TerminateOption that sets the context. // Default: context.Background(). func StopContext(ctx context.Context) TerminateOption { - return func(c *terminateOptions) { + return func(c *TerminateOptions) { c.ctx = ctx } } @@ -76,7 +76,7 @@ func StopContext(ctx context.Context) TerminateOption { // StopTimeout returns a TerminateOption that sets the timeout. // Default: See [Container.Stop]. func StopTimeout(timeout time.Duration) TerminateOption { - return func(c *terminateOptions) { + return func(c *TerminateOptions) { c.stopTimeout = &timeout } } @@ -86,7 +86,7 @@ func StopTimeout(timeout time.Duration) TerminateOption { // which are not removed by default. // Default: nil. func RemoveVolumes(volumes ...string) TerminateOption { - return func(c *terminateOptions) { + return func(c *TerminateOptions) { c.volumes = volumes } } diff --git a/docker.go b/docker.go index 7351091a85..c8467b2a43 100644 --- a/docker.go +++ b/docker.go @@ -296,20 +296,6 @@ func (c *DockerContainer) Stop(ctx context.Context, timeout *time.Duration) erro return nil } -// WithStopTimeout is a functional option that sets the timeout for the container termination. -func WithStopTimeout(timeout time.Duration) TerminateOption { - return func(c *terminateOptions) { - c.stopTimeout = &timeout - } -} - -// WithTerminateVolumes is a functional option that sets the volumes for the container termination. -func WithTerminateVolumes(volumes ...string) TerminateOption { - return func(opts *terminateOptions) { - opts.volumes = volumes - } -} - // Terminate calls stops and then removes the container including its volumes. // If its image was built it and all child images are also removed unless // the [FromDockerfile.KeepImage] on the [ContainerRequest] was set to true. @@ -356,7 +342,7 @@ func (c *DockerContainer) Terminate(ctx context.Context, opts ...TerminateOption c.sessionID = "" c.isRunning = false - if err := options.Cleanup(); err != nil { + if err = options.Cleanup(); err != nil { errs = append(errs, err) } diff --git a/docker_test.go b/docker_test.go index 884dedee6c..b9cbf22cd8 100644 --- a/docker_test.go +++ b/docker_test.go @@ -287,7 +287,7 @@ func TestContainerStateAfterTermination(t *testing.T) { err = nginx.Start(ctx) require.NoError(t, err, "expected no error from container start.") - err = nginx.Terminate(ctx, WithStopTimeout(5*time.Microsecond)) + err = nginx.Terminate(ctx, StopTimeout(5*time.Microsecond)) require.NoError(t, err) }) @@ -1127,34 +1127,34 @@ func TestContainerCreationWithVolumeCleaning(t *testing.T) { Started: true, }) require.NoError(t, err) - err = bashC.Terminate(ctx, WithTerminateVolumes(volumeName)) + err = bashC.Terminate(ctx, RemoveVolumes(volumeName)) CleanupContainer(t, bashC, RemoveVolumes(volumeName)) require.NoError(t, err) } func TestContainerTerminationOptions(t *testing.T) { t.Run("volumes", func(t *testing.T) { - var options terminateOptions - WithTerminateVolumes("vol1", "vol2")(&options) - require.Equal(t, terminateOptions{ + var options TerminateOptions + RemoveVolumes("vol1", "vol2")(&options) + require.Equal(t, TerminateOptions{ volumes: []string{"vol1", "vol2"}, }, options) }) t.Run("stop-timeout", func(t *testing.T) { - var options terminateOptions + var options TerminateOptions timeout := 11 * time.Second - WithStopTimeout(timeout)(&options) - require.Equal(t, terminateOptions{ + StopTimeout(timeout)(&options) + require.Equal(t, TerminateOptions{ stopTimeout: &timeout, }, options) }) t.Run("all", func(t *testing.T) { - var options terminateOptions + var options TerminateOptions timeout := 9 * time.Second - WithStopTimeout(timeout)(&options) - WithTerminateVolumes("vol1", "vol2")(&options) - require.Equal(t, terminateOptions{ + StopTimeout(timeout)(&options) + RemoveVolumes("vol1", "vol2")(&options) + require.Equal(t, TerminateOptions{ stopTimeout: &timeout, volumes: []string{"vol1", "vol2"}, }, options)