add uts and optimize expression

cmd/profile.go

@@ -38,13 +38,13 @@ goc profile
 # Get coverage counter from specified register center, the result output to specified file.
 goc profile --center=http://192.168.1.1:8080 --output=./coverage.cov
 
-# Get coverage counter of several specified services
+# Get coverage counter of several specified services. You can get all available service names from command 'goc list'. Use 'service' and 'address' flag at the same time is illegal.
-goc profile --service=service1,service2 --service=service3
+goc profile --service=service1,service2,service3
 
-# Get coverage counter of several specified address
+# Get coverage counter of several specified addresses. You can get all available addresses from command 'goc list'. Use 'service' and 'address' flag at the same time is illegal.
-goc profile --address=address1,address2 --address=address3
+goc profile --address=address1,address2,address3
 
-# Force to get coverage counter, ignore any internal error
+# Force to get the coverage counter of all the available services you want.
 goc profile --force
 `,
 	Run: func(cmd *cobra.Command, args []string) {
@@ -81,9 +81,9 @@ var addrList []string
 
 func init() {
 	profileCmd.Flags().StringVarP(&output, "output", "o", "", "download cover profile")
-	profileCmd.Flags().StringSliceVarP(&svrList, "service", "", nil, "service to get cover profile")
+	profileCmd.Flags().StringSliceVarP(&svrList, "service", "", nil, "get the cover profile of these services, you can get all available service names from command `goc list`, use this flag and 'address' flag at the same time is illegal.")
-	profileCmd.Flags().StringSliceVarP(&addrList, "address", "", nil, "address to get cover profile")
+	profileCmd.Flags().StringSliceVarP(&addrList, "address", "", nil, "get the cover profile of these addresses, you can get all available addresses from command `goc list`, use this flag and 'service' flag at the same time is illegal.")
-	profileCmd.Flags().BoolVarP(&force, "force", "f", false, "force to get cover profile, ignore any internal error")
+	profileCmd.Flags().BoolVarP(&force, "force", "f", false, "force to get the coverage counter of all the available services you want")
 	addBasicFlags(profileCmd.Flags())
 	rootCmd.AddCommand(profileCmd)
 }

pkg/cover/client.go

@@ -17,7 +17,6 @@
 package cover
 
 import (
-	"errors"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -92,6 +91,10 @@ func (c *client) ListServices() ([]byte, error) {
 
 func (c *client) Profile(param ProfileParam) ([]byte, error) {
 	u := fmt.Sprintf("%s%s?force=%s", c.Host, CoverProfileAPI, strconv.FormatBool(param.Force))
+	if len(param.Service) != 0 && len(param.Address) != 0 {
+		return nil, fmt.Errorf("use 'service' and 'address' flag at the same time is illegal")
+	}
+
 	for _, svr := range param.Service {
 		u = u + "&service=" + svr
 	}
@@ -133,10 +136,6 @@ func (c *client) do(method, url string, body io.Reader) ([]byte, error) {
 	if err != nil {
 		return nil, err
 	}
-	if res.StatusCode != http.StatusOK {
-		err = errors.New(string(responseBody))
-		return nil, err
-	}
 	return responseBody, nil
 }
 

pkg/cover/client_test.go

@@ -22,74 +22,109 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"net/http"
 )
 
 func TestClientAction(t *testing.T) {
+	// mock goc server
 	ts := httptest.NewServer(GocServer(os.Stdout))
 	defer ts.Close()
 	var client = NewWorker(ts.URL)
 
+	// mock profile server
+	profileMockResponse := "mode: count\nmockService/main.go:30.13,48.33 13 1"
+	profileSuccessMockSvr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusOK)
+		w.Write([]byte(profileMockResponse))
+	}))
+	defer profileSuccessMockSvr.Close()
+
+	profileErrMockSvr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusOK)
+		w.Write([]byte("error"))
+	}))
+	defer profileErrMockSvr.Close()
+
 	// regsiter service into goc server
 	var src Service
-	src.Name = "goc"
+	src.Name = "serviceSuccess"
-	src.Address = "http://127.0.0.1:7777"
+	src.Address = profileSuccessMockSvr.URL
 	res, err := client.RegisterService(src)
 	assert.NoError(t, err)
 	assert.Contains(t, string(res), "success")
 
-	// get porfile from goc server
-	profileItems := []struct {
-		param ProfileParam
-		err   string
-	}{
-		{
-			param: ProfileParam{Force: false, Service: []string{src.Name}, Address: []string{src.Address}},
-			err:   "invalid param",
-		},
-		{
-			param: ProfileParam{Force: false, Address: []string{src.Address, "http://unknown.com"}},
-			err:   "not found",
-		},
-		{
-			param: ProfileParam{Force: true, Address: []string{src.Address, "http://unknown.com"}},
-			err:   "no profile",
-		},
-		{
-			param: ProfileParam{Force: true, Service: []string{src.Name, "unknownSvr"}},
-			err:   "no profile",
-		},
-		{
-			param: ProfileParam{Force: false, Service: []string{src.Name, "unknownSvr"}},
-			err:   "not found",
-		},
-		{
-			param: ProfileParam{},
-			err:   "connection refused",
-		},
-		{
-			param: ProfileParam{Service: []string{src.Name, src.Name}},
-			err:   "connection refused",
-		},
-		{
-			param: ProfileParam{Address: []string{src.Address, src.Address}},
-			err:   "connection refused",
-		},
-	}
-	for _, item := range profileItems {
-		res, err = client.Profile(item.param)
-		if err != nil {
-			assert.Contains(t, err.Error(), item.err)
-		} else {
-			assert.Contains(t, string(res), item.err)
-		}
-	}
-
 	// do list and check service
 	res, err = client.ListServices()
 	assert.NoError(t, err)
 	assert.Contains(t, string(res), src.Address)
 	assert.Contains(t, string(res), src.Name)
 
+	// get porfile from goc server
+	profileItems := []struct {
+		service Service
+		param   ProfileParam
+		res     string
+	}{
+		{
+			service: Service{Name: "serviceOK", Address: profileSuccessMockSvr.URL},
+			param:   ProfileParam{Force: false, Service: []string{"serviceOK"}, Address: []string{profileSuccessMockSvr.URL}},
+			res:     "use 'service' and 'address' flag at the same time is illegal",
+		},
+		{
+			service: Service{Name: "serviceOK", Address: profileSuccessMockSvr.URL},
+			param:   ProfileParam{},
+			res:     profileMockResponse,
+		},
+		{
+			service: Service{Name: "serviceOK", Address: profileSuccessMockSvr.URL},
+			param:   ProfileParam{Service: []string{"serviceOK"}},
+			res:     profileMockResponse,
+		},
+		{
+			service: Service{Name: "serviceOK", Address: profileSuccessMockSvr.URL},
+			param:   ProfileParam{Address: []string{profileSuccessMockSvr.URL}},
+			res:     profileMockResponse,
+		},
+		{
+			service: Service{Name: "serviceOK", Address: profileSuccessMockSvr.URL},
+			param:   ProfileParam{Service: []string{"unknown"}},
+			res:     "service [unknown] not found",
+		},
+		{
+			service: Service{Name: "serviceErr", Address: profileErrMockSvr.URL},
+			res:     "bad mode line: error",
+		},
+		{
+			service: Service{Name: "serviceErr", Address: profileErrMockSvr.URL},
+			param:   ProfileParam{Force: true},
+			res:     "no profiles",
+		},
+		{
+			service: Service{Name: "serviceNotExist", Address: "http://172.0.0.2:7777"},
+			res:     "connection refused",
+		},
+		{
+			service: Service{Name: "serviceNotExist", Address: "http://172.0.0.2:7777"},
+			param:   ProfileParam{Force: true},
+			res:     "no profiles",
+		},
+	}
+	for _, item := range profileItems {
+		// init server
+		res, err := client.InitSystem()
+		assert.NoError(t, err)
+		// register server
+		res, err = client.RegisterService(item.service)
+		assert.NoError(t, err)
+		assert.Contains(t, string(res), "success")
+		res, err = client.Profile(item.param)
+		if err != nil {
+			assert.Equal(t, err.Error(), item.res)
+		} else {
+			assert.Contains(t, string(res), item.res)
+		}
+	}
+
 	// init system and check service again
 	res, err = client.InitSystem()
 	assert.NoError(t, err)

pkg/cover/cover.go

@@ -41,7 +41,6 @@ import (
 var (
 	ErrCoverPkgFailed  = errors.New("fail to inject code to project")
 	ErrCoverListFailed = errors.New("fail to list package dependencies")
-	ErrStoreDuplicated = errors.New("storage duplicated")
 )
 
 // TestCover is a collection of all counters

pkg/cover/server.go

@@ -119,12 +119,17 @@ func registerService(c *gin.Context) {
 		log.Printf("the registed host %s of service %s is different with the real one %s, here we choose the real one", service.Name, host, realIP)
 		service.Address = fmt.Sprintf("http://%s:%s", realIP, port)
 	}
-	if err := DefaultStore.Add(service); err != nil {
+
-		c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
+	address := DefaultStore.Get(service.Name)
-		return
+	if !contains(address, service.Address) {
+		if err := DefaultStore.Add(service); err != nil {
+			c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
+			return
+		}
 	}
 
 	c.JSON(http.StatusOK, gin.H{"result": "success"})
+	return
 }
 
 func profile(c *gin.Context) {
@@ -136,68 +141,29 @@ func profile(c *gin.Context) {
 	svrList := c.QueryArray("service")
 	addrList := c.QueryArray("address")
 	svrsAll := DefaultStore.GetAll()
-	svrsUnderTest := make(map[string][]string)
+	svrsUnderTest, err := getSvrUnderTest(svrList, addrList, force, svrsAll)
-	if len(svrList) != 0 && len(addrList) != 0 {
+	if err != nil {
-		c.JSON(http.StatusExpectationFailed, gin.H{"error": "invalid param"})
+		c.JSON(http.StatusExpectationFailed, gin.H{"error": err.Error()})
-		return
-	}
-	if len(svrList) == 0 && len(addrList) == 0 {
-		svrsUnderTest = svrsAll
-	} else {
-		if len(svrList) != 0 {
-			for _, name := range svrList {
-				if addr, ok := svrsAll[name]; ok {
-					svrsUnderTest[name] = addr
-					continue
-				}
-				if !force {
-					c.JSON(http.StatusNotFound, fmt.Sprintf("service [%s] not found!", name))
-					return
-				}
-			}
-		}
-		if len(addrList) != 0 {
-		I:
-			for _, addr := range addrList {
-				for svr, addrs := range svrsAll {
-					if contains(svrsUnderTest[svr], addr) {
-						continue I
-					}
-					for _, a := range addrs {
-						if a == addr {
-							svrsUnderTest[svr] = append(svrsUnderTest[svr], a)
-							continue I
-						}
-					}
-				}
-				if !force {
-					c.JSON(http.StatusNotFound, fmt.Sprintf("address [%s] not found!", addr))
-					return
-				}
-			}
-		}
 	}
+
 	var mergedProfiles = make([][]*cover.Profile, 0)
 	for _, svrs := range svrsUnderTest {
 		for _, addr := range svrs {
 			pp, err := NewWorker(addr).Profile(ProfileParam{})
 			if err != nil {
-				if !force {
+				if force {
-
-					c.JSON(http.StatusExpectationFailed, gin.H{"error": err.Error()})
-					return
-				} else {
 					continue
 				}
+				c.JSON(http.StatusExpectationFailed, gin.H{"error": err.Error()})
+				return
 			}
 			profile, err := convertProfile(pp)
 			if err != nil {
-				if !force {
+				if force {
-					c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
-					return
-				} else {
 					continue
 				}
+				c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
+				return
 			}
 			mergedProfiles = append(mergedProfiles, profile)
 		}
@@ -268,3 +234,49 @@ func contains(arr []string, str string) bool {
 	}
 	return false
 }
+
+// getSvrUnderTest get service map by service and address list
+func getSvrUnderTest(svrList, addrList []string, force bool, svrsAll map[string][]string) (svrsUnderTest map[string][]string, err error) {
+	svrsUnderTest = map[string][]string{}
+	if len(svrList) != 0 && len(addrList) != 0 {
+		return nil, fmt.Errorf("use this flag and 'address' flag at the same time is illegal")
+	}
+	// Return all servers when all param is nil
+	if len(svrList) == 0 && len(addrList) == 0 {
+		return svrsAll, nil
+	} else {
+		// Add matched services to map
+		if len(svrList) != 0 {
+			for _, name := range svrList {
+				if addr, ok := svrsAll[name]; ok {
+					svrsUnderTest[name] = addr
+					continue // jump to match the next service
+				}
+				if !force {
+					return nil, fmt.Errorf("service [%s] not found", name)
+				}
+			}
+		}
+		// Add matched addresses to map
+		if len(addrList) != 0 {
+		I:
+			for _, addr := range addrList {
+				for svr, addrs := range svrsAll {
+					if contains(svrsUnderTest[svr], addr) {
+						continue I // The address is duplicate, jump over
+					}
+					for _, a := range addrs {
+						if a == addr {
+							svrsUnderTest[svr] = append(svrsUnderTest[svr], a)
+							continue I // jump to match the next address
+						}
+					}
+				}
+				if !force {
+					return nil, fmt.Errorf("address [%s] not found", addr)
+				}
+			}
+		}
+	}
+	return svrsUnderTest, nil
+}

pkg/cover/server_test.go

@@ -0,0 +1,60 @@
+package cover
+
+import (
+	"github.com/stretchr/testify/assert"
+	"testing"
+)
+
+func TestContains(t *testing.T) {
+	assert.Equal(t, contains([]string{"a", "b"}, "a"), true)
+	assert.Equal(t, contains([]string{"a", "b"}, "c"), false)
+}
+
+func TestGetSvrUnderTest(t *testing.T) {
+	svrAll := map[string][]string{
+		"service1": {"http://127.0.0.1:7777", "http://127.0.0.1:8888"},
+		"service2": {"http://127.0.0.1:9999"},
+	}
+	items := []struct {
+		svrList  []string
+		addrList []string
+		force    bool
+		err      string
+		svrRes   map[string][]string
+	}{
+		{
+			svrList:  []string{"service1"},
+			addrList: []string{"http://127.0.0.1:7777"},
+			err:      "use this flag and 'address' flag at the same time is illegal",
+		},
+		{
+			svrRes: svrAll,
+		},
+		{
+			svrList: []string{"service1", "unknown"},
+			err:     "service [unknown] not found",
+		},
+		{
+			svrList: []string{"service1", "service1", "service2", "unknown"},
+			force:   true,
+			svrRes:  svrAll,
+		},
+		{
+			addrList: []string{"http://127.0.0.1:7777", "http://127.0.0.2:7777"},
+			err:      "address [http://127.0.0.2:7777] not found",
+		},
+		{
+			addrList: []string{"http://127.0.0.1:7777", "http://127.0.0.1:7777", "http://127.0.0.1:9999", "http://127.0.0.2:7777"},
+			force:    true,
+			svrRes:   map[string][]string{"service1": {"http://127.0.0.1:7777"}, "service2": {"http://127.0.0.1:9999"}},
+		},
+	}
+	for _, item := range items {
+		svrs, err := getSvrUnderTest(item.svrList, item.addrList, item.force, svrAll)
+		if err != nil {
+			assert.Equal(t, err.Error(), item.err)
+		} else {
+			assert.Equal(t, svrs, item.svrRes)
+		}
+	}
+}

pkg/cover/store.go

@@ -23,7 +23,6 @@ import (
 	"strings"
 	"sync"
 
-	"errors"
 	log "github.com/sirupsen/logrus"
 )
 
@@ -72,11 +71,8 @@ func NewFileStore() Store {
 
 // Add adds the given service to file Store
 func (l *fileStore) Add(s Service) error {
-	err := l.memoryStore.Add(s)
+	l.memoryStore.Add(s)
-	log.Println(errors.Is(err, ErrStoreDuplicated))
+
-	if errors.Is(err, ErrStoreDuplicated) {
-		return nil
-	}
 	// persistent to local store
 	l.mu.Lock()
 	defer l.mu.Unlock()
@@ -197,7 +193,7 @@ func (l *memoryStore) Add(s Service) error {
 		for _, addr := range addrs {
 			if addr == s.Address {
 				log.Printf("service registered already, name: %s, address: %s", s.Name, s.Address)
-				return ErrStoreDuplicated
+				return nil
 			}
 		}
 		addrs = append(addrs, s.Address)

pkg/cover/store_test.go

@@ -17,7 +17,6 @@
 package cover
 
 import (
-	"github.com/stretchr/testify/assert"
 	"testing"
 )
 
@@ -43,8 +42,6 @@ func TestLocalStore(t *testing.T) {
 	localStore.Add(tc2)
 	localStore.Add(tc3)
 	localStore.Add(tc4)
-	err := localStore.Add(tc1)
-	assert.NoError(t, err)
 	addrs := localStore.Get(tc1.Name)
 	if len(addrs) != 2 {
 		t.Error("unexpected result")
@@ -65,21 +62,3 @@ func TestLocalStore(t *testing.T) {
 		t.Error("local store init failed")
 	}
 }
-
-func TestMemoryStore(t *testing.T) {
-	memoryStore := NewMemoryStore()
-	var tc1 = Service{
-		Name:    "a",
-		Address: "http://127.0.0.1",
-	}
-	err := memoryStore.Add(tc1)
-	if err != nil {
-		t.Error("add server to memory store failed")
-	}
-	addrs := memoryStore.Get(tc1.Name)
-	if len(addrs) != 1 {
-		t.Error("memory store check failed")
-	}
-	err = memoryStore.Add(tc1)
-	assert.Equal(t, err, ErrStoreDuplicated)
-}