Fix coverage files combining

[saschagrunert]

Hey, this enhancement should fix #518.

[williammartin]

Thanks for the PR. Is there a test we can write to get coverage of this?

[saschagrunert]

Yeah sure, I will add an integration test.

[saschagrunert]

@williammartin I am wondering why the test on go 1.9 fails. Could it be possible that it is the case because the travis machine still installs from github.com/onsi/ginkgo/ginkgo and runs this executable?

I guess for integration testing of forks the binary needs to be built locally, right?

[NilsJPWerner]

Any updates on when this will be fixed/merged?

[williammartin]

I can reproduce this failure on go 1.10.7 by focussing the changed spec. Will keep digging.

[saschagrunert]

@williammartin Is there anything I can do here? I guess the failing pipeline is related to the go fetch source...

[williammartin]

Well, I was just having a look at the contents of this file: https://github.com/onsi/ginkgo/pull/552/files#diff-28605e74bddc91d19789edefa99640b8R121

And indeed, in 1.10 it looks like this:

mode: atomic
mode: atomic
github.com/onsi/ginkgo/integration/_fixtures/combined_coverage_fixture/first_package/coverage.go:3.17,5.2 1 1
github.com/onsi/ginkgo/integration/_fixtures/combined_coverage_fixture/first_package/coverage.go:7.17,9.2 1 1
github.com/onsi/ginkgo/integration/_fixtures/combined_coverage_fixture/first_package/coverage.go:11.17,13.2 1 1
github.com/onsi/ginkgo/integration/_fixtures/combined_coverage_fixture/first_package/coverage.go:15.17,17.2 1 1
github.com/onsi/ginkgo/integration/_fixtures/combined_coverage_fixture/first_package/coverage.go:19.17,21.2 1 0
github.com/onsi/ginkgo/integration/_fixtures/combined_coverage_fixture/second_package/coverage.go:3.17,5.2 1 1
github.com/onsi/ginkgo/integration/_fixtures/combined_coverage_fixture/second_package/coverage.go:7.17,9.2 1 1
github.com/onsi/ginkgo/integration/_fixtures/combined_coverage_fixture/second_package/coverage.go:11.17,13.2 1 1
github.com/onsi/ginkgo/integration/_fixtures/combined_coverage_fixture/second_package/coverage.go:15.17,17.2 1 1
github.com/onsi/ginkgo/integration/_fixtures/combined_coverage_fixture/second_package/coverage.go:19.17,21.2 1 1

and in 1.11:

mode: atomic
github.com/onsi/ginkgo/integration/_fixtures/combined_coverage_fixture/first_package/coverage.go:3.17,5.2 1 1
github.com/onsi/ginkgo/integration/_fixtures/combined_coverage_fixture/first_package/coverage.go:7.17,9.2 1 1
github.com/onsi/ginkgo/integration/_fixtures/combined_coverage_fixture/first_package/coverage.go:11.17,13.2 1 1
github.com/onsi/ginkgo/integration/_fixtures/combined_coverage_fixture/first_package/coverage.go:15.17,17.2 1 1
github.com/onsi/ginkgo/integration/_fixtures/combined_coverage_fixture/first_package/coverage.go:19.17,21.2 1 0
github.com/onsi/ginkgo/integration/_fixtures/combined_coverage_fixture/second_package/coverage.go:3.17,5.2 1 1
github.com/onsi/ginkgo/integration/_fixtures/combined_coverage_fixture/second_package/coverage.go:7.17,9.2 1 1
github.com/onsi/ginkgo/integration/_fixtures/combined_coverage_fixture/second_package/coverage.go:11.17,13.2 1 1
github.com/onsi/ginkgo/integration/_fixtures/combined_coverage_fixture/second_package/coverage.go:15.17,17.2 1 1
github.com/onsi/ginkgo/integration/_fixtures/combined_coverage_fixture/second_package/coverage.go:19.17,21.2 1 1

Something funny in 1.10 is resulting in keeping the duplicate mode: atomic lines

[williammartin]

For some reason the coverage file in Go 1.10 has mode: atomic in it already.

https://github.com/onsi/ginkgo/blob/master/ginkgo/run_command.go#L166

[saschagrunert]

I must say this makes me not happy. So do we have to add a hack-around for go 1.10/11?

[williammartin]

I don't fully understand what is causing the difference yet. Ideally I'd like to reproduce a simple case of this behaviour without ginkgo to ensure I understand where the difference is and what the behaviour is.

[rohitsakala]

Hi, any update on this issue ?

[zhandao]

Looking forward to merging it

[williammartin]

I do not have the time to look at this issue right now (sorry for the lack of communication), if we want to merge this soon, I'll need help debugging the failing builds.

Thanks!

[saschagrunert]

Fixed the go 1.10 bug by removing os.O_APPEND

[saschagrunert]

Rebased on top of the latest master and applied a fix for the go1.10 issue.

[williammartin]

LGTM thanks