Skip to content

Commit

Permalink
Fix data race in AssertLogContains and functional race in TestDocChan…
Browse files Browse the repository at this point in the history
…gedLogging (#6517)

* Fix race in AssertLogContains by forcing console log buffer flush before checking contents - avoids multiple buffer reads during log writes
* Fix TestDocChangedLogging test race
  • Loading branch information
bbrks committed Mar 28, 2024
1 parent d352d57 commit ed3f078
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 12 deletions.
16 changes: 4 additions & 12 deletions base/logging.go
Original file line number Diff line number Diff line change
Expand Up @@ -388,22 +388,14 @@ func LogTraceEnabled(logKey LogKey) bool {
// AssertLogContains asserts that the logs produced by function f contain string s.
func AssertLogContains(t *testing.T, s string, f func()) {
// Temporarily override logger output
b := bytes.Buffer{}
mw := io.MultiWriter(&b, os.Stderr)
b := &bytes.Buffer{}
mw := io.MultiWriter(b, os.Stderr)
consoleLogger.logger.SetOutput(mw)
defer func() { consoleLogger.logger.SetOutput(os.Stderr) }()

// Call the given function
f()

// Allow time for logs to be printed
retry := func() (shouldRetry bool, err error, value interface{}) {
if strings.Contains(b.String(), s) {
return false, nil, nil
}
return true, nil, nil
}
err, _ := RetryLoop(TestCtx(t), "wait for logs", retry, CreateSleeperFunc(10, 100))

assert.NoError(t, err, "Console logs did not contain %q", s)
consoleLogger.FlushBufferToLog()
assert.True(t, strings.Contains(b.String(), s), "Console logs did not contain %q", s)
}
4 changes: 4 additions & 0 deletions rest/changestest/changes_api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4337,6 +4337,10 @@ func TestDocChangedLogging(t *testing.T) {
base.AssertLogContains(t, "Ignoring non-metadata mutation for doc", func() {
err := rt.GetDatabase().MetadataStore.Set("doc1", 0, nil, db.Body{"foo": "bar"})
require.NoError(t, err)
// write another doc to ensure the previous non-metadata doc has been seen...
// no other way of synchronising this no-op as no stats to wait on
response = rt.SendAdminRequest("PUT", "/{{.keyspace1}}/doc2", `{"foo":"bar"}`)
rest.RequireStatus(t, response, http.StatusCreated)
require.NoError(t, rt.WaitForPendingChanges())
})
assert.Equal(t, warnCountBefore, base.SyncGatewayStats.GlobalStats.ResourceUtilizationStats().WarnCount.Value())
Expand Down

0 comments on commit ed3f078

Please sign in to comment.