From 273ccbfbf21b73dcb1c899256ce51042647c89ca Mon Sep 17 00:00:00 2001 From: tongjingran Date: Thu, 16 Jul 2020 12:19:26 +0800 Subject: [PATCH] add uts and optimize expression --- cmd/profile.go | 16 ++--- pkg/cover/client.go | 9 ++- pkg/cover/client_test.go | 133 ++++++++++++++++++++++++--------------- pkg/cover/cover.go | 1 - pkg/cover/server.go | 116 +++++++++++++++++++--------------- pkg/cover/server_test.go | 60 ++++++++++++++++++ pkg/cover/store.go | 10 +-- pkg/cover/store_test.go | 21 ------- 8 files changed, 223 insertions(+), 143 deletions(-) create mode 100644 pkg/cover/server_test.go diff --git a/cmd/profile.go b/cmd/profile.go index 37f549d..1050928 100644 --- a/cmd/profile.go +++ b/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 -goc profile --service=service1,service2 --service=service3 +# 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,service3 -# Get coverage counter of several specified address -goc profile --address=address1,address2 --address=address3 +# 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,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(&addrList, "address", "", nil, "address to get cover profile") - profileCmd.Flags().BoolVarP(&force, "force", "f", false, "force to get cover profile, ignore any internal error") + 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, "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 the coverage counter of all the available services you want") addBasicFlags(profileCmd.Flags()) rootCmd.AddCommand(profileCmd) } diff --git a/pkg/cover/client.go b/pkg/cover/client.go index c526a65..2c8f030 100644 --- a/pkg/cover/client.go +++ b/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 } diff --git a/pkg/cover/client_test.go b/pkg/cover/client_test.go index e242b42..9bff493 100644 --- a/pkg/cover/client_test.go +++ b/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.Address = "http://127.0.0.1:7777" + src.Name = "serviceSuccess" + 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) diff --git a/pkg/cover/cover.go b/pkg/cover/cover.go index 921ec11..201dd57 100644 --- a/pkg/cover/cover.go +++ b/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 diff --git a/pkg/cover/server.go b/pkg/cover/server.go index 4939b73..10da691 100644 --- a/pkg/cover/server.go +++ b/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()}) - return + + address := DefaultStore.Get(service.Name) + 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) - if len(svrList) != 0 && len(addrList) != 0 { - c.JSON(http.StatusExpectationFailed, gin.H{"error": "invalid param"}) - 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 - } - } - } + svrsUnderTest, err := getSvrUnderTest(svrList, addrList, force, svrsAll) + if err != nil { + c.JSON(http.StatusExpectationFailed, gin.H{"error": err.Error()}) } + 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 { - - c.JSON(http.StatusExpectationFailed, gin.H{"error": err.Error()}) - return - } else { + if force { continue } + c.JSON(http.StatusExpectationFailed, gin.H{"error": err.Error()}) + return } profile, err := convertProfile(pp) if err != nil { - if !force { - c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) - return - } else { + if force { 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 +} diff --git a/pkg/cover/server_test.go b/pkg/cover/server_test.go new file mode 100644 index 0000000..1e6d928 --- /dev/null +++ b/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) + } + } +} diff --git a/pkg/cover/store.go b/pkg/cover/store.go index d079d28..a18aa9a 100644 --- a/pkg/cover/store.go +++ b/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) - log.Println(errors.Is(err, ErrStoreDuplicated)) - if errors.Is(err, ErrStoreDuplicated) { - return nil - } + l.memoryStore.Add(s) + // 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) diff --git a/pkg/cover/store_test.go b/pkg/cover/store_test.go index 512fd87..f4bd8aa 100644 --- a/pkg/cover/store_test.go +++ b/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) -}