Merge pull request #114 from smacker/better_integration_tests

Better integration tests

CONTRIBUTING.md

@@ -5,6 +5,18 @@ This document outlines some of the
 conventions on development workflow, commit message formatting, contact points,
 and other resources to make it easier to get your contribution accepted.
 
+## Build
+
+To build lookout binary run:
+```bash
+make build
+```
+
+To build sdk & dummy analyzer run:
+```bash
+build-sdk
+```
+
 ## Code generation
 
 * Go code from proto files: `make protogen`
@@ -18,6 +30,23 @@ and other resources to make it easier to get your contribution accepted.
     make bindata
     ```
 
+## Testing
+
+For unit-tests run:
+```bash
+make test
+```
+
+For SDK integration tests:
+```bash
+make test-sdk
+```
+
+For lookout serve integration tests:
+```bash
+make test-json
+```
+
 ## Certificate of Origin
 
 By contributing to this project you agree to the [Developer Certificate of

Makefile

@@ -79,32 +79,12 @@ $(PROTOC):
 # Integration test for sdk client
 .PHONY: test-sdk
 test-sdk: clean-sdk build-sdk
-	DUMMY_BIN=$(DUMMY_BIN) LOOKOUT_BIN=$(LOOKOUT_BIN) $(GOCMD) run sdk-test/main.go
+	DUMMY_BIN=$(DUMMY_BIN) LOOKOUT_BIN=$(LOOKOUT_BIN) $(GOCMD) run cmd/sdk-test/main.go
 
 # Integration test for lookout serve
 .PHONY: test-json
-test-json: build-sdk
-	$(DUMMY_BIN) serve &>/dev/null & \
-	DUMMY_PID=$$!; \
-	cat fixtures/events.jsonl | $(LOOKOUT_BIN) serve --provider json dummy-repo-url > serve.txt 2> serve-err.txt & \
-	LOOKOUT_PID=$$!; \
-	for i in `seq 0 120`; do \
-		sleep 1; \
-		grep '{"file":"provider/common.go","text":"The file has increased in 5 lines."}' serve.txt; \
-		if [ $$? = 0 ] ; then \
-			kill $$LOOKOUT_PID; \
-			kill $$DUMMY_PID; \
-			exit 0; \
-		fi; \
-	done; \
-	echo "timeout reached, inspect serve.txt and serve-err.txt for details"; \
-	echo -e "\nserve.txt:"; \
-	cat serve.txt; \
-	echo -e "\nserve-err.txt:"; \
-	cat serve-err.txt; \
-	kill $$LOOKOUT_PID; \
-	kill $$DUMMY_PID; \
-	exit 1; \
+test-json: clean-sdk build-sdk
+	DUMMY_BIN=$(DUMMY_BIN) LOOKOUT_BIN=$(LOOKOUT_BIN) $(GOCMD) run cmd/server-test/main.go
 
 # Build sdk client and dummy analyzer
 .PHONY: build-sdk

cmd/sdk-test/main.go

@@ -0,0 +1,48 @@
+package main
+
+import (
+	"fmt"
+
+	"github.com/src-d/lookout/util/cmdtest"
+)
+
+func main() {
+	fmt.Println("start integration testing")
+
+	ctx, stop := cmdtest.StoppableCtx()
+	cmdtest.StartDummy(ctx)
+	testCase("testing review", func() {
+		r := cmdtest.RunCli(ctx, "review", "ipv4://localhost:10302")
+		cmdtest.GrepTrue(r, "posting analysis")
+	})
+
+	testCase("testing push", func() {
+		r := cmdtest.RunCli(ctx, "push", "ipv4://localhost:10302")
+		cmdtest.GrepTrue(r, "dummy comment for push event")
+	})
+
+	// next tests require analyzier started with UAST, restart analyzer
+	stop()
+	ctx, stop = cmdtest.StoppableCtx()
+	cmdtest.StartDummy(ctx, "--uast")
+
+	testCase("should return error without bblfsh", func() {
+		r := cmdtest.RunCli(ctx, "review", "ipv4://localhost:10302", "--bblfshd=ipv4://localhost:0000")
+		cmdtest.GrepTrue(r, "WantUAST isn't allowed")
+	})
+
+	testCase("should notify about lack of uast", func() {
+		r := cmdtest.RunCli(ctx, "review", "ipv4://localhost:10302",
+			"--from=66924f49aa9987273a137857c979ee5f0e709e30",
+			"--to=2c9f56bcb55be47cf35d40d024ec755399f699c7")
+		cmdtest.GrepTrue(r, "The file doesn't have UAST")
+	})
+
+	stop()
+}
+
+func testCase(name string, fn func()) {
+	fmt.Print(name + "...")
+	fn()
+	fmt.Println("OK!")
+}

cmd/server-test/main.go

@@ -0,0 +1,162 @@
+package main
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"io"
+	"os"
+	"strings"
+
+	"github.com/src-d/lookout"
+	"github.com/src-d/lookout/util/cmdtest"
+	"github.com/src-d/lookout/util/grpchelper"
+	log "gopkg.in/src-d/go-log.v1"
+)
+
+func main() {
+	fmt.Println("start integration testing")
+
+	grepTrue := cmdtest.GrepTrue
+	grepAndNot := cmdtest.GrepAndNot
+
+	cmdtest.ResetDB()
+
+	ctx, stop := cmdtest.StoppableCtx()
+	cmdtest.StartDummy(ctx)
+	r, w := cmdtest.StartServe(ctx, "--provider", "json", "dummy-repo-url")
+
+	// make sure server started correctly
+	grepTrue(r, "Starting watcher")
+
+	successJSON := `{"event":"review", "internal_id": "1", "number": 1, "commit_revision":{"base":{"internal_repository_url":"https://github.com/src-d/lookout.git","reference_name":"refs/heads/master","hash":"4eebef102d7979570aadf69ff54ae1ffcca7ce00"},"head":{"internal_repository_url":"https://github.com/src-d/lookout.git","reference_name":"refs/heads/master","hash":"d304499cb2a9cad3ea260f06ad59c1658db4763d"}}}`
+	testCase("success review", func() {
+		sendEvent(w, successJSON)
+		grepTrue(r, "processing pull request")
+		grepTrue(r, `{"analyzer-name":"Dummy","file":"provider/common.go","text":"The file has increased in 5 lines."}`)
+		grepTrue(r, `status=success`)
+	})
+
+	testCase("skip review event", func() {
+		sendEvent(w, successJSON)
+		grepTrue(r, `event successfully processed, skipping...`)
+	})
+
+	testCase("process review but don't post anything", func() {
+		json := `{"event":"review", "internal_id": "2", "number": 1, "commit_revision":{"base":{"internal_repository_url":"https://github.com/src-d/lookout.git","reference_name":"refs/heads/master","hash":"4eebef102d7979570aadf69ff54ae1ffcca7ce00"},"head":{"internal_repository_url":"https://github.com/src-d/lookout.git","reference_name":"refs/heads/master","hash":"d304499cb2a9cad3ea260f06ad59c1658db4763d"}}}`
+		sendEvent(w, json)
+		grepTrue(r, "processing pull request")
+		grepAndNot(r, `status=success`, `posting analysis`)
+	})
+
+	testCase("wrong commit revision", func() {
+		json := `{"event":"review", "internal_id": "3", "number": 3, "commit_revision": {"base":{"internal_repository_url":"https://github.com/src-d/lookout.git","reference_name":"refs/heads/master","hash":"0000000000000000000000000000000000000000"},"head":{"internal_repository_url":"https://github.com/src-d/lookout.git","reference_name":"refs/heads/master","hash":"0000000000000000000000000000000000000000"}}}`
+		sendEvent(w, json)
+		grepTrue(r, `event processing failed`)
+	})
+
+	testCase("success push", func() {
+		successPushJSON := `{"event":"push", "internal_id": "1", "commit_revision":{"base":{"internal_repository_url":"https://github.com/src-d/lookout.git","reference_name":"refs/heads/master","hash":"4eebef102d7979570aadf69ff54ae1ffcca7ce00"},"head":{"internal_repository_url":"https://github.com/src-d/lookout.git","reference_name":"refs/heads/master","hash":"d304499cb2a9cad3ea260f06ad59c1658db4763d"}}}`
+		sendEvent(w, successPushJSON)
+		grepTrue(r, "processing push")
+		grepTrue(r, "comments can belong only to review event but 1 is given")
+		grepTrue(r, `status=success`)
+	})
+
+	// restart server with multiple analyzers
+	stop()
+	cmdtest.ResetDB()
+
+	ctx, stop = cmdtest.StoppableCtx()
+	cmdtest.StartDummy(ctx)
+	cmdtest.StartDummy(ctx, "--analyzer", "ipv4://localhost:10303")
+	r, w = cmdtest.StartServe(ctx, "--provider", "json", "-c", "fixtures/double_dummy_config.yml", "dummy-repo-url")
+
+	grepTrue(r, "Starting watcher")
+
+	testCase("multiple analyzers", func() {
+		sendEvent(w, successJSON)
+		grepTrue(r, "processing pull request")
+		grepTrue(r, "posting analysis")
+		found, buf := cmdtest.Grep(r, `status=success`)
+		if !found {
+			fmt.Printf("'%s' not found in:\n%s", `status=success`, buf.String())
+			stop()
+			os.Exit(1)
+		}
+		if !strings.Contains(
+			buf.String(),
+			`{"analyzer-name":"Dummy1","file":"provider/common.go","text":"The file has increased in 5 lines."}`) {
+			fmt.Println("no comments from the first analyzer")
+			stop()
+			os.Exit(1)
+		}
+		if !strings.Contains(
+			buf.String(),
+			`{"analyzer-name":"Dummy2","file":"provider/common.go","text":"The file has increased in 5 lines."}`) {
+			fmt.Println("no comments from the second analyzer")
+			stop()
+			os.Exit(1)
+		}
+	})
+
+	// restart server with error anylyzer
+	stop()
+	cmdtest.ResetDB()
+
+	ctx, stop = cmdtest.StoppableCtx()
+	startAnalyzer(ctx, &errAnalyzer{})
+	r, w = cmdtest.StartServe(ctx, "--provider", "json", "dummy-repo-url")
+	testCase("error from analyzer", func() {
+		sendEvent(w, successJSON)
+		grepTrue(r, `msg="analysis failed" analyzer=Dummy app=lookout error="rpc error: code = Unknown desc = review error"`)
+	})
+
+	stop()
+}
+
+func startAnalyzer(ctx context.Context, a lookout.AnalyzerServer) error {
+	log.DefaultFactory = &log.LoggerFactory{
+		Level: log.ErrorLevel,
+	}
+	log.DefaultLogger = log.New(log.Fields{"app": "test"})
+
+	server := grpchelper.NewServer()
+	lookout.RegisterAnalyzerServer(server, a)
+
+	lis, err := grpchelper.Listen("ipv4://localhost:10302")
+	if err != nil {
+		return err
+	}
+
+	go server.Serve(lis)
+	go func() {
+		<-ctx.Done()
+		server.Stop()
+	}()
+	return nil
+}
+
+type errAnalyzer struct{}
+
+func (a *errAnalyzer) NotifyReviewEvent(ctx context.Context, e *lookout.ReviewEvent) (*lookout.EventResponse, error) {
+	return nil, errors.New("review error")
+}
+
+func (a *errAnalyzer) NotifyPushEvent(ctx context.Context, e *lookout.PushEvent) (*lookout.EventResponse, error) {
+	return nil, errors.New("push error")
+}
+
+func sendEvent(w io.Writer, json string) {
+	_, err := fmt.Fprintln(w, json)
+	if err != nil {
+		fmt.Println("can't write event:", err)
+		os.Exit(1)
+	}
+}
+
+func testCase(name string, fn func()) {
+	fmt.Print(name + "...")
+	fn()
+	fmt.Println("OK!")
+}

fixtures/double_dummy_config.yml

@@ -0,0 +1,5 @@
+analyzers:
+  - name: Dummy1
+    addr: ipv4://localhost:10302
+  - name: Dummy2
+    addr: ipv4://localhost:10303

provider/json/poster.go

@@ -26,13 +26,13 @@ func NewPoster(w io.Writer) *Poster {
 	}
 }
 
-// Post prints json comments to sdtout
+// Post prints json comments to stdout
 func (p *Poster) Post(ctx context.Context, e lookout.Event,
 	aCommentsList []lookout.AnalyzerComments) error {
 
 	for _, a := range aCommentsList {
 		for _, c := range a.Comments {
-			if err := p.enc.Encode(c); err != nil {
+			if err := p.enc.Encode(commentToPrint{AnalyzerName: a.Config.Name, Comment: c}); err != nil {
 				return err
 			}
 		}
@@ -48,3 +48,8 @@ func (p *Poster) Status(ctx context.Context, e lookout.Event,
 	ctxlog.Get(ctx).With(log.Fields{"status": status}).Infof("New status")
 	return nil
 }
+
+type commentToPrint struct {
+	AnalyzerName string `json:"analyzer-name"`
+	*lookout.Comment
+}

provider/json/poster_test.go

@@ -60,10 +60,10 @@ func TestPoster_Post_OK(t *testing.T) {
 	err := p.Post(context.Background(), ev, aCommentsList)
 	require.NoError(err)
 
-	expected := `{"text":"This is a global comment"}
-{"file":"main.go","text":"This is a file comment"}
-{"file":"main.go","line":5,"text":"This is a line comment"}
-{"text":"This is a another global comment"}
+	expected := `{"analyzer-name":"mock","text":"This is a global comment"}
+{"analyzer-name":"mock","file":"main.go","text":"This is a file comment"}
+{"analyzer-name":"mock","file":"main.go","line":5,"text":"This is a line comment"}
+{"analyzer-name":"mock","text":"This is a another global comment"}
 `
 
 	require.Equal(expected, b.String())