fix: address review feedback — testify assertions and dead code removal
- Replace manual t.Fatalf checks with testify/require in resolver tests - Remove dead ctx.Err() check in finalizePreviousContainer — the caller now passes context.WithoutCancel so this branch was unreachable
- commit
- 210b4824f0c0995b30cff90575347a6f139c67ee
- author
- brice · · 2026-03-19 04:03 UTC
- parent
- 8547d4d
- signature
- unsigned
2 files · +12 -26
M internal/adapters/in/cli/controlplane_resolver_test.go +12 -20
| @@ -4,6 +4,8 @@ import ( | |||
| 4 | 4 | "os" | |
| 5 | 5 | "path/filepath" | |
| 6 | 6 | "testing" | |
| 7 | + | ||
| 8 | + | "github.com/stretchr/testify/require" | |
| 7 | 9 | ) | |
| 8 | 10 | ||
| 9 | 11 | func TestResolveControlPlane_LocalAllowedWhenAuthEnabled(t *testing.T) { | |
| @@ -31,18 +33,13 @@ data_dir = "`+filepath.Join(tmpDir, "data")+`" | |||
| 31 | 33 | enabled = true | |
| 32 | 34 | secrets_backend = "unsafe" | |
| 33 | 35 | `), 0o600) | |
| 34 | - | if err != nil { | |
| 35 | - | t.Fatalf("failed to write config: %v", err) | |
| 36 | - | } | |
| 36 | + | require.NoError(t, err) | |
| 37 | 37 | ||
| 38 | 38 | handle, err := resolveControlPlane(configPath) | |
| 39 | - | if err != nil { | |
| 40 | - | t.Fatalf("expected local control-plane handle, got error: %v", err) | |
| 41 | - | } | |
| 42 | - | if handle == nil || handle.plane == nil { | |
| 43 | - | t.Fatalf("expected non-nil local control-plane handle") | |
| 44 | - | } | |
| 45 | - | handle.close() | |
| 39 | + | require.NoError(t, err) | |
| 40 | + | require.NotNil(t, handle) | |
| 41 | + | require.NotNil(t, handle.plane) | |
| 42 | + | defer handle.close() | |
| 46 | 43 | } | |
| 47 | 44 | ||
| 48 | 45 | func TestResolveControlPlane_LocalAllowedWhenAuthDisabled(t *testing.T) { | |
| @@ -70,16 +67,11 @@ data_dir = "`+filepath.Join(tmpDir, "data")+`" | |||
| 70 | 67 | enabled = false | |
| 71 | 68 | secrets_backend = "unsafe" | |
| 72 | 69 | `), 0o600) | |
| 73 | - | if err != nil { | |
| 74 | - | t.Fatalf("failed to write config: %v", err) | |
| 75 | - | } | |
| 70 | + | require.NoError(t, err) | |
| 76 | 71 | ||
| 77 | 72 | handle, err := resolveControlPlane(configPath) | |
| 78 | - | if err != nil { | |
| 79 | - | t.Fatalf("expected local control-plane handle, got error: %v", err) | |
| 80 | - | } | |
| 81 | - | if handle == nil || handle.plane == nil { | |
| 82 | - | t.Fatalf("expected non-nil local control-plane handle") | |
| 83 | - | } | |
| 84 | - | handle.close() | |
| 73 | + | require.NoError(t, err) | |
| 74 | + | require.NotNil(t, handle) | |
| 75 | + | require.NotNil(t, handle.plane) | |
| 76 | + | defer handle.close() | |
| 85 | 77 | } | |
| @@ -4,6 +4,8 @@ import ( | |||||
| 4 | "os" | 4 | "os" | ||
| 5 | "path/filepath" | 5 | "path/filepath" | ||
| 6 | "testing" | 6 | "testing" | ||
| 7 | + | ||||
| 8 | + | "github.com/stretchr/testify/require" | |||
| 7 | ) | 9 | ) | ||
| 8 | 10 | ||||
| 9 | func TestResolveControlPlane_LocalAllowedWhenAuthEnabled(t *testing.T) { | 11 | func TestResolveControlPlane_LocalAllowedWhenAuthEnabled(t *testing.T) { | ||
| @@ -31,18 +33,13 @@ data_dir = "`+filepath.Join(tmpDir, "data")+`" | |||||
| 31 | enabled = true | 33 | enabled = true | ||
| 32 | secrets_backend = "unsafe" | 34 | secrets_backend = "unsafe" | ||
| 33 | `), 0o600) | 35 | `), 0o600) | ||
| 34 | - | if err != nil { | 36 | + | require.NoError(t, err) |
| 35 | - | t.Fatalf("failed to write config: %v", err) | |||
| 36 | - | } | |||
| 37 | 37 | ||||
| 38 | handle, err := resolveControlPlane(configPath) | 38 | handle, err := resolveControlPlane(configPath) | ||
| 39 | - | if err != nil { | 39 | + | require.NoError(t, err) |
| 40 | - | t.Fatalf("expected local control-plane handle, got error: %v", err) | 40 | + | require.NotNil(t, handle) |
| 41 | - | } | 41 | + | require.NotNil(t, handle.plane) |
| 42 | - | if handle == nil || handle.plane == nil { | 42 | + | defer handle.close() |
| 43 | - | t.Fatalf("expected non-nil local control-plane handle") | |||
| 44 | - | } | |||
| 45 | - | handle.close() | |||
| 46 | } | 43 | } | ||
| 47 | 44 | ||||
| 48 | func TestResolveControlPlane_LocalAllowedWhenAuthDisabled(t *testing.T) { | 45 | func TestResolveControlPlane_LocalAllowedWhenAuthDisabled(t *testing.T) { | ||
| @@ -70,16 +67,11 @@ data_dir = "`+filepath.Join(tmpDir, "data")+`" | |||||
| 70 | enabled = false | 67 | enabled = false | ||
| 71 | secrets_backend = "unsafe" | 68 | secrets_backend = "unsafe" | ||
| 72 | `), 0o600) | 69 | `), 0o600) | ||
| 73 | - | if err != nil { | 70 | + | require.NoError(t, err) |
| 74 | - | t.Fatalf("failed to write config: %v", err) | |||
| 75 | - | } | |||
| 76 | 71 | ||||
| 77 | handle, err := resolveControlPlane(configPath) | 72 | handle, err := resolveControlPlane(configPath) | ||
| 78 | - | if err != nil { | 73 | + | require.NoError(t, err) |
| 79 | - | t.Fatalf("expected local control-plane handle, got error: %v", err) | 74 | + | require.NotNil(t, handle) |
| 80 | - | } | 75 | + | require.NotNil(t, handle.plane) |
| 81 | - | if handle == nil || handle.plane == nil { | 76 | + | defer handle.close() |
| 82 | - | t.Fatalf("expected non-nil local control-plane handle") | |||
| 83 | - | } | |||
| 84 | - | handle.close() | |||
| 85 | } | 77 | } | ||
M internal/usecase/container/service.go +0 -6
| @@ -743,17 +743,11 @@ func (s *Service) finalizePreviousContainer(ctx context.Context, domainName stri | |||
| 743 | 743 | if !hasExisting { | |
| 744 | 744 | return | |
| 745 | 745 | } | |
| 746 | - | log := zerowrap.FromCtx(ctx) | |
| 747 | 746 | ||
| 748 | 747 | if invalidated { | |
| 749 | 748 | s.waitForDrain(ctx, existing.ID) | |
| 750 | 749 | } | |
| 751 | 750 | ||
| 752 | - | if err := ctx.Err(); err != nil { | |
| 753 | - | log.Debug().Err(err).Str("domain", domainName).Str("new_container_id", newContainerID).Msg("skipping old container cleanup due to canceled context") | |
| 754 | - | return | |
| 755 | - | } | |
| 756 | - | ||
| 757 | 751 | s.cleanupOldContainer(ctx, existing, newContainerID, domainName) | |
| 758 | 752 | } | |
| 759 | 753 | ||
| @@ -743,17 +743,11 @@ func (s *Service) finalizePreviousContainer(ctx context.Context, domainName stri | |||||
| 743 | if !hasExisting { | 743 | if !hasExisting { | ||
| 744 | return | 744 | return | ||
| 745 | } | 745 | } | ||
| 746 | - | log := zerowrap.FromCtx(ctx) | |||
| 747 | 746 | ||||
| 748 | if invalidated { | 747 | if invalidated { | ||
| 749 | s.waitForDrain(ctx, existing.ID) | 748 | s.waitForDrain(ctx, existing.ID) | ||
| 750 | } | 749 | } | ||
| 751 | 750 | ||||
| 752 | - | if err := ctx.Err(); err != nil { | |||
| 753 | - | log.Debug().Err(err).Str("domain", domainName).Str("new_container_id", newContainerID).Msg("skipping old container cleanup due to canceled context") | |||
| 754 | - | return | |||
| 755 | - | } | |||
| 756 | - | ||||
| 757 | s.cleanupOldContainer(ctx, existing, newContainerID, domainName) | 751 | s.cleanupOldContainer(ctx, existing, newContainerID, domainName) | ||
| 758 | } | 752 | } | ||
| 759 | 753 | ||||