From 1d521556ae47e4373aa1e938f66dd29a31efeac9 Mon Sep 17 00:00:00 2001 From: fanmin shi Date: Mon, 30 Jan 2017 15:59:28 -0800 Subject: [PATCH 1/5] e2e: modify e2e to run code coverage --- e2e/etcd_spawn_cov.go.go | 76 ++++++++++++++++++++++++++++++++++++++++ e2e/etcd_spawn_nocov.go | 23 ++++++++++++ e2e/etcd_test.go | 4 --- main_test.go | 35 ++++++++++++++++++ pkg/expect/expect.go | 12 +++++-- 5 files changed, 144 insertions(+), 6 deletions(-) create mode 100644 e2e/etcd_spawn_cov.go.go create mode 100644 e2e/etcd_spawn_nocov.go create mode 100644 main_test.go diff --git a/e2e/etcd_spawn_cov.go.go b/e2e/etcd_spawn_cov.go.go new file mode 100644 index 00000000000..94717e27888 --- /dev/null +++ b/e2e/etcd_spawn_cov.go.go @@ -0,0 +1,76 @@ +// Copyright 2017 The etcd Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// +build cov + +package e2e + +import ( + "fmt" + "os" + "strings" + "time" + + "path/filepath" + + "github.com/coreos/etcd/pkg/expect" + "github.com/coreos/etcd/pkg/fileutil" + "github.com/coreos/etcd/pkg/flags" +) + +func spawnCmd(args []string) (*expect.ExpectProcess, error) { + if args[0] == binPath { + coverPath := os.Getenv("COVERDIR") + if !filepath.IsAbs(coverPath) { + // COVERDIR is relative to etcd root but e2e test has its path set to be relative to the e2e folder. + // adding ".." in front of COVERDIR ensures that e2e saves coverage reports to the correct location. + coverPath = filepath.Join("..", coverPath) + } + if !fileutil.Exist(coverPath) { + return nil, fmt.Errorf("could not find coverage folder") + } + covArgs := []string{ + fmt.Sprintf("-test.coverprofile=e2e.%v.coverprofile", time.Now().UnixNano()), + "-test.outputdir=" + coverPath, + } + return expect.NewExpectWithEnv(binDir+"/etcd_test", covArgs, args2env(args[1:])) + } + return expect.NewExpect(args[0], args[1:]...) +} + +func args2env(args []string) []string { + var covEnvs []string + for i := range args[1:] { + if !strings.HasPrefix(args[i], "--") { + continue + } + flag := strings.Split(args[i], "--")[1] + val := "true" + // split the flag that has "=" + // e.g --auto-tls=true" => flag=auto-tls and val=true + if strings.Contains(args[i], "=") { + split := strings.Split(flag, "=") + flag = split[0] + val = split[1] + } + + if i+1 < len(args) { + if !strings.HasPrefix(args[i+1], "--") { + val = args[i+1] + } + } + covEnvs = append(covEnvs, flags.FlagToEnv("ETCD", flag)+"="+val) + } + return covEnvs +} diff --git a/e2e/etcd_spawn_nocov.go b/e2e/etcd_spawn_nocov.go new file mode 100644 index 00000000000..1205432dbcd --- /dev/null +++ b/e2e/etcd_spawn_nocov.go @@ -0,0 +1,23 @@ +// Copyright 2017 The etcd Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// +build !cov + +package e2e + +import "github.com/coreos/etcd/pkg/expect" + +func spawnCmd(args []string) (*expect.ExpectProcess, error) { + return expect.NewExpect(args[0], args[1:]...) +} diff --git a/e2e/etcd_test.go b/e2e/etcd_test.go index bebc52f3c06..01702c60f9a 100644 --- a/e2e/etcd_test.go +++ b/e2e/etcd_test.go @@ -494,10 +494,6 @@ func waitReadyExpectProc(exproc *expect.ExpectProcess, isProxy bool) error { return err } -func spawnCmd(args []string) (*expect.ExpectProcess, error) { - return expect.NewExpect(args[0], args[1:]...) -} - func spawnWithExpect(args []string, expected string) error { return spawnWithExpects(args, []string{expected}...) } diff --git a/main_test.go b/main_test.go new file mode 100644 index 00000000000..0f5dfb983d1 --- /dev/null +++ b/main_test.go @@ -0,0 +1,35 @@ +// Copyright 2017 The etcd Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package main + +import ( + "os" + "os/signal" + "strings" + "syscall" + "testing" +) + +func TestMain(t *testing.T) { + // don't launch etcd server when invoked via go test + if strings.HasSuffix(os.Args[0], "etcd.test") { + return + } + + notifier := make(chan os.Signal, 1) + signal.Notify(notifier, syscall.SIGINT, syscall.SIGTERM, syscall.SIGKILL) + go main() + <-notifier +} diff --git a/pkg/expect/expect.go b/pkg/expect/expect.go index 3322b11f4ba..94650f33f63 100644 --- a/pkg/expect/expect.go +++ b/pkg/expect/expect.go @@ -44,7 +44,15 @@ var printDebugLines = os.Getenv("EXPECT_DEBUG") != "" // NewExpect creates a new process for expect testing. func NewExpect(name string, arg ...string) (ep *ExpectProcess, err error) { - ep = &ExpectProcess{cmd: exec.Command(name, arg...)} + // if env[] is nil, use current system env + return NewExpectWithEnv(name, arg, nil) +} + +// NewExpectWithEnv creates a new process with user defined env variables for expect testing. +func NewExpectWithEnv(name string, args []string, env []string) (ep *ExpectProcess, err error) { + cmd := exec.Command(name, args...) + cmd.Env = env + ep = &ExpectProcess{cmd: cmd} ep.cond = sync.NewCond(&ep.mu) ep.cmd.Stderr = ep.cmd.Stdout ep.cmd.Stdin = nil @@ -132,7 +140,7 @@ func (ep *ExpectProcess) close(kill bool) error { return ep.err } if kill { - ep.cmd.Process.Kill() + ep.Signal(os.Interrupt) } err := ep.cmd.Wait() From 80ab321f9d54f22dc9ba61c0e0b79c4e0c7c745c Mon Sep 17 00:00:00 2001 From: fanmin shi Date: Mon, 30 Jan 2017 16:04:40 -0800 Subject: [PATCH 2/5] etcdmain: whitelist etcd binary flags --- etcdmain/config.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/etcdmain/config.go b/etcdmain/config.go index 27aaaa2221f..c4c63a942e5 100644 --- a/etcdmain/config.go +++ b/etcdmain/config.go @@ -52,6 +52,9 @@ var ( "snapshot", "v", "vv", + // for coverage testing + "test.coverprofile", + "test.outputdir", } ) From b5be18a7446fa7777e68f016c6ea2cb1e30b640c Mon Sep 17 00:00:00 2001 From: fanmin shi Date: Mon, 30 Jan 2017 16:05:21 -0800 Subject: [PATCH 3/5] test: add e2e to coverage test --- test | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/test b/test index 134258e5737..3ea2d8fa825 100755 --- a/test +++ b/test @@ -10,7 +10,8 @@ # PKG=snap ./test # # Run code coverage -# COVERDIR=coverage PASSES=cov ./test +# COVERDIR must either be a absolute path or a relative path to the etcd root +# COVERDIR=coverage PASSES="build_cov cov" ./test set -e source ./build @@ -94,21 +95,31 @@ function cov_pass { exit 255 fi + if [ ! -f "bin/etcd_test" ]; then + echo "etcd_test binary not found" + exit 255 + fi + mkdir -p "$COVERDIR" # PKGS_DELIM contains all the core etcd pkgs delimited by ',' which will be profiled for code coverage. # Integration tests will generate code coverage for those pkgs PKGS_DELIM=$(echo $TEST | sed 's/ /,/g') - # TODO create coverage to e2e test PKGS=`echo "$TEST_PKGS" | egrep -v "(e2e|functional-tester)"` - + # run code coverage for unit and integration tests for t in ${PKGS}; do tf=`echo $t | tr / _` - # uses -run=Test to skip examples because clientv3/ example tests will leak goroutines + # uses -run=Test to skip examples because clientv3/ example tests will leak goroutines go test -covermode=set -coverpkg $PKGS_DELIM -timeout 15m -run=Test -v -coverprofile "$COVERDIR/${tf}.coverprofile" ${REPO_PATH}/$t done + # run code coverage for e2e tests + # use 30m timeout because e2e coverage takes longer + # due to many tests cause etcd process to wait + # on leadership transfer timeout during gracefully shutdown + go test -tags cov -timeout 30m -v ${REPO_PATH}"/e2e" + gocovmerge "$COVERDIR"/*.coverprofile >"$COVERDIR"/cover.out } @@ -283,6 +294,14 @@ function dep_pass { fi } +function build_cov_pass { + out="bin" + if [ -n "${BINDIR}" ]; then out="${BINDIR}"; fi + PKGS=$TEST + ETCD_PKGS_DELIM=$(echo $PKGS | sed 's/ /,/g') + go test -c -covermode=set -coverpkg=$ETCD_PKGS_DELIM -o ${out}/etcd_test +} + function compile_pass { echo "Checking build..." go build -v ./tools/... From 07129a6370706b283e5fc750e76ae9fedd298499 Mon Sep 17 00:00:00 2001 From: fanmin shi Date: Fri, 17 Feb 2017 09:33:32 -0800 Subject: [PATCH 4/5] *: add and expose StopSignal field in ExpectProcess add and expose StopSignal to ExpectProcess allows user to define what signal to send on ExpectProcess.close() coverage testing code sets StopSignal to SIGTERM allowing the test binary to shutdown gracefully so that it can generate a coverage report. --- e2e/etcd_spawn_cov.go.go | 12 +++++++++--- pkg/expect/expect.go | 11 +++++++++-- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/e2e/etcd_spawn_cov.go.go b/e2e/etcd_spawn_cov.go.go index 94717e27888..d7b8f8c9880 100644 --- a/e2e/etcd_spawn_cov.go.go +++ b/e2e/etcd_spawn_cov.go.go @@ -19,11 +19,11 @@ package e2e import ( "fmt" "os" + "path/filepath" "strings" + "syscall" "time" - "path/filepath" - "github.com/coreos/etcd/pkg/expect" "github.com/coreos/etcd/pkg/fileutil" "github.com/coreos/etcd/pkg/flags" @@ -44,7 +44,13 @@ func spawnCmd(args []string) (*expect.ExpectProcess, error) { fmt.Sprintf("-test.coverprofile=e2e.%v.coverprofile", time.Now().UnixNano()), "-test.outputdir=" + coverPath, } - return expect.NewExpectWithEnv(binDir+"/etcd_test", covArgs, args2env(args[1:])) + ep := expect.NewExpectWithEnv(binDir+"/etcd_test", covArgs, args2env(args[1:])) + // ep sends SIGTERM to etcd_test process on ep.close() + // allowing the process to exit gracefully in order to generate a coverage report. + // note: go runtime ignores SIGINT but not SIGTERM + // if e2e test is run as a background process. + ep.StopSignal = syscall.SIGTERM + return nil, ep } return expect.NewExpect(args[0], args[1:]...) } diff --git a/pkg/expect/expect.go b/pkg/expect/expect.go index 94650f33f63..a58121ccf9b 100644 --- a/pkg/expect/expect.go +++ b/pkg/expect/expect.go @@ -23,6 +23,7 @@ import ( "os/exec" "strings" "sync" + "syscall" "github.com/kr/pty" ) @@ -38,6 +39,9 @@ type ExpectProcess struct { lines []string count int // increment whenever new line gets added err error + + // StopSignal is the signal Stop sends to the process; defaults to SIGKILL. + StopSignal os.Signal } var printDebugLines = os.Getenv("EXPECT_DEBUG") != "" @@ -52,7 +56,10 @@ func NewExpect(name string, arg ...string) (ep *ExpectProcess, err error) { func NewExpectWithEnv(name string, args []string, env []string) (ep *ExpectProcess, err error) { cmd := exec.Command(name, args...) cmd.Env = env - ep = &ExpectProcess{cmd: cmd} + ep = &ExpectProcess{ + cmd: cmd, + StopSignal: syscall.SIGKILL, + } ep.cond = sync.NewCond(&ep.mu) ep.cmd.Stderr = ep.cmd.Stdout ep.cmd.Stdin = nil @@ -140,7 +147,7 @@ func (ep *ExpectProcess) close(kill bool) error { return ep.err } if kill { - ep.Signal(os.Interrupt) + ep.Signal(ep.StopSignal) } err := ep.cmd.Wait() From f203a61469064af987d7252191eea0c446f0834e Mon Sep 17 00:00:00 2001 From: fanmin shi Date: Fri, 17 Feb 2017 10:43:07 -0800 Subject: [PATCH 5/5] e2e: unshadow err and remove bogus err checking in spawnWithExpects() --- e2e/etcd_test.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/e2e/etcd_test.go b/e2e/etcd_test.go index 01702c60f9a..74d5a386f90 100644 --- a/e2e/etcd_test.go +++ b/e2e/etcd_test.go @@ -511,10 +511,10 @@ func spawnWithExpects(args []string, xs ...string) error { ) for _, txt := range xs { for { - l, err := proc.ExpectFunc(lineFunc) - if err != nil { + l, lerr := proc.ExpectFunc(lineFunc) + if lerr != nil { proc.Close() - return fmt.Errorf("%v (expected %q, got %q)", err, txt, lines) + return fmt.Errorf("%v (expected %q, got %q)", lerr, txt, lines) } lines = append(lines, l) if strings.Contains(l, txt) { @@ -523,9 +523,6 @@ func spawnWithExpects(args []string, xs ...string) error { } } perr := proc.Close() - if err != nil { - return err - } if len(xs) == 0 && proc.LineCount() != 0 { // expect no output return fmt.Errorf("unexpected output (got lines %q, line count %d)", lines, proc.LineCount()) }