Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pin proto / gogo versions for repeatable builds #984

Merged
merged 1 commit into from
Aug 14, 2018

Conversation

yurishkuro
Copy link
Member

@yurishkuro yurishkuro commented Aug 13, 2018

Undo some upgrades introduced in #940. A short term fix to #985.

@yurishkuro
Copy link
Member Author

yurishkuro commented Aug 13, 2018

https://travis-ci.org/jaegertracing/jaeger/jobs/415593435 failed with

=== RUN   TestKafkaStorage/GetTrace
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0xb3c342]
goroutine 15 [running]:
testing.tRunner.func1(0xc42034a0f0)
	/home/travis/.gimme/versions/go1.10.linux.amd64/src/testing/testing.go:742 +0x567
panic(0xfcc0e0, 0x16eed60)
	/home/travis/.gimme/versions/go1.10.linux.amd64/src/runtime/panic.go:505 +0x24a
github.com/jaegertracing/jaeger/model.SortTrace(0x0)
	/home/travis/gopath/src/github.com/jaegertracing/jaeger/model/sort.go:51 +0x42
github.com/jaegertracing/jaeger/plugin/storage/integration.CompareTraces(0xc42034a0f0, 0xc420312280, 0x0)
	/home/travis/gopath/src/github.com/jaegertracing/jaeger/plugin/storage/integration/domain_trace_compare_test.go:51 +0x7f
github.com/jaegertracing/jaeger/plugin/storage/integration.(*StorageIntegration).testGetTrace(0xc4202e4060, 0xc42034a0f0)
	/home/travis/gopath/src/github.com/jaegertracing/jaeger/plugin/storage/integration/integration_test.go:191 +0x356
github.com/jaegertracing/jaeger/plugin/storage/integration.(*StorageIntegration).(github.com/jaegertracing/jaeger/plugin/storage/integration.testGetTrace)-fm(0xc42034a0f0)
	/home/travis/gopath/src/github.com/jaegertracing/jaeger/plugin/storage/integration/integration_test.go:381 +0x4c
testing.tRunner(0xc42034a0f0, 0xc42004c250)
	/home/travis/.gimme/versions/go1.10.linux.amd64/src/testing/testing.go:777 +0x16e
created by testing.(*T).Run
	/home/travis/.gimme/versions/go1.10.linux.amd64/src/testing/testing.go:824 +0x565
FAIL	github.com/jaegertracing/jaeger/plugin/storage/integration	6.308s

@davit-y @vprithvi do you think it's protobuf-related or just an unstable test?

Signed-off-by: Yuri Shkuro <ys@uber.com>
@codecov
Copy link

codecov bot commented Aug 13, 2018

Codecov Report

Merging #984 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #984   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files         139    139           
  Lines        6400   6400           
=====================================
  Hits         6400   6400

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 c77bb1d...e889314. Read the comment docs.

@vprithvi
Copy link
Contributor

I think this test is unstable, I was able to get it to fail locally without the protobuf changes.

@yurishkuro yurishkuro merged commit 48695e6 into master Aug 14, 2018
@ghost ghost removed the review label Aug 14, 2018
@isaachier
Copy link
Contributor

According to @vprithvi we should reconsider #985.

@yurishkuro yurishkuro deleted the fix-proto-gen-versions branch August 14, 2018 03:28
isaachier pushed a commit to isaachier/jaeger that referenced this pull request Sep 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants