Use Errorf instead of fatal in testSampleTracepointStack #18
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The rule I was taught by another developer was "use Fatal if there is nothing
useful you can further do in the test, otherwise use Error". This test in
particular does do some useful things, it logs the stacks that it observed,
which immediately makes the problem clear to me.
Before we'd just get the first message, now we get all of this:
```
--- FAIL: TestReadRecord/SampleTracepointStack (0.04s)
record_test.go:1101: Go (0x50a0ff) and kernel (0x513eff) PC differ
record_test.go:1101: Go (0x513eff) and kernel (0x5043fb) PC differ
record_test.go:1117: kernel callchain:
record_test.go:1110: 0xfffffffffffffe00 <nil>
record_test.go:1113: 0x5043fb /home/pwaller/go/pkg/mod/golang.org/x/sys@v0.0.0-20190309122539-980fc434d28e/unix/asm_linux_amd64.s:52 golang.org/x/sys/unix.RawSyscallNoError
record_test.go:1113: 0x513eff /home/pwaller/.local/src/acln.ro/perf/perf_amd64.s:18 acln.ro/perf.doEnableRunDisable
record_test.go:1113: 0x52681b /home/pwaller/.local/src/acln.ro/perf/record_test.go:1070 acln.ro/perf_test.testSampleTracepointStack
record_test.go:1113: 0x4c0bb0 /snap/go/4098/src/testing/testing.go:868 testing.tRunner
record_test.go:1113: 0x45aa41 /snap/go/4098/src/runtime/asm_amd64.s:1338 runtime.goexit
record_test.go:1122:
record_test.go:1124: Go stack:
record_test.go:1113: 0x513eff /home/pwaller/.local/src/acln.ro/perf/perf_amd64.s:18 acln.ro/perf.doEnableRunDisable
record_test.go:1113: 0x50a0ff /home/pwaller/.local/src/acln.ro/perf/perf.go:276 acln.ro/perf.(*Event).Measure
record_test.go:1113: 0x52681b /home/pwaller/.local/src/acln.ro/perf/record_test.go:1070 acln.ro/perf_test.testSampleTracepointStack
record_test.go:1113: 0x4c0bb0 /snap/go/4098/src/testing/testing.go:868 testing.tRunner
record_test.go:1113: 0x45aa41 /snap/go/4098/src/runtime/asm_amd64.s:1338 runtime.goexit
```
---
I have looked at the usage of Fatalf elsewhere in the tests and decided there
were too many of them for me to go and update them all quickly and without
making a mistake late a night :)
Updates acln0#13.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The rule I was taught by another developer was "use Fatal if there is nothing
useful you can further do in the test, otherwise use Error". This test in
particular does do some useful things, it logs the stacks that it observed,
which immediately makes the problem clear to me.
Before we'd just get the first message, now we get all of this:
I have looked at the usage of Fatalf elsewhere in the tests and decided there
were too many of them for me to go and update them all quickly and without
making a mistake late a night :)
Updates #13.