Merge pull request #104 from CarlJi/0831

goc profile: add coverfile flag

cmd/diff_test.go

@@ -104,7 +104,3 @@ func TestDoDiffForLocalProfiles(t *testing.T) {
 	}
 
 }
-
-func TestDoDiffUnderProw(t *testing.T) {
-
-}

cmd/profile.go

@@ -19,11 +19,11 @@ package cmd
 import (
 	"bytes"
 	"fmt"
-	log "github.com/sirupsen/logrus"
 	"io"
 	"os"
 
 	"github.com/qiniu/goc/pkg/cover"
+	log "github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 )
 
@@ -44,6 +44,9 @@ goc profile --service=service1,service2,service3
 # 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 may cause ambiguity, please use them separately.
 goc profile --address=address1,address2,address3
 
+# Only get the coverage data of files matching the special patterns
+goc profile --coverfile=pattern1,pattern2,pattern3
+
 # Force fetching all available profiles.
 goc profile --force
 `,
@@ -52,6 +55,7 @@ goc profile --force
 			Force:             force,
 			Service:           svrList,
 			Address:           addrList,
+			CoverFilePatterns: coverFilePatterns,
 		}
 		res, err := cover.NewWorker(center).Profile(p)
 		if err != nil {
@@ -74,16 +78,20 @@ goc profile --force
 	},
 }
 
-var output string
+var (
-var force bool
+	svrList           []string // --service flag
-var svrList []string
+	addrList          []string // --address flag
-var addrList []string
+	force             bool     // --force flag
+	output            string   // --output flag
+	coverFilePatterns []string // --coverfile flag
+)
 
 func init() {
 	profileCmd.Flags().StringVarP(&output, "output", "o", "", "download cover profile")
 	profileCmd.Flags().StringSliceVarP(&svrList, "service", "", nil, "service name to fetch profile, see 'goc list' for all services.")
 	profileCmd.Flags().StringSliceVarP(&addrList, "address", "", nil, "address to fetch profile, see 'goc list' for all addresses.")
 	profileCmd.Flags().BoolVarP(&force, "force", "f", false, "force fetching all available profiles")
+	profileCmd.Flags().StringSliceVarP(&coverFilePatterns, "coverfile", "", nil, "only output coverage data of the files matching the patterns")
 	addBasicFlags(profileCmd.Flags())
 	rootCmd.AddCommand(profileCmd)
 }

pkg/cover/client.go

@@ -17,13 +17,14 @@
 package cover
 
 import (
+	"bytes"
+	"encoding/json"
 	"fmt"
 	"io"
 	"io/ioutil"
 	"net"
 	"net/http"
 	"net/url"
-	"strconv"
 	"strings"
 
 	log "github.com/sirupsen/logrus"
@@ -76,36 +77,36 @@ func (c *client) RegisterService(srv Service) ([]byte, error) {
 		return nil, fmt.Errorf("invalid service name")
 	}
 	u := fmt.Sprintf("%s%s?name=%s&address=%s", c.Host, CoverRegisterServiceAPI, srv.Name, srv.Address)
-	_, res, err := c.do("POST", u, nil)
+	_, res, err := c.do("POST", u, "", nil)
 	return res, err
 }
 
 func (c *client) ListServices() ([]byte, error) {
 	u := fmt.Sprintf("%s%s", c.Host, CoverServicesListAPI)
-	_, services, err := c.do("GET", u, nil)
+	_, services, err := c.do("GET", u, "", nil)
 	if err != nil && isNetworkError(err) {
-		_, services, err = c.do("GET", u, nil)
+		_, services, err = c.do("GET", u, "", nil)
 	}
 
 	return services, err
 }
 
 func (c *client) Profile(param ProfileParam) ([]byte, error) {
-	u := fmt.Sprintf("%s%s?force=%s", c.Host, CoverProfileAPI, strconv.FormatBool(param.Force))
+	u := fmt.Sprintf("%s%s", c.Host, CoverProfileAPI)
 	if len(param.Service) != 0 && len(param.Address) != 0 {
 		return nil, fmt.Errorf("use 'service' flag and 'address' flag at the same time may cause ambiguity, please use them separately")
 	}
 
-	for _, svr := range param.Service {
+	body, err := json.Marshal(param)
-		u = u + "&service=" + svr
+	if err != nil {
+		return nil, fmt.Errorf("json.Marshal failed, param: %v, err:%v", param, err)
 	}
-	for _, addr := range param.Address {
+
-		u = u + "&address=" + addr
+	res, profile, err := c.do("POST", u, "application/json", bytes.NewReader(body))
-	}
-	res, profile, err := c.do("GET", u, nil)
 	if err != nil && isNetworkError(err) {
-		res, profile, err = c.do("GET", u, nil)
+		res, profile, err = c.do("POST", u, "application/json", bytes.NewReader(body))
 	}
+
 	if err == nil && res.StatusCode != 200 {
 		err = fmt.Errorf(string(profile))
 	}
@@ -114,29 +115,35 @@ func (c *client) Profile(param ProfileParam) ([]byte, error) {
 
 func (c *client) Clear() ([]byte, error) {
 	u := fmt.Sprintf("%s%s", c.Host, CoverProfileClearAPI)
-	_, resp, err := c.do("POST", u, nil)
+	_, resp, err := c.do("POST", u, "", nil)
 	if err != nil && isNetworkError(err) {
-		_, resp, err = c.do("POST", u, nil)
+		_, resp, err = c.do("POST", u, "", nil)
 	}
 	return resp, err
 }
 
 func (c *client) InitSystem() ([]byte, error) {
 	u := fmt.Sprintf("%s%s", c.Host, CoverInitSystemAPI)
-	_, body, err := c.do("POST", u, nil)
+	_, body, err := c.do("POST", u, "", nil)
 	return body, err
 }
 
-func (c *client) do(method, url string, body io.Reader) (*http.Response, []byte, error) {
+func (c *client) do(method, url, contentType string, body io.Reader) (*http.Response, []byte, error) {
 	req, err := http.NewRequest(method, url, body)
 	if err != nil {
 		return nil, nil, err
 	}
+
+	if contentType != "" {
+		req.Header.Set("Content-Type", contentType)
+	}
+
 	res, err := c.client.Do(req)
 	if err != nil {
 		return nil, nil, err
 	}
 	defer res.Body.Close()
+
 	responseBody, err := ioutil.ReadAll(res.Body)
 	if err != nil {
 		return res, nil, err

pkg/cover/client_test.go

@@ -33,10 +33,10 @@ func TestClientAction(t *testing.T) {
 	var client = NewWorker(ts.URL)
 
 	// mock profile server
-	profileMockResponse := "mode: count\nmockService/main.go:30.13,48.33 13 1"
+	profileMockResponse := []byte("mode: count\nmockService/main.go:30.13,48.33 13 1\nb/b.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))
+		_, _ = w.Write(profileMockResponse)
 	}))
 	defer profileSuccessMockSvr.Close()
 
@@ -60,65 +60,89 @@ func TestClientAction(t *testing.T) {
 	assert.Contains(t, string(res), src.Address)
 	assert.Contains(t, string(res), src.Name)
 
-	// get porfile from goc server
+	// get profile from goc server
-	profileItems := []struct {
+	tcs := []struct {
+		name        string
 		service     Service
 		param       ProfileParam
-		res     string
+		expected    string
+		expectedErr bool
 	}{
 		{
+			name:        "both service and address existed",
 			service:     Service{Name: "serviceOK", Address: profileSuccessMockSvr.URL},
 			param:       ProfileParam{Force: false, Service: []string{"serviceOK"}, Address: []string{profileSuccessMockSvr.URL}},
-			res:     "use 'service' flag and 'address' flag at the same time may cause ambiguity, please use them separately",
+			expectedErr: true,
 		},
 		{
+			name:     "valid test with no service flag provied",
 			service:  Service{Name: "serviceOK", Address: profileSuccessMockSvr.URL},
 			param:    ProfileParam{},
-			res:     profileMockResponse,
+			expected: "mockService/main.go:30.13,48.33 13 1",
 		},
 		{
+			name:     "valid test with service flag provied",
 			service:  Service{Name: "serviceOK", Address: profileSuccessMockSvr.URL},
 			param:    ProfileParam{Service: []string{"serviceOK"}},
-			res:     profileMockResponse,
+			expected: "mockService/main.go:30.13,48.33 13 1",
 		},
 		{
+			name:     "valid test with address flag provied",
 			service:  Service{Name: "serviceOK", Address: profileSuccessMockSvr.URL},
 			param:    ProfileParam{Address: []string{profileSuccessMockSvr.URL}},
-			res:     profileMockResponse,
+			expected: "mockService/main.go:30.13,48.33 13 1",
 		},
 		{
 			service:     Service{Name: "serviceOK", Address: profileSuccessMockSvr.URL},
 			param:       ProfileParam{Service: []string{"unknown"}},
-			res:     "service [unknown] not found",
+			expected:    "service [unknown] not found",
+			expectedErr: true,
 		},
 		{
 			service:     Service{Name: "serviceErr", Address: profileErrMockSvr.URL},
-			res:     "bad mode line: error",
+			expected:    "bad mode line: error",
+			expectedErr: true,
 		},
 		{
 			service:     Service{Name: "serviceNotExist", Address: "http://172.0.0.2:7777"},
-			res:     "connection refused",
+			expected:    "connection refused",
+			expectedErr: true,
 		},
 		{
 			service:  Service{Name: "serviceNotExist", Address: "http://172.0.0.2:7777"},
 			param:    ProfileParam{Force: true},
-			res:     "no profiles",
+			expected: `{"message":"no profiles"}`,
+		},
+		{
+			name:     "valid test with coverfile flag provied",
+			service:  Service{Name: "serviceOK", Address: profileSuccessMockSvr.URL},
+			param:    ProfileParam{CoverFilePatterns: []string{"b.go$"}},
+			expected: "b/b.go",
 		},
 	}
-	for _, item := range profileItems {
+	for _, tc := range tcs {
+		t.Run(tc.name, func(t *testing.T) {
 			// init server
 			_, err = client.InitSystem()
 			assert.NoError(t, err)
 			// register server
-		res, err = client.RegisterService(item.service)
+			res, err = client.RegisterService(tc.service)
 			assert.NoError(t, err)
 			assert.Contains(t, string(res), "success")
-		res, err = client.Profile(item.param)
+			res, err = client.Profile(tc.param)
 			if err != nil {
-			assert.Contains(t, err.Error(), item.res)
+				if !tc.expectedErr {
-		} else {
+					t.Errorf("unexpected err got: %v", err)
-			assert.Contains(t, string(res), item.res)
 				}
+				return
+			}
+
+			if tc.expectedErr {
+				t.Errorf("Expected an error, but got value %s", string(res))
+			}
+
+			assert.Regexp(t, tc.expected, string(res))
+		})
 	}
 
 	// init system and check service again
@@ -162,6 +186,6 @@ func TestClientDo(t *testing.T) {
 	c := &client{
 		client: http.DefaultClient,
 	}
-	_, _, err := c.do(" ", "http://127.0.0.1:7777", nil) // a invalid method
+	_, _, err := c.do(" ", "http://127.0.0.1:7777", "", nil) // a invalid method
 	assert.Contains(t, err.Error(), "invalid method")
 }

pkg/cover/server.go

@@ -25,7 +25,7 @@ import (
 	"net/http"
 	"net/url"
 	"os"
-	"strconv"
+	"regexp"
 
 	"github.com/gin-gonic/gin"
 	log "github.com/sirupsen/logrus"
@@ -69,6 +69,7 @@ func GocServer(w io.Writer) *gin.Engine {
 	{
 		v1.POST("/cover/register", registerService)
 		v1.GET("/cover/profile", profile)
+		v1.POST("/cover/profile", profile)
 		v1.POST("/cover/clear", clear)
 		v1.POST("/cover/init", initSystem)
 		v1.GET("/cover/list", listServices)
@@ -83,11 +84,12 @@ type Service struct {
 	Address string `form:"address" json:"address" binding:"required"`
 }
 
-// ProfileParam is param of profile API (TODO)
+// ProfileParam is param of profile API
 type ProfileParam struct {
-	Force   bool     `form:"force"`
+	Force             bool     `form:"force" json:"force"`
 	Service           []string `form:"service" json:"service"`
 	Address           []string `form:"address" json:"address"`
+	CoverFilePatterns []string `form:"coverfile" json:"coverfile"`
 }
 
 //listServices list all the registered services
@@ -132,16 +134,18 @@ func registerService(c *gin.Context) {
 	return
 }
 
+// profile API examples:
+// POST /v1/cover/profile
+// { "force": "true", "service":["a","b"], "address":["c","d"],"coverfile":["e","f"] }
 func profile(c *gin.Context) {
-	force, err := strconv.ParseBool(c.Query("force"))
+	var body ProfileParam
-	if err != nil {
+	if err := c.ShouldBind(&body); err != nil {
-		c.JSON(http.StatusExpectationFailed, gin.H{"error": "invalid param"})
+		c.JSON(http.StatusExpectationFailed, gin.H{"error": err.Error()})
 		return
 	}
-	serviceList := removeDuplicateElement(c.QueryArray("service"))
+
-	addressList := removeDuplicateElement(c.QueryArray("address"))
 	allInfos := DefaultStore.GetAll()
-	filterAddrList, err := filterAddrs(serviceList, addressList, force, allInfos)
+	filterAddrList, err := filterAddrs(body.Service, body.Address, body.Force, allInfos)
 	if err != nil {
 		c.JSON(http.StatusExpectationFailed, gin.H{"error": err.Error()})
 		return
@@ -151,13 +155,15 @@ func profile(c *gin.Context) {
 	for _, addr := range filterAddrList {
 		pp, err := NewWorker(addr).Profile(ProfileParam{})
 		if err != nil {
-			if force {
+			if body.Force {
 				log.Warnf("get profile from [%s] failed, error: %s", addr, err.Error())
 				continue
 			}
-			c.JSON(http.StatusExpectationFailed, gin.H{"error": err.Error()})
+
+			c.JSON(http.StatusExpectationFailed, gin.H{"error": fmt.Sprintf("failed to get profile from %s, error %s", addr, err.Error())})
 			return
 		}
+
 		profile, err := convertProfile(pp)
 		if err != nil {
 			c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
@@ -167,7 +173,7 @@ func profile(c *gin.Context) {
 	}
 
 	if len(mergedProfiles) == 0 {
-		c.JSON(http.StatusOK, "no profiles")
+		c.JSON(http.StatusOK, gin.H{"message": "no profiles"})
 		return
 	}
 
@@ -177,12 +183,39 @@ func profile(c *gin.Context) {
 		return
 	}
 
+	if len(body.CoverFilePatterns) > 0 {
+		merged, err = filterProfile(body.CoverFilePatterns, merged)
+		if err != nil {
+			c.JSON(http.StatusInternalServerError, gin.H{"error": fmt.Sprintf("failed to filter profile based on the patterns: %v, error: %v", body.CoverFilePatterns, err)})
+			return
+		}
+	}
+
 	if err := cov.DumpProfile(merged, c.Writer); err != nil {
 		c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
 		return
 	}
 }
 
+// filterProfile filters profiles of the packages matching the coverFile pattern
+func filterProfile(coverFile []string, profiles []*cover.Profile) ([]*cover.Profile, error) {
+	var out = make([]*cover.Profile, 0)
+	for _, profile := range profiles {
+		for _, pattern := range coverFile {
+			matched, err := regexp.MatchString(pattern, profile.FileName)
+			if err != nil {
+				return nil, fmt.Errorf("filterProfile failed with pattern %s for profile %s, err: %v", pattern, profile.FileName, err)
+			}
+			if matched {
+				out = append(out, profile)
+				break // no need to check again for the file
+			}
+		}
+	}
+
+	return out, nil
+}
+
 func clear(c *gin.Context) {
 	svrsUnderTest := DefaultStore.GetAll()
 	for svc, addrs := range svrsUnderTest {
@@ -238,9 +271,11 @@ func filterAddrs(serviceList, addressList []string, force bool, allInfos map[str
 	for _, addr := range allInfos {
 		addressAll = append(addressAll, addr...)
 	}
+
 	if len(serviceList) != 0 && len(addressList) != 0 {
 		return nil, fmt.Errorf("use 'service' flag and 'address' flag at the same time may cause ambiguity, please use them separately")
 	}
+
 	// Add matched services to map
 	for _, name := range serviceList {
 		if addr, ok := allInfos[name]; ok {
@@ -252,6 +287,7 @@ func filterAddrs(serviceList, addressList []string, force bool, allInfos map[str
 		}
 		log.Warnf("service [%s] not found", name)
 	}
+
 	// Add matched addresses to map
 	for _, addr := range addressList {
 		if contains(addressAll, addr) {
@@ -263,22 +299,11 @@ func filterAddrs(serviceList, addressList []string, force bool, allInfos map[str
 		}
 		log.Warnf("address [%s] not found", addr)
 	}
+
 	if len(addressList) == 0 && len(serviceList) == 0 {
 		filterAddrList = addressAll
 	}
+
 	// Return all servers when all param is nil
 	return filterAddrList, nil
 }
-
-// removeDuplicateElement remove duplicate element in slice
-func removeDuplicateElement(addrs []string) []string {
-	result := make([]string, 0, len(addrs))
-	temp := map[string]struct{}{}
-	for _, item := range addrs {
-		if _, ok := temp[item]; !ok {
-			temp[item] = struct{}{}
-			result = append(result, item)
-		}
-	}
-	return result
-}

pkg/cover/server_test.go

@@ -6,11 +6,13 @@ import (
 	"net/http/httptest"
 	"net/url"
 	"os"
+	"reflect"
 	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
+	"golang.org/x/tools/cover"
 )
 
 // MockStore is mock store mainly for unittest
@@ -108,11 +110,6 @@ func TestFilterAddrs(t *testing.T) {
 	}
 }
 
-func TestRemoveDuplicateElement(t *testing.T) {
-	strArr := []string{"a", "a", "b"}
-	assert.Equal(t, removeDuplicateElement(strArr), []string{"a", "b"})
-}
-
 func TestRegisterService(t *testing.T) {
 	router := GocServer(os.Stdout)
 
@@ -178,7 +175,7 @@ func TestProfileService(t *testing.T) {
 	router.ServeHTTP(w, req)
 
 	assert.Equal(t, http.StatusExpectationFailed, w.Code)
-	assert.Contains(t, w.Body.String(), "invalid param")
+	assert.Contains(t, w.Body.String(), "invalid syntax")
 }
 
 func TestClearService(t *testing.T) {
@@ -214,3 +211,120 @@ func TestInitService(t *testing.T) {
 	assert.Equal(t, http.StatusInternalServerError, w.Code)
 	assert.Contains(t, w.Body.String(), "lala error")
 }
+
+func TestFilterProfile(t *testing.T) {
+	var tcs = []struct {
+		name      string
+		pattern   []string
+		input     []*cover.Profile
+		output    []*cover.Profile
+		expectErr bool
+	}{
+		{
+			name:    "normal path",
+			pattern: []string{"some/fancy/gopath", "a/fancy/gopath"},
+			input: []*cover.Profile{
+				{
+					FileName: "some/fancy/gopath/a.go",
+				},
+				{
+					FileName: "some/fancy/gopath/b/a.go",
+				},
+				{
+					FileName: "a/fancy/gopath/a.go",
+				},
+				{
+					FileName: "b/fancy/gopath/a.go",
+				},
+				{
+					FileName: "b/a/fancy/gopath/a.go",
+				},
+			},
+			output: []*cover.Profile{
+				{
+					FileName: "some/fancy/gopath/a.go",
+				},
+				{
+					FileName: "some/fancy/gopath/b/a.go",
+				},
+				{
+					FileName: "a/fancy/gopath/a.go",
+				},
+				{
+					FileName: "b/a/fancy/gopath/a.go",
+				},
+			},
+		},
+		{
+			name:    "with regular expression",
+			pattern: []string{"fancy/gopath/a.go$", "^b/a/"},
+			input: []*cover.Profile{
+				{
+					FileName: "some/fancy/gopath/a.go",
+				},
+				{
+					FileName: "some/fancy/gopath/b/a.go",
+				},
+				{
+					FileName: "a/fancy/gopath/a.go",
+				},
+				{
+					FileName: "b/fancy/gopath/c/a.go",
+				},
+				{
+					FileName: "b/a/fancy/gopath/a.go",
+				},
+			},
+			output: []*cover.Profile{
+				{
+					FileName: "some/fancy/gopath/a.go",
+				},
+				{
+					FileName: "a/fancy/gopath/a.go",
+				},
+				{
+					FileName: "b/a/fancy/gopath/a.go",
+				},
+			},
+		},
+		{
+			name:    "with invalid regular expression",
+			pattern: []string{"(?!a)"},
+			input: []*cover.Profile{
+				{
+					FileName: "some/fancy/gopath/a.go",
+				},
+			},
+			expectErr: true,
+		},
+	}
+
+	for _, tc := range tcs {
+		t.Run(tc.name, func(t *testing.T) {
+			out, err := filterProfile(tc.pattern, tc.input)
+			if err != nil {
+				if !tc.expectErr {
+					t.Errorf("Unexpected error: %v", err)
+				}
+				return
+			}
+
+			if tc.expectErr {
+				t.Errorf("Expected an error, but got value %s", stringifyCoverProfile(out))
+			}
+
+			if !reflect.DeepEqual(out, tc.output) {
+				t.Errorf("Mismatched results. \nExpected: %s\nActual:%s", stringifyCoverProfile(tc.output), stringifyCoverProfile(out))
+			}
+		})
+	}
+}
+
+func stringifyCoverProfile(profiles []*cover.Profile) string {
+	res := make([]cover.Profile, 0, len(profiles))
+	for _, p := range profiles {
+		res = append(res, *p)
+	}
+
+	return fmt.Sprintf("%#v", res)
+}

tests/profile.bats

@@ -77,3 +77,23 @@ setup() {
     wait $profile_pid
     kill -9 $SAMPLE_PID
 }
+
+@test "test goc profile with coverfile flag" {
+    ./simple-project 3>&- &
+    SAMPLE_PID=$!
+    sleep 2
+
+    wait_profile_backend "profile3" &
+    profile_pid=$!
+
+    run gocc profile --center=http://127.0.0.1:60001 --coverfile="a.go$,b.go$" --debug --debugcisyncfile ci-sync.bak;
+    info $output
+    [ "$status" -eq 0 ]
+    [[ "$output" == *"mode: count"* ]]
+    [[ "$output" == *"a.go"* ]] # contains a.go file
+    [[ "$output" == *"b.go"* ]] # contains b.go file
+    [[ "$output" != *"main.go"* ]] # not contains main.go file
+
+    wait $profile_pid
+    kill -9 $SAMPLE_PID
+}

tests/samples/run_for_several_seconds/a/a.go

@@ -0,0 +1,6 @@
+package a
+
+// Say Hello A
+func Say() {
+	println("Hello A")
+}

tests/samples/run_for_several_seconds/b/b.go

@@ -0,0 +1,6 @@
+package b
+
+// Say Hello B
+func Say() {
+	println("Hello B")
+}

tests/samples/run_for_several_seconds/main.go

@@ -3,9 +3,14 @@ package main
 import (
 	"fmt"
 	"time"
+
+	"example.com/simple-project/a"
+	"example.com/simple-project/b"
 )
 
 func main() {
 	fmt.Println("hello")
+	a.Say()
+	b.Say()
 	time.Sleep(time.Second * 15)
 }