log: use fake stacktrace when test-exiting

[tbg]

Having stack traces littered across tests has been a long-standing pet
peeve of mine (and presumably others). We search through logs more often
than we would like to admit, and having to skip a number of intentional
stack traces is annoying.

Make it so that whenever the logging exit func is mocked, the stack
trace indicates that.

Release note: None

[cockroach-teamcity]

This change is 

[benesch]

It just occurred to me: I think it would be cleaner to split this into two APIs. What we really want is an atexit callback for non tests, and an exit func override for tests. The atexit callback should not be able to prevent the exit, nor should it be responsible for issuing the os.Exit call; it should just run some cleanup code.

though

Reviewed 11 of 11 files at r1.
Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale)

---

pkg/util/log/clog.go, line 695 at r1 (raw file):

	exitOverride struct {
		f         func(int) // overrides os.Exit when non-nil; testing only
		hideStack bool      // hides stack trace; only in effect when f is nil

when f is non-nil, no?

---

pkg/util/log/clog.go, line 857 at r1 (raw file):

			// We're running in a test. It's a little dirty that we're detecting
			// this here, but we definitely don't want to litter stacktraces in
			// the test output when there isn't anything going wrong.

I'm confused by this comment. f can be non-nil outside of tests.

---

pkg/util/log/clog_test.go, line 97 at r1 (raw file):

// setFlags resets the logging flags and exit function to what tests expect.
func setFlags() {
	SetExitFunc(false, nil)

This would read much more nicely as ResetExitFunc().

---

pkg/util/log/clog_test.go, line 701 at r1 (raw file):

	setFlags()
	logging.stderrThreshold = Severity_NONE
	SetExitFunc(false, func(int) {})

/* hideStack */

---

pkg/util/log/log.go, line 53 at r1 (raw file):

	if f == nil && hideStack {
		panic("can't pass hideStack when resetting logger")
	}

Maybe introduce ResetExitFunc() that does the right thing?

[tbg]

TFTR! I introduced ResetExitFunc but stopped shy of the two separate APIs (started but realized it wasn't worth it for the three obscure users).

Reviewable status: complete! 1 of 0 LGTMs obtained

---

pkg/util/log/clog.go, line 695 at r1 (raw file):

Previously, benesch (Nikhil Benesch) wrote…

when f is non-nil, no?

Done.

---

pkg/util/log/clog.go, line 857 at r1 (raw file):

Previously, benesch (Nikhil Benesch) wrote…

I'm confused by this comment. f can be non-nil outside of tests.

Done. The comment predates my finding out this was the case.

[benesch]

Reviewed 7 of 7 files at r2.
Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale)

[tbg]

bors r=benesch

[craig]

Build succeeded

• GitHub CI (Cockroach)