Skip to content

Commit

Permalink
Deterministic calc of POSIX return code for Fatal
Browse files Browse the repository at this point in the history
  • Loading branch information
cardil committed Apr 22, 2022
1 parent d38507c commit 65ee9c6
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 14 deletions.
12 changes: 7 additions & 5 deletions internal/exit/exit.go
Expand Up @@ -24,18 +24,19 @@ package exit

import "os"

var real = func() { os.Exit(1) }
var real = func(code int) { os.Exit(code) }

// Exit normally terminates the process by calling os.Exit(1). If the package
// is stubbed, it instead records a call in the testing spy.
func Exit() {
real()
func Exit(code int) {
real(code)
}

// A StubbedExit is a testing fake for os.Exit.
type StubbedExit struct {
Exited bool
prev func()
Code int
prev func(code int)
}

// Stub substitutes a fake for the call to os.Exit(1).
Expand All @@ -59,6 +60,7 @@ func (se *StubbedExit) Unstub() {
real = se.prev
}

func (se *StubbedExit) exit() {
func (se *StubbedExit) exit(code int) {
se.Exited = true
se.Code = code
}
22 changes: 15 additions & 7 deletions internal/exit/exit_test.go
Expand Up @@ -18,25 +18,33 @@
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
// THE SOFTWARE.

package exit
package exit_test

import (
"testing"

"github.com/stretchr/testify/assert"
"go.uber.org/zap/internal/exit"
)

func TestStub(t *testing.T) {
type want struct {
exit bool
code int
}
tests := []struct {
f func()
want bool
f func()
want
}{
{Exit, true},
{func() {}, false},
{func() {
exit.Exit(42)
}, want{exit: true, code: 42}},
{func() {}, want{}},
}

for _, tt := range tests {
s := WithStub(tt.f)
assert.Equal(t, tt.want, s.Exited, "Stub captured unexpected exit value.")
s := exit.WithStub(tt.f)
assert.Equal(t, tt.want.exit, s.Exited, "Stub captured unexpected exit value.")
assert.Equal(t, tt.want.code, s.Code, "Stub captured unexpected exit value.")
}
}
9 changes: 8 additions & 1 deletion logger_test.go
Expand Up @@ -213,15 +213,22 @@ func TestLoggerAlwaysFatals(t *testing.T) {
// Users can disable writing out fatal-level logs, but calls to logger.Fatal()
// should still terminate the process.
withLogger(t, FatalLevel+1, nil, func(logger *Logger, logs *observer.ObservedLogs) {
stub := exit.WithStub(func() { logger.Fatal("") })
stub := exit.WithStub(func() { logger.Fatal("foo") })
assert.True(t, stub.Exited, "Expected calls to logger.Fatal to terminate process.")
assert.Equal(t, 38, stub.Code, "Expected calls to logger.Fatal to terminate process with predictable retcode.")

err := errors.New("bar")
stub = exit.WithStub(func() { logger.Fatal("foo", Error(err)) })
assert.True(t, stub.Exited, "Expected calls to logger.Fatal to terminate process.")
assert.Equal(t, 129, stub.Code, "Expected calls to logger.Fatal to terminate process with predictable retcode.")

stub = exit.WithStub(func() {
if ce := logger.Check(FatalLevel, ""); ce != nil {
ce.Write()
}
})
assert.True(t, stub.Exited, "Expected calls to logger.Check(FatalLevel, ...) to terminate process.")
assert.Equal(t, 1, stub.Code, "Expected calls to logger.Check(FatalLevel, ...) to terminate process with predictable retcode.")

assert.Equal(t, 0, logs.Len(), "Shouldn't write out logs when fatal-level logging is disabled.")
})
Expand Down
14 changes: 13 additions & 1 deletion zapcore/entry.go
Expand Up @@ -22,6 +22,7 @@ package zapcore

import (
"fmt"
"hash/crc32"
"runtime"
"strings"
"sync"
Expand Down Expand Up @@ -231,7 +232,7 @@ func (ce *CheckedEntry) Write(fields ...Field) {
case WriteThenPanic:
panic(msg)
case WriteThenFatal:
exit.Exit()
exit.Exit(retcode(ce, fields))
case WriteThenGoexit:
runtime.Goexit()
}
Expand Down Expand Up @@ -260,3 +261,14 @@ func (ce *CheckedEntry) Should(ent Entry, should CheckWriteAction) *CheckedEntry
ce.should = should
return ce
}

func retcode(ce *CheckedEntry, fields []Field) int {
msg := ce.Message
for _, field := range fields {
if field.Type == ErrorType {
msg = field.Interface.(error).Error()
break
}
}
return int(crc32.ChecksumIEEE([]byte(msg)))%254 + 1
}
2 changes: 2 additions & 0 deletions zapcore/entry_test.go
Expand Up @@ -124,9 +124,11 @@ func TestCheckedEntryWrite(t *testing.T) {
t.Run("WriteThenFatal", func(t *testing.T) {
var ce *CheckedEntry
ce = ce.Should(Entry{}, WriteThenFatal)
ce.Message = t.Name()
stub := exit.WithStub(func() {
ce.Write()
})
assert.True(t, stub.Exited, "Expected to exit when WriteThenFatal is set.")
assert.Equal(t, 57, stub.Code, "Expected to exit with specific code when WriteThenFatal is set.")
})
}

0 comments on commit 65ee9c6

Please sign in to comment.