[Jenkins] Go Agent Pipeline version 0

[kuisathaverat]

[codecov-io]

Codecov Report

Merging #292 into master will decrease coverage by 3.41%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #292      +/-   ##
==========================================
- Coverage   84.44%   81.02%   -3.42%     
==========================================
  Files          98       98              
  Lines        5857     5856       -1     
==========================================
- Hits         4946     4745     -201     
- Misses        839      840       +1     
- Partials       72      271     +199

Impacted Files	Coverage Δ
internal/sqlscanner/token.go	60% <0%> (-20%)	⬇️
internal/apmhostutil/docker_linux.go	62.5% <0%> (-12.5%)	⬇️
utils_linux.go	77.77% <0%> (-11.12%)	⬇️
model/marshal_fastjson.go	72.74% <0%> (-9.47%)	⬇️
sampler.go	81.81% <0%> (-9.1%)	⬇️
module/apmsql/utils.go	18.18% <0%> (-9.1%)	⬇️
internal/ringbuffer/buffer.go	81.03% <0%> (-8.63%)	⬇️
apmtest/withtransaction.go	83.33% <0%> (-8.34%)	⬇️
module/apmsql/driver_go110.go	64% <0%> (-8%)	⬇️
transport/transporttest/err.go	84.61% <0%> (-7.7%)	⬇️
... and 28 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 d2b6137...06f339b. Read the comment docs.

[axw]

Thanks, looking good. Is it possible to enable the job in apm-ci so we test on this PR before merging?

[axw]

I don't think we need coverage for both this and the docker tests. Having it for the docker tests should be enough, since those tests are a superset.

[kuisathaverat]

this one thing I have to ask you, there are differences between two coverage reports, the docker one has more files, these are they:

go.elastic.co/apm/apmtest/httpsuite.go
go.elastic.co/apm/apmtest/testlogger.go
go.elastic.co/apm/apmtest/withtransaction.go
go.elastic.co/apm/internal/apmcontext/context.go
go.elastic.co/apm/internal/apmdebug/debug.go
go.elastic.co/apm/internal/apmschema/schema.go
go.elastic.co/apm/internal/radix/radix.go
go.elastic.co/apm/transport/transporttest/err.go
go.elastic.co/apm/transport/transporttest/recorder.go

What is it the good coverage test?
I did not check the unit test executed but probably are different too.

[axw]

It's because of the "-coverpkg" flag.

go test -cover ./... will run the tests for each package in subdirectories of ./, but for each package it only includes coverage information of its own package. i.e. test coverage for go.elastic.co/apm/module/apmhttp only contains coverage information for go.elastic.co/apm/module/apmhttp.

In the docker case, it runs "make coverage", which runs scripts/test_coverage.sh. In that script you can see we add a flag, -coverpkg=go.elastic.co/apm/.... This instructs the go command to include coverage for all packages under go.elastic.co/apm/...

That script is slightly more complex than needed here, since it also works with older versions of Go. You could just run go test ./... -v -coverprofile="${COV_FILE}" -coverpkg=go.elastic.co/apm/... to get the same effect, for recent versions of Go.

[kuisathaverat]

so if we add -coverpkg=go.elastic.co/apm/... to the test.sh (test stage) both will be the same, ok, in that case the docker test can be removed.

[axw]

so if we add -coverpkg=go.elastic.co/apm/... to the test.sh (test stage) both will be the same, ok, in that case the docker test can be removed.

They might cover the same files, but not the same lines. The docker tests run a super-set of tests: integration tests that only run when certain environment variables are set (e.g. PGHOST). The docker one should be kept, and this one removed.

[kuisathaverat]

gotcha, it is the other way around

[kuisathaverat]

yep, as soon as Infra reprovision the Jenkins instance this pipeline will appear in the apm-agent-go-mbp project as a PR, then we would test it on apm-ci. I will try to it happens in the next two days.

[kuisathaverat]

@axw The creation of tags was removed, so the integration tests are disabled for now, the other stuff I think that was resolved, thus I think we are ready to Go ;)

[kuisathaverat]

The only decision pending, it is to set which steps only run on the master branch (or other branches) and which steps only run on PRs, at the moment this is may suggestion

checkout - all
build - all
test - all
benchmarks - master
docker-test - all
integration test (PR vs server stable) - PR
integration test Axis (agent vs the versions matrix) - master
documentation - master

The integration test and axis test are disabled until we implemented the SHA1 support on the integration tests

[axw]

The only decision pending, it is to set which steps only run on the master branch (or other branches) and which steps only run on PRs, at the moment this is may suggestion

checkout - all
build - all
test - all
benchmarks - master
docker-test - all
integration test (PR vs server stable) - PR
integration test Axis (agent vs the versions matrix) - master
documentation - master

That all sounds good to me.

Still a couple of coverage bits to be removed from the test stage / test.sh script. After that, and commits are squashed, LGTM :) Thanks for bearing with me.

[axw]

Please squash before merging. Maybe hold off until we set the default branch to master?

[kuisathaverat]

sure, we have to squash it before merge, there is no rush, so we can merge it next week.