Skip to content

Commit 09d3fe8

Browse files
ncodencode
authored
Merge pull request #21 from ncode/ncode/coverage
(feat) start moving things to a factory to simplify to test
2 parents 1436999 + 451c087 commit 09d3fe8

File tree

8 files changed

+753
-55
lines changed

8 files changed

+753
-55
lines changed

‎cmd/cleanup.go‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import (
2020
"log/slog"
2121
"os"
2222

23-
"github.com/hashicorp/consul/api"
23+
"github.com/ncode/tagit/pkg/consul"
2424
"github.com/ncode/tagit/pkg/tagit"
2525
"github.com/spf13/cobra"
2626
)
@@ -34,21 +34,20 @@ var cleanupCmd = &cobra.Command{
3434
Level: slog.LevelInfo,
3535
}))
3636

37-
config := api.DefaultConfig()
38-
config.Address = cmd.InheritedFlags().Lookup("consul-addr").Value.String()
39-
config.Token = cmd.InheritedFlags().Lookup("token").Value.String()
37+
consulAddr := cmd.InheritedFlags().Lookup("consul-addr").Value.String()
38+
token := cmd.InheritedFlags().Lookup("token").Value.String()
4039

41-
consulClient, err := api.NewClient(config)
40+
consulClient, err := consul.CreateClient(consulAddr, token)
4241
if err != nil {
4342
logger.Error("Failed to create Consul client", "error", err)
44-
return fmt.Errorf("failed to create Consul client: %w", err)
43+
return err
4544
}
4645

4746
serviceID := cmd.InheritedFlags().Lookup("service-id").Value.String()
4847
tagPrefix := cmd.InheritedFlags().Lookup("tag-prefix").Value.String()
4948

5049
t := tagit.New(
51-
tagit.NewConsulAPIWrapper(consulClient),
50+
consulClient,
5251
&tagit.CmdExecutor{},
5352
serviceID,
5453
"", // script is not needed for cleanup

‎cmd/cleanup_test.go‎

Lines changed: 229 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,11 @@ package cmd
22

33
import (
44
"bytes"
5+
"fmt"
56
"testing"
67

8+
"github.com/hashicorp/consul/api"
9+
"github.com/ncode/tagit/pkg/consul"
710
"github.com/spf13/cobra"
811
"github.com/stretchr/testify/assert"
912
)
@@ -244,19 +247,36 @@ func TestCleanupCmdFlagRetrieval(t *testing.T) {
244247

245248
func TestCleanupCmdSuccessFlow(t *testing.T) {
246249
// Test the successful flow of cleanup command
250+
// Since the actual cleanupCmd creates a real Consul client internally,
251+
// we test with a mock command that simulates the successful path
247252
cmd := &cobra.Command{Use: "tagit"}
248253
cmd.PersistentFlags().StringP("consul-addr", "c", "127.0.0.1:8500", "consul address")
249254
cmd.PersistentFlags().StringP("service-id", "s", "", "consul service id")
250255
cmd.PersistentFlags().StringP("script", "x", "", "path to script used to generate tags")
251256
cmd.PersistentFlags().StringP("tag-prefix", "p", "tagged", "prefix to be added to tags")
252257
cmd.PersistentFlags().StringP("token", "t", "", "consul token")
253258

259+
var logOutput bytes.Buffer
254260
testCleanupCmd := &cobra.Command{
255261
Use: "cleanup",
256262
Short: "cleanup removes all services with the tag prefix from a given consul service",
257263
RunE: func(cmd *cobra.Command, args []string) error {
258-
// Simulate successful cleanup without actual consul connection
259-
// This tests the success path that returns nil
264+
// Verify all required flags are accessible
265+
serviceID := cmd.InheritedFlags().Lookup("service-id").Value.String()
266+
tagPrefix := cmd.InheritedFlags().Lookup("tag-prefix").Value.String()
267+
consulAddr := cmd.InheritedFlags().Lookup("consul-addr").Value.String()
268+
token := cmd.InheritedFlags().Lookup("token").Value.String()
269+
270+
// Simulate the logging that would happen
271+
fmt.Fprintf(&logOutput, "Starting tag cleanup, serviceID=%s, tagPrefix=%s, consulAddr=%s\n",
272+
serviceID, tagPrefix, consulAddr)
273+
274+
if token != "" {
275+
fmt.Fprintf(&logOutput, "Using token authentication\n")
276+
}
277+
278+
// Simulate successful cleanup
279+
fmt.Fprintf(&logOutput, "Tag cleanup completed successfully\n")
260280
return nil
261281
},
262282
}
@@ -268,8 +288,215 @@ func TestCleanupCmdSuccessFlow(t *testing.T) {
268288
"--script=/tmp/test.sh",
269289
"--consul-addr=localhost:8500",
270290
"--tag-prefix=test",
291+
"--token=secret-token",
271292
})
272293

273294
err := cmd.Execute()
274295
assert.NoError(t, err)
296+
297+
// Verify the command would have executed with the right parameters
298+
output := logOutput.String()
299+
assert.Contains(t, output, "serviceID=test-service")
300+
assert.Contains(t, output, "tagPrefix=test")
301+
assert.Contains(t, output, "consulAddr=localhost:8500")
302+
assert.Contains(t, output, "Using token authentication")
303+
assert.Contains(t, output, "Tag cleanup completed successfully")
304+
}
305+
306+
// MockConsulClient for testing
307+
type MockConsulClient struct {
308+
MockAgent consul.Agent
309+
}
310+
311+
func (m *MockConsulClient) Agent() consul.Agent {
312+
return m.MockAgent
313+
}
314+
315+
// MockAgent implements the Agent interface
316+
type MockAgent struct {
317+
ServiceFunc func(serviceID string, q *api.QueryOptions) (*api.AgentService, *api.QueryMeta, error)
318+
ServiceRegisterFunc func(reg *api.AgentServiceRegistration) error
319+
}
320+
321+
func (m *MockAgent) Service(serviceID string, q *api.QueryOptions) (*api.AgentService, *api.QueryMeta, error) {
322+
if m.ServiceFunc != nil {
323+
return m.ServiceFunc(serviceID, q)
324+
}
325+
return &api.AgentService{
326+
ID: "test-service",
327+
Service: "test",
328+
Tags: []string{"tagged:old", "other-tag"},
329+
}, nil, nil
330+
}
331+
332+
func (m *MockAgent) ServiceRegister(reg *api.AgentServiceRegistration) error {
333+
if m.ServiceRegisterFunc != nil {
334+
return m.ServiceRegisterFunc(reg)
335+
}
336+
return nil
337+
}
338+
339+
func TestCleanupCmdWithMockFactory(t *testing.T) {
340+
// Save and restore the original factory
341+
originalFactory := consul.Factory
342+
defer func() {
343+
consul.Factory = originalFactory
344+
}()
345+
346+
t.Run("Successful cleanup with mock", func(t *testing.T) {
347+
// Create a mock agent that simulates a service with tags
348+
mockAgent := &MockAgent{
349+
ServiceFunc: func(serviceID string, q *api.QueryOptions) (*api.AgentService, *api.QueryMeta, error) {
350+
return &api.AgentService{
351+
ID: serviceID,
352+
Service: "test",
353+
Tags: []string{"tagged-value1", "tagged-value2", "other-tag"},
354+
}, nil, nil
355+
},
356+
ServiceRegisterFunc: func(reg *api.AgentServiceRegistration) error {
357+
// Verify that the tags were cleaned up
358+
assert.Equal(t, "test-service", reg.ID)
359+
assert.NotContains(t, reg.Tags, "tagged-value1")
360+
assert.NotContains(t, reg.Tags, "tagged-value2")
361+
assert.Contains(t, reg.Tags, "other-tag")
362+
return nil
363+
},
364+
}
365+
366+
// Create mock client with the mock agent
367+
mockClient := &MockConsulClient{
368+
MockAgent: mockAgent,
369+
}
370+
371+
// Set up the mock factory
372+
mockFactory := &consul.MockFactory{
373+
MockClient: mockClient,
374+
}
375+
consul.SetFactory(mockFactory)
376+
377+
// Create a new command instance for this test
378+
cmd := &cobra.Command{
379+
Use: "cleanup",
380+
RunE: cleanupCmd.RunE,
381+
}
382+
// Set up parent command for flags inheritance
383+
parent := &cobra.Command{}
384+
parent.PersistentFlags().String("consul-addr", "127.0.0.1:8500", "")
385+
parent.PersistentFlags().String("token", "", "")
386+
parent.PersistentFlags().String("service-id", "test-service", "")
387+
parent.PersistentFlags().String("tag-prefix", "tagged", "")
388+
parent.AddCommand(cmd)
389+
390+
// Run the actual cleanup command
391+
err := cmd.RunE(cmd, []string{})
392+
assert.NoError(t, err)
393+
})
394+
395+
t.Run("Cleanup with connection error", func(t *testing.T) {
396+
// Set up a factory that returns an error
397+
mockFactory := &consul.MockFactory{
398+
MockError: fmt.Errorf("connection failed"),
399+
}
400+
consul.SetFactory(mockFactory)
401+
402+
// Create a new command instance for this test
403+
cmd := &cobra.Command{
404+
Use: "cleanup",
405+
RunE: cleanupCmd.RunE,
406+
}
407+
// Set up parent command for flags inheritance
408+
parent := &cobra.Command{}
409+
parent.PersistentFlags().String("consul-addr", "127.0.0.1:8500", "")
410+
parent.PersistentFlags().String("token", "", "")
411+
parent.PersistentFlags().String("service-id", "test-service", "")
412+
parent.PersistentFlags().String("tag-prefix", "tagged", "")
413+
parent.AddCommand(cmd)
414+
415+
// Run the cleanup command - should fail
416+
err := cmd.RunE(cmd, []string{})
417+
assert.Error(t, err)
418+
})
419+
420+
t.Run("Cleanup with service not found", func(t *testing.T) {
421+
// Create a mock agent that returns nil service
422+
mockAgent := &MockAgent{
423+
ServiceFunc: func(serviceID string, q *api.QueryOptions) (*api.AgentService, *api.QueryMeta, error) {
424+
return nil, nil, nil
425+
},
426+
}
427+
428+
// Create mock client with the mock agent
429+
mockClient := &MockConsulClient{
430+
MockAgent: mockAgent,
431+
}
432+
433+
// Set up the mock factory
434+
mockFactory := &consul.MockFactory{
435+
MockClient: mockClient,
436+
}
437+
consul.SetFactory(mockFactory)
438+
439+
// Create a new command instance for this test
440+
cmd := &cobra.Command{
441+
Use: "cleanup",
442+
RunE: cleanupCmd.RunE,
443+
}
444+
// Set up parent command for flags inheritance
445+
parent := &cobra.Command{}
446+
parent.PersistentFlags().String("consul-addr", "127.0.0.1:8500", "")
447+
parent.PersistentFlags().String("token", "", "")
448+
parent.PersistentFlags().String("service-id", "test-service", "")
449+
parent.PersistentFlags().String("tag-prefix", "tagged", "")
450+
parent.AddCommand(cmd)
451+
452+
// Run the cleanup command - should fail
453+
err := cmd.RunE(cmd, []string{})
454+
assert.Error(t, err)
455+
assert.Contains(t, err.Error(), "service test-service not found")
456+
})
457+
458+
t.Run("Cleanup with service register error", func(t *testing.T) {
459+
// Create a mock agent that simulates a service with tags but fails on register
460+
mockAgent := &MockAgent{
461+
ServiceFunc: func(serviceID string, q *api.QueryOptions) (*api.AgentService, *api.QueryMeta, error) {
462+
return &api.AgentService{
463+
ID: serviceID,
464+
Service: "test",
465+
Tags: []string{"tagged-value1", "other-tag"},
466+
}, nil, nil
467+
},
468+
ServiceRegisterFunc: func(reg *api.AgentServiceRegistration) error {
469+
return fmt.Errorf("failed to register service")
470+
},
471+
}
472+
473+
// Create mock client with the mock agent
474+
mockClient := &MockConsulClient{
475+
MockAgent: mockAgent,
476+
}
477+
478+
// Set up the mock factory
479+
mockFactory := &consul.MockFactory{
480+
MockClient: mockClient,
481+
}
482+
consul.SetFactory(mockFactory)
483+
484+
// Create a new command instance for this test
485+
cmd := &cobra.Command{
486+
Use: "cleanup",
487+
RunE: cleanupCmd.RunE,
488+
}
489+
// Set up parent command for flags inheritance
490+
parent := &cobra.Command{}
491+
parent.PersistentFlags().String("consul-addr", "127.0.0.1:8500", "")
492+
parent.PersistentFlags().String("token", "", "")
493+
parent.PersistentFlags().String("service-id", "test-service", "")
494+
parent.PersistentFlags().String("tag-prefix", "tagged", "")
495+
parent.AddCommand(cmd)
496+
497+
// Run the cleanup command - should fail
498+
err := cmd.RunE(cmd, []string{})
499+
assert.Error(t, err)
500+
assert.Contains(t, err.Error(), "failed to clean up tags")
501+
})
275502
}

‎cmd/run.go‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import (
2424
"syscall"
2525
"time"
2626

27-
"github.com/hashicorp/consul/api"
27+
"github.com/ncode/tagit/pkg/consul"
2828
"github.com/ncode/tagit/pkg/tagit"
2929
"github.com/spf13/cobra"
3030
)
@@ -59,22 +59,21 @@ example: tagit run -s my-super-service -x '/tmp/tag-role.sh'
5959
return fmt.Errorf("invalid interval %q: %w", interval, err)
6060
}
6161

62-
config := api.DefaultConfig()
63-
config.Address, err = cmd.InheritedFlags().GetString("consul-addr")
62+
consulAddr, err := cmd.InheritedFlags().GetString("consul-addr")
6463
if err != nil {
6564
logger.Error("Failed to get consul-addr flag", "error", err)
6665
return err
6766
}
68-
config.Token, err = cmd.InheritedFlags().GetString("token")
67+
token, err := cmd.InheritedFlags().GetString("token")
6968
if err != nil {
7069
logger.Error("Failed to get token flag", "error", err)
7170
return err
7271
}
7372

74-
consulClient, err := api.NewClient(config)
73+
consulClient, err := consul.CreateClient(consulAddr, token)
7574
if err != nil {
7675
logger.Error("Failed to create Consul client", "error", err)
77-
return fmt.Errorf("failed to create Consul client: %w", err)
76+
return err
7877
}
7978

8079
serviceID, err := cmd.InheritedFlags().GetString("service-id")
@@ -94,7 +93,7 @@ example: tagit run -s my-super-service -x '/tmp/tag-role.sh'
9493
}
9594

9695
t := tagit.New(
97-
tagit.NewConsulAPIWrapper(consulClient),
96+
consulClient,
9897
&tagit.CmdExecutor{},
9998
serviceID,
10099
script,

0 commit comments

Comments
 (0)