mrkschan / nginxbeat

Superseded by https://github.com/elastic/beats/tree/master/metricbeat
Apache License 2.0
58 stars 7 forks source link

gotestcover is deprecated #45

Open pierrre opened 8 years ago

pierrre commented 8 years ago

Please read https://github.com/pierrre/gotestcover#deprecated

ruflin commented 8 years ago

@pierrre Thanks for the heads up. I have to apply this to https://github.com/elastic/beats

pierrre commented 8 years ago

@ruflin Are you OK with my new script? It does exactly the same thing.

ruflin commented 8 years ago

@pierrre Will test it in the next hours / days as soon as I get the time for it.

ruflin commented 8 years ago

@pierrre Only did a very brief test and got the error: `cannot use test profile flag with multiple packages``

Our code looks as following:

$(GOPATH)/bin/gotestcover $(RACE) -coverprofile=${COVERAGE_DIR}/unit.cov  ${GOPACKAGES}

I assume the package list is the problem. No sure if I should put this in case of go list?

pierrre commented 8 years ago

@ruflin have you read the new script that replaces gotestcover? https://github.com/pierrre/gotestcover#deprecated

echo 'mode: atomic' > coverage.txt && go list ./... | xargs -n1 -I{} sh -c 'go test -covermode=atomic -coverprofile=coverage.tmp {} && tail -n +2 coverage.tmp >> coverage.txt' && rm coverage.tmp
ruflin commented 8 years ago

I wanted to paste the modified line of course that I tried out...

echo 'mode: atomic' > coverage.txt && go list ./... | xargs -n1 -I{} sh -c 'go test $(RACE) -coverprofile=${COVERAGE_DIR}/unit.cov  ${GOPACKAGES} && tail -n +2 coverage.tmp >> coverage.txt' && rm coverage.tmp
pierrre commented 8 years ago

I use it in my project: https://github.com/pierrre/imageserver/blob/master/.travis.yml#L24 It works: https://travis-ci.org/pierrre/imageserver/jobs/130788141#L223

pierrre commented 8 years ago

OK I understand. What is ${GOPACKAGES} in your script?

No sure if I should put this in case of go list?

Yes, try that

pierrre commented 8 years ago

Please try

echo 'mode: atomic' > ${COVERAGE_DIR}/unit.cov && go list ${GOPACKAGES} | xargs -n1 -I{} sh -c 'go test $(RACE) -covermode=atomic -coverprofile=coverage.tmp {} && tail -n +2 coverage.tmp >> ${COVERAGE_DIR}/unit.cov' && rm coverage.tmp
ruflin commented 8 years ago

Looks like this gets the job done. Thanks.

Now I need to find a way how to make this line shorter / less complex again as we have it multiple times: https://github.com/elastic/beats/blob/master/libbeat/scripts/Makefile#L124

pierrre commented 8 years ago

You could paste this script in a bash file and run it each time you need it. But it does not allow you to customize arguments for each call.

In my projects, I've copied/pasted this small script :)

pierrre commented 8 years ago

I'm sorry if this deprecation notice is too abrupt for you. But I find this new solution easier to maintain and more customizable.

(FYI I love Go and I hate bash scripts)

ruflin commented 8 years ago

@pierrre No worries, I really appreciate that you actively inform that the project will not be maintained anymore in the future and even provide a replacement script. The script will work quite well in our Makefiles, no worries.

I don't have it on top of my priority list as gotestcover works and I assume the repo will stay around a little bit.

pierrre commented 8 years ago

Don't be afraid, I will not remove the repository :)