Testing refactor for Jenkins by zramsay · Pull Request #1098 · tendermint/tendermint

zramsay · 2018-01-11T15:08:01Z

What used to be make test_integration and a rabbit hole of bash scripts is now:

make test_cover_fast
make test_cover_slow
make test_apps
make test_persistence
make test_p2p

where each test is called by a seperate Jenkins job.

Some optimizations remain but require feedback.

ebuchman · 2018-01-17T22:27:14Z

test/p2p/client.sh

 # run the test container on the local network
 docker run -t --rm \
-	-v "$GOPATH/src/github.com/tendermint/tendermint/test/p2p/:/go/src/github.com/tendermint/tendermint/test/p2p" \
+	-v "$GOPATH/src/github.com/tendermint/tendermint/test/p2p/:/go/src/github.com/tendermint/tendermint/test/p2p":Z \


NOTE: :Z is needed for SELinux to allow writes to the mounted volume on host

ebuchman · 2018-01-17T22:39:52Z

test/test_cover.sh

+
+if [ $1 == "slow" ]; then 
+	for pkg in ${PKGS[@]}; do
+		echo $pkg


not sure what determiend these distinctions. types runs way slower on my machine than node or rpc/client.

Is this really helpful ? Why not just run them all together as make test ...

i split them up because of non-deterministic failures in the "slow" tests. Sure, another package might be slower or faster; these three "slow" packages just really mean "takes relatively long to run and has been known to reliably fail non-deterministically

it's much easier to debug failures when they're in different jobs. The "slow" tests also generally pump out tons of logs. IMO the goal should be to, yeah, move the slow tests into the fast section, once they're sped up and more deterministic.

great thanks for explanation!

ebuchman · 2018-01-17T22:40:36Z

test/persist/test_failure_indices.sh

-        $DUMMY_CMD &> "dummy_${name}.log" &
-    fi
+    echo "Starting persistent dummy and tendermint: $name"
+    $DUMMY_CMD &> "dummy_${name}.log" &


will I be able to see this log on jenkins ? same for ones below

working on it

yes, they'll be archived as aritfacts, e.g., here's the coverage.txt that can be copied by a job that sends it to codecov. https://ci.interblock.io/job/05.Test.Tendermint.FeatureBranches.Cover.Fast/

ebuchman · 2018-01-17T22:41:25Z

test/persist/test_failure_indices.sh

    # tendermint should already have exited when it hits the fail index
    # but kill -9 for good measure
-    kill_procs
+    # kill_procs


? we dont want to leave zombie processes

ebuchman · 2018-01-17T22:42:17Z

Makefile

@@ -62,16 +62,47 @@ draw_deps:
 ########################################
 ### Testing


can we get a single makefile target to run all the tests so we can easily run the full suite locally? need to ensure its easy to see logs and debug when doing this so we can figure out issues ...

melekes · 2018-01-21T11:09:03Z

Makefile

+	# requires 'tester' the image from above
+	bash test/p2p/test.sh tester
+
+test_all:


I personally liked the old test_integrations target

melekes · 2018-01-21T11:11:31Z

Makefile

+	vagrant ssh -c 'make test_all'
+
+### go tests without docker
+test_unit:


I feel like we should still keep test target. Maybe alias it to test_unit by default?

test: test_unit

let's just keep test to conform with:

tendermint/test/test_libs.sh

Line 15 in 381fe19

# All libs should define `make test` and `make get_vendor_deps`

melekes · 2018-01-24T11:37:41Z

Makefile

 BUILD_FLAGS = -ldflags "-X github.com/tendermint/tendermint/version.GitCommit=`git rev-parse --short HEAD`"

-all: check build test install
+all: check build install test_integrations


I think Jae wanted to run tests before install.

the tests need the binary available on $PATH

ebuchman · 2018-01-26T22:06:36Z

Can we get this rebased on develop and squash the commits please

ebuchman · 2018-02-05T15:52:58Z

Let's get this passing on circle so we can merge @zramsay

zramsay · 2018-02-05T18:45:37Z

proto: no slice oenc for *reflect.rtype = []*reflect.rtype
proto: no encoder for Validators []types.Validator [GetProperties]
proto: no slice oenc for *reflect.rtype = []*reflect.rtype
proto: no encoder for Validators []types.Validator [GetProperties]
proto: no coders for types.Header
proto: no encoder for Header types.Header [GetProperties]
proto: no slice oenc for *reflect.rtype = []*reflect.rtype
proto: no encoder for ByzantineValidators []types.Evidence [GetProperties]
proto: no coders for types.Header
proto: no encoder for Header types.Header [GetProperties]
proto: no slice oenc for *reflect.rtype = []*reflect.rtype
proto: no encoder for ByzantineValidators []types.Evidence [GetProperties]
proto: no slice oenc for *reflect.rtype = []*reflect.rtype
proto: no encoder for ValidatorUpdates []types.Validator [GetProperties]
proto: no slice oenc for *reflect.rtype = []*reflect.rtype
proto: no encoder for ValidatorUpdates []types.Validator [GetProperties]

ebuchman · 2018-03-02T05:04:43Z

Can we get this rebased and cleaned up? Maybe @xla can take a look too? Not really Jenkins relevant (though Jenkins uses some of these targets) but would be nice to get merged. Also the unit tests run much faster now so we dont need the fast/slow targets. `

zramsay · 2018-03-02T06:07:33Z

the fast/slow is gone

xla

@ebuchman @zramsay This is green like bean and significantly faster than what we had before for the CI runs.

👍 🍡

ebuchman · 2018-03-02T14:15:50Z

Makefile

+	docker run --name run_persistence -t tester bash test/persist/test_failure_indices.sh
+
+	# TODO undockerize
+	# bash test/persist/test_failure_indices.sh


one reason the tests are faster here

ebuchman · 2018-03-02T14:16:44Z

circle.yml

-    - cd "$PROJECT_PATH" && mv test_integrations.log "${CIRCLE_ARTIFACTS}"
-    - cd "$PROJECT_PATH" && bash <(curl -s https://codecov.io/bash) -f coverage.txt
-    - cd "$PROJECT_PATH" && mv coverage.txt "${CIRCLE_ARTIFACTS}"
-    - cd "$PROJECT_PATH" && cp test/logs/messages "${CIRCLE_ARTIFACTS}/docker.log"


I think we still need this log

The docker log usually isn't very telling, what information would we lose?

The docker log usually isn't very telling, what information would we lose?

huh? all docker containers are logging msgs to docker.log through syslog

Yeh the eg. p2p integration tests spin up 4 containers and do a bunch of tests with them. The only way to see what's happening on those nodes is the docker.log (we don't pipe it out to circle stdout directly because its so verbose)

Surely, but the majority of those lines are the output of our binaries which we captured with the test_integration.log.

Cool, clarified now. Would be nice to have a better split of those logs for easier tracing when doing forensics.

zramsay · 2018-03-05T02:53:47Z

so are we just missing the logs from that last comment?

melekes · 2018-03-05T18:26:27Z

Makefile

 BUILD_FLAGS = -ldflags "-X github.com/tendermint/tendermint/version.GitCommit=`git rev-parse --short=8 HEAD`"

-all: check build test install
+all: check build test_integrations install


do we really want to run test_integrations here?

IIRC this was requested (somewhere, can't remember)

melekes · 2018-03-05T18:27:18Z

Makefile

+	docker run --name run_persistence -t tester bash test/persist/test_failure_indices.sh
+
+	# TODO undockerize
+	# bash test/persist/test_failure_indices.sh


xla · 2018-03-06T21:36:52Z

I'm not sure if this PR makes sense anymore, with the changes happened to Circle and @greg-szabo efforts. Was able to rebase latest develop and it builds: https://circleci.com/workflow-run/7da83251-b7d6-4284-a572-dd908256b753

ebuchman · 2018-03-06T22:39:51Z

This PR is worth keeping for some of the cleanup (deleting unused files etc.). If someone wants to rebase and merge thatd be great!

* makefile cleanup * tests: split fast and slow go tests, closes #1055

codecov-io · 2018-03-06T23:03:27Z

Codecov Report

Merging #1098 into develop will decrease coverage by 1.14%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop    #1098      +/-   ##
===========================================
- Coverage    60.58%   59.43%   -1.15%     
===========================================
  Files          112      121       +9     
  Lines        10863    11040     +177     
===========================================
- Hits          6581     6562      -19     
- Misses        3644     3865     +221     
+ Partials       638      613      -25

Impacted Files	Coverage Δ
lite/memprovider.go	`79.06% <0%> (-18.37%)`	⬇️
types/validator.go	`61.4% <0%> (-8.6%)`	⬇️
types/validator_set.go	`43.68% <0%> (-6.5%)`	⬇️
p2p/switch.go	`47.03% <0%> (-4.22%)`	⬇️
types/block.go	`29.91% <0%> (-3.16%)`	⬇️
p2p/node_info.go	`51.78% <0%> (-2.94%)`	⬇️
state/execution.go	`68.54% <0%> (-1.85%)`	⬇️
consensus/replay.go	`67.75% <0%> (-1.68%)`	⬇️
cmd/tendermint/commands/root.go	`58.33% <0%> (-1.67%)`	⬇️
p2p/conn/connection.go	`81.11% <0%> (-1.23%)`	⬇️
... and 59 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2ce57a6...b0c3942. Read the comment docs.

xla · 2018-03-06T23:04:46Z

@ebuchman Done.

We might wanna squash for a bit cleaner history.

melekes · 2018-03-08T14:34:40Z

Makefile

 # unless there is a reason not to.
 # https://www.gnu.org/software/make/manual/html_node/Phony-Targets.html
-.PHONY: check build build_race dist install check_tools get_tools update_tools get_vendor_deps draw_deps get_deps_bin_size test test_race test_integrations test_release test100 vagrant_test fmt metalinter metalinter_all
+.PHONY: check build build_race dist install check_tools get_tools update_tools get_vendor_deps draw_depsbuild_test_docker_image test_cover test_apps test_persistence test_p2p test test_race test_libs test_integrations test_release test100 vagrant_test fmt


"draw_depsbuild_test_docker_image" space is missing

zramsay requested review from ebuchman and melekes as code owners January 11, 2018 15:08

zramsay force-pushed the jenkins branch from 3986470 to 67eae87 Compare January 16, 2018 14:59

ebuchman reviewed Jan 17, 2018

View reviewed changes

zramsay force-pushed the jenkins branch 2 times, most recently from 69e985e to 9a32495 Compare January 19, 2018 15:43

melekes reviewed Jan 21, 2018

View reviewed changes

zramsay mentioned this pull request Jan 22, 2018

WIP: Jenkins cosmos/ethermint-archive#379

Closed

melekes reviewed Jan 24, 2018

View reviewed changes

zramsay force-pushed the jenkins branch from 9781bbd to 934465c Compare January 30, 2018 16:50

zramsay closed this Jan 31, 2018

zramsay reopened this Jan 31, 2018

zramsay closed this Jan 31, 2018

zramsay reopened this Jan 31, 2018

ebuchman force-pushed the jenkins branch from 934465c to 2254162 Compare February 1, 2018 02:52

ebuchman approved these changes Feb 1, 2018

View reviewed changes

ebuchman assigned zramsay Feb 5, 2018

zramsay closed this Feb 6, 2018

zramsay reopened this Feb 6, 2018

zramsay assigned xla Mar 2, 2018

zramsay requested a review from xla March 2, 2018 07:23

xla approved these changes Mar 2, 2018

View reviewed changes

ebuchman reviewed Mar 2, 2018

View reviewed changes

melekes reviewed Mar 5, 2018

View reviewed changes

zramsay and others added 8 commits March 6, 2018 23:49

de-mystify tests & run them in parallel (#1031)

cd827d7

test optimization for jenkins (#1093)

14ddd66

* makefile cleanup * tests: split fast and slow go tests, closes #1055

pr comments

ddd4180

restore circle conditions

0d6f45d

fix need_abci

7fbf72e

...

f1a00bf

docker run: no :Z for circle?

93202e1

Remove cmd breaking comment

b0c3942

xla force-pushed the jenkins branch from d48ebbb to b0c3942 Compare March 6, 2018 22:57

melekes reviewed Mar 8, 2018

View reviewed changes

melekes merged commit 13a2013 into develop Mar 8, 2018

melekes deleted the jenkins branch March 8, 2018 14:52

		@@ -62,16 +62,47 @@ draw_deps:
		########################################
		### Testing

Conversation

zramsay commented Jan 11, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ebuchman Jan 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zramsay Jan 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ebuchman commented Jan 26, 2018

Uh oh!

ebuchman commented Feb 5, 2018

Uh oh!

zramsay commented Feb 5, 2018

Uh oh!

ebuchman commented Mar 2, 2018

Uh oh!

zramsay commented Mar 2, 2018

Uh oh!

xla left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zramsay commented Mar 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

ebuchman Jan 17, 2018 •

edited

Loading

zramsay Jan 18, 2018 •

edited

Loading

zramsay commented Mar 5, 2018 •

edited

Loading

codecov-io commented Mar 6, 2018 •

edited

Loading