Merge pull request #19 from tukaelu/refactor-roughly

Refactor roughly

cmd/subcommand/cli_host.go

@@ -38,7 +38,7 @@ func NewHostSubcommand() *cli.Command {
 				},
 				host: ctx.String("id"),
 			}
-			return doHost(ctx, cmd)
+			return cmd.run(ctx)
 		},
 		Flags: []cli.Flag{
 			&cli.StringFlag{

cmd/subcommand/cli_service.go

@@ -38,7 +38,7 @@ func NewServiceSubCommand() *cli.Command {
 				withExternal: ctx.Bool("with-external-monitors"),
 			}
 
-			return doService(ctx, cmd)
+			return cmd.run(ctx)
 		},
 		Flags: []cli.Flag{
 			&cli.StringFlag{

cmd/subcommand/cmd_host.go

@@ -23,17 +23,17 @@ type hostCommand struct {
 	host string
 }
 
-func doHost(ctx *cli.Context, h *hostCommand) error {
+func (c *hostCommand) run(ctx *cli.Context) error {
 
-	exportDir, err := fileutil.CreateExportDir(h.host, h.rawFrom, h.rawTo)
+	exportDir, err := fileutil.CreateExportDir(c.host, c.rawFrom, c.rawTo)
 	if err != nil {
 		return err
 	}
 
-	interval := converter.GranularityToInterval(h.granularity)
-	attempts := (h.to-h.from)/interval + 1
+	interval := converter.GranularityToInterval(c.granularity)
+	attempts := (c.to-c.from)/interval + 1
 
-	metricNames, err := h.client.ListHostMetricNames(h.host)
+	metricNames, err := c.client.ListHostMetricNames(c.host)
 	if err != nil {
 		return err
 	}
@@ -47,7 +47,7 @@ func doHost(ctx *cli.Context, h *hostCommand) error {
 		for {
 			select {
 			case res := <-ch:
-				if err := fileutil.WriteFile(exportDir, res.MetricName, "csv", res.Records, h.withFriendly); err != nil {
+				if err := fileutil.WriteFile(exportDir, res.MetricName, "csv", res.Records, c.withFriendly); err != nil {
 					fmt.Printf("Failed to write CSV file. (reason: %s)\n", err)
 					return
 				}
@@ -58,13 +58,13 @@ func doHost(ctx *cli.Context, h *hostCommand) error {
 		}
 	}(ch)
 
-	from := h.from
+	from := c.from
 	to := int64(0)
 	wg := &sync.WaitGroup{}
 	for i := int64(0); i < attempts; i++ {
 		to = from + interval
-		if to > h.to {
-			to = h.to
+		if to > c.to {
+			to = c.to
 		}
 
 		for _, metricName := range metricNames {
@@ -73,7 +73,7 @@ func doHost(ctx *cli.Context, h *hostCommand) error {
 				defer wg.Done()
 
 				metrics, err := retriever.Retrieve(ctx, metricName, func() ([]mackerel.MetricValue, error) {
-					metricValues, err := h.client.FetchHostMetricValues(h.host, metricName, from, to)
+					metricValues, err := c.client.FetchHostMetricValues(c.host, metricName, from, to)
 					if err != nil {
 						return nil, fmt.Errorf("[ERROR] failed to retrieve metrics (metric: %s, from: %d, to: %d) (reason: %s)", metricName, from, to, err)
 					}

cmd/subcommand/cmd_service.go

@@ -24,23 +24,23 @@ type serviceCommand struct {
 	withExternal bool
 }
 
-func doService(ctx *cli.Context, s *serviceCommand) error {
+func (c *serviceCommand) run(ctx *cli.Context) error {
 
-	exportDir, err := fileutil.CreateExportDir(s.name, s.rawFrom, s.rawTo)
+	exportDir, err := fileutil.CreateExportDir(c.name, c.rawFrom, c.rawTo)
 	if err != nil {
 		return err
 	}
 
-	interval := converter.GranularityToInterval(s.granularity)
-	attempts := (s.to-s.from)/interval + 1
+	interval := converter.GranularityToInterval(c.granularity)
+	attempts := (c.to-c.from)/interval + 1
 
-	metricNames, err := s.client.ListServiceMetricNames(s.name)
+	metricNames, err := c.client.ListServiceMetricNames(c.name)
 	if err != nil {
 		return err
 	}
 
-	if s.withExternal {
-		externalMetricNames, err := listExternalMonitorMetricNames(s)
+	if c.withExternal {
+		externalMetricNames, err := c.listExternalMonitorMetricNames()
 		if err != nil {
 			return err
 		}
@@ -60,7 +60,7 @@ func doService(ctx *cli.Context, s *serviceCommand) error {
 		for {
 			select {
 			case res := <-ch:
-				if err := fileutil.WriteFile(exportDir, res.MetricName, "csv", res.Records, s.withFriendly); err != nil {
+				if err := fileutil.WriteFile(exportDir, res.MetricName, "csv", res.Records, c.withFriendly); err != nil {
 					fmt.Printf("Failed to write CSV file. (reason: %s)\n", err)
 					return
 				}
@@ -71,13 +71,13 @@ func doService(ctx *cli.Context, s *serviceCommand) error {
 		}
 	}(ch)
 
-	from := s.from
+	from := c.from
 	to := int64(0)
 	wg := &sync.WaitGroup{}
 	for i := int64(0); i < attempts; i++ {
 		to = from + interval
-		if to > s.to {
-			to = s.to
+		if to > c.to {
+			to = c.to
 		}
 
 		for _, metricName := range metricNames {
@@ -86,7 +86,7 @@ func doService(ctx *cli.Context, s *serviceCommand) error {
 				defer wg.Done()
 
 				metrics, err := retriever.Retrieve(ctx, metricName, func() ([]mackerel.MetricValue, error) {
-					metricValues, err := s.client.FetchServiceMetricValues(s.name, metricName, from, to)
+					metricValues, err := c.client.FetchServiceMetricValues(c.name, metricName, from, to)
 					if err != nil {
 						return nil, fmt.Errorf("[ERROR] failed to retrieve metrics (metric: %s, from: %d, to: %d) (reason: %s)", metricName, from, to, err)
 					}
@@ -117,8 +117,8 @@ func doService(ctx *cli.Context, s *serviceCommand) error {
 	return nil
 }
 
-func listExternalMonitorMetricNames(s *serviceCommand) ([]string, error) {
-	monitors, err := s.client.FindMonitors()
+func (c *serviceCommand) listExternalMonitorMetricNames() ([]string, error) {
+	monitors, err := c.client.FindMonitors()
 	if err != nil {
 		return nil, err
 	}
@@ -129,7 +129,7 @@ func listExternalMonitorMetricNames(s *serviceCommand) ([]string, error) {
 			continue
 		}
 		em := monitor.(*mackerel.MonitorExternalHTTP)
-		if em.Service != s.name {
+		if em.Service != c.name {
 			continue
 		}
 		metricNames = append(metricNames, fmt.Sprintf("__externalhttp.responsetime.%s", monitor.MonitorID()))

cmd/subcommand/cmd_service_test.go

@@ -0,0 +1,54 @@
+package subcommand
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/mackerelio/mackerel-client-go"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestListExternalMonitorMetricNames(t *testing.T) {
+	sv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		resJson := `
+		{
+			"monitors": [
+				{
+					"id": "1XXXXXXXXXX",
+					"type": "host"
+				},
+				{
+					"id": "2XXXXXXXXXX",
+					"type": "external",
+					"service": "test",
+					"url": "https://example.com"
+				},
+				{
+					"id": "3XXXXXXXXXX",
+					"type": "external",
+					"service": "test",
+					"url": "https://example.com"
+				}
+			]
+		}
+		`
+		_, _ = w.Write([]byte(resJson))
+	}))
+	defer sv.Close()
+
+	client, _ := mackerel.NewClientWithOptions("dummy", sv.URL, false)
+
+	c := &serviceCommand{
+		baseCommand: baseCommand{
+			client: client,
+		},
+		name: "test",
+	}
+
+	metricNames, _ := c.listExternalMonitorMetricNames()
+
+	assert.Equal(t, 2, len(metricNames))
+	assert.Equal(t, "__externalhttp.responsetime.2XXXXXXXXXX", metricNames[0])
+	assert.Equal(t, "__externalhttp.responsetime.3XXXXXXXXXX", metricNames[1])
+}

internal/retriever/retriever_test.go

@@ -0,0 +1,29 @@
+package retriever
+
+import (
+	"testing"
+
+	"github.com/mackerelio/mackerel-client-go"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestRetrieve(t *testing.T) {
+	fn := func() ([]mackerel.MetricValue, error) {
+		var metrics []mackerel.MetricValue
+		metrics = append(metrics, mackerel.MetricValue{Time: 1672498800, Value: 1})
+		metrics = append(metrics, mackerel.MetricValue{Time: 1672498860, Value: 2})
+		metrics = append(metrics, mackerel.MetricValue{Time: 1672498920, Value: 3})
+		return metrics, nil
+	}
+	ret, _ := Retrieve(nil, "foo.bar", fn)
+
+	assert.Equal(t, "foo.bar", ret[0].Name)
+	assert.Equal(t, int64(1672498800), ret[0].Time)
+	assert.Equal(t, 1, ret[0].Value)
+	assert.Equal(t, "foo.bar", ret[1].Name)
+	assert.Equal(t, int64(1672498860), ret[1].Time)
+	assert.Equal(t, 2, ret[1].Value)
+	assert.Equal(t, "foo.bar", ret[2].Name)
+	assert.Equal(t, int64(1672498920), ret[2].Time)
+	assert.Equal(t, 3, ret[2].Value)
+}