Skip to content

Commit

Permalink
CBG-3851 move AssertLogContains tests to run without race detector (#…
Browse files Browse the repository at this point in the history
…6873)

AssertLogContains swaps the global logger which can trigger a data race
if other things are writing to the logger (e.g. walrus or gocb
background tasks)

The benefit of running race on these tests would be marginal.
  • Loading branch information
torcolvin authored Jun 5, 2024
1 parent f95497b commit 7268258
Show file tree
Hide file tree
Showing 6 changed files with 210 additions and 147 deletions.
34 changes: 34 additions & 0 deletions rest/config_no_race_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// Copyright 2024-Present Couchbase, Inc.
//
// Use of this software is governed by the Business Source License included
// in the file licenses/BSL-Couchbase.txt. As of the Change Date specified
// in that file, in accordance with the Business Source License, use of this
// software will be governed by the Apache License, Version 2.0, included in
// the file licenses/APL2.txt.

//go:build !race

package rest

import (
"testing"

"github.com/couchbase/sync_gateway/auth"
"github.com/couchbase/sync_gateway/base"
)

// AssertLogContains can hit the race detector due to swapping the global loggers

func TestBadCORSValuesConfig(t *testing.T) {
rt := NewRestTester(t, &RestTesterConfig{PersistentConfig: true})
defer rt.Close()

// expect database to be created with bad CORS values, but do log a warning
dbConfig := rt.NewDbConfig()
dbConfig.CORS = &auth.CORSConfig{
Origin: []string{"http://example.com", "1http://example.com"},
}
base.AssertLogContains(t, "cors.origin contains values", func() {
rt.CreateDatabase("db", dbConfig)
})
}
14 changes: 0 additions & 14 deletions rest/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2768,17 +2768,3 @@ func TestDatabaseConfigDropScopes(t *testing.T) {
require.Contains(t, resp.Body.String(), "cannot change scope")

}

func TestBadCORSValuesConfig(t *testing.T) {
rt := NewRestTester(t, &RestTesterConfig{PersistentConfig: true})
defer rt.Close()

// expect database to be created with bad CORS values, but do log a warning
dbConfig := rt.NewDbConfig()
dbConfig.CORS = &auth.CORSConfig{
Origin: []string{"http://example.com", "1http://example.com"},
}
base.AssertLogContains(t, "cors.origin contains values", func() {
rt.CreateDatabase("db", dbConfig)
})
}
112 changes: 112 additions & 0 deletions rest/handler_no_race_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
// Copyright 2024-Present Couchbase, Inc.
//
// Use of this software is governed by the Business Source License included
// in the file licenses/BSL-Couchbase.txt. As of the Change Date specified
// in that file, in accordance with the Business Source License, use of this
// software will be governed by the Apache License, Version 2.0, included in
// the file licenses/APL2.txt.

//go:build !race

package rest

import (
"fmt"
"net/http"
"testing"

"github.com/couchbase/sync_gateway/base"
)

// AssertLogContains can hit the race detector due to swapping the global loggers
func TestHTTPLoggingRedaction(t *testing.T) {

base.SetUpTestLogging(t, base.LevelInfo, base.KeyHTTP)
base.LongRunningTest(t)
rt := NewRestTester(t, nil)
defer rt.Close()

keyspace := rt.GetSingleKeyspace()

cases := []struct {
name, method, path, expectedLog string
admin bool
}{
{
name: "docid",
method: http.MethodGet,
path: "/db/test",
expectedLog: "/db/<ud>test</ud>",
},
{
name: "local",
method: http.MethodGet,
path: "/db/_local/test",
expectedLog: "/db/_local/<ud>test</ud>",
},
{
name: "raw-docid",
method: http.MethodGet,
path: fmt.Sprintf("/%s/_raw/test", keyspace),
expectedLog: fmt.Sprintf("/%s/_raw/<ud>test</ud>", keyspace),
admin: true,
},
{
name: "revtree-docid",
method: http.MethodGet,
path: fmt.Sprintf("/%s/_revtree/test", keyspace),
expectedLog: fmt.Sprintf("/%s/_revtree/<ud>test</ud>", keyspace),
admin: true,
},
{
name: "docid-attach",
method: http.MethodGet,
path: fmt.Sprintf("/%s/test/attach", keyspace),
expectedLog: fmt.Sprintf("/%s/<ud>test</ud>/<ud>attach</ud>", keyspace),
},
{
name: "docid-attach-equalnames",
method: http.MethodGet,
path: fmt.Sprintf("/%s/test/test", keyspace),
expectedLog: fmt.Sprintf("/%s/<ud>test</ud>/<ud>test</ud>", keyspace),
},
{
name: "user",
method: http.MethodGet,
path: "/db/_user/foo",
expectedLog: "/db/_user/<ud>foo</ud>",
admin: true,
},
{
name: "userSession",
method: http.MethodDelete,
path: "/db/_user/foo/_session",
expectedLog: "/db/_user/<ud>foo</ud>/_session",
admin: true,
},
{
name: "role",
method: http.MethodGet,
path: "/db/_role/foo",
expectedLog: "/db/_role/<ud>foo</ud>",
admin: true,
},
{
name: "CBG-2059",
method: http.MethodGet,
path: fmt.Sprintf("/%s/db", keyspace),
expectedLog: fmt.Sprintf("/%s/<ud>db</ud>", keyspace),
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
base.AssertLogContains(t, tc.expectedLog, func() {
if tc.admin {
_ = rt.SendAdminRequest(tc.method, tc.path, "")
} else {
_ = rt.SendRequest(tc.method, tc.path, "")
}
})
})
}
}
92 changes: 0 additions & 92 deletions rest/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,98 +82,6 @@ func TestParseHTTPRangeHeader(t *testing.T) {
}
}

func TestHTTPLoggingRedaction(t *testing.T) {

base.SetUpTestLogging(t, base.LevelInfo, base.KeyHTTP)
base.LongRunningTest(t)
rt := NewRestTester(t, nil)
defer rt.Close()

keyspace := rt.GetSingleKeyspace()

cases := []struct {
name, method, path, expectedLog string
admin bool
}{
{
name: "docid",
method: http.MethodGet,
path: "/db/test",
expectedLog: "/db/<ud>test</ud>",
},
{
name: "local",
method: http.MethodGet,
path: "/db/_local/test",
expectedLog: "/db/_local/<ud>test</ud>",
},
{
name: "raw-docid",
method: http.MethodGet,
path: fmt.Sprintf("/%s/_raw/test", keyspace),
expectedLog: fmt.Sprintf("/%s/_raw/<ud>test</ud>", keyspace),
admin: true,
},
{
name: "revtree-docid",
method: http.MethodGet,
path: fmt.Sprintf("/%s/_revtree/test", keyspace),
expectedLog: fmt.Sprintf("/%s/_revtree/<ud>test</ud>", keyspace),
admin: true,
},
{
name: "docid-attach",
method: http.MethodGet,
path: fmt.Sprintf("/%s/test/attach", keyspace),
expectedLog: fmt.Sprintf("/%s/<ud>test</ud>/<ud>attach</ud>", keyspace),
},
{
name: "docid-attach-equalnames",
method: http.MethodGet,
path: fmt.Sprintf("/%s/test/test", keyspace),
expectedLog: fmt.Sprintf("/%s/<ud>test</ud>/<ud>test</ud>", keyspace),
},
{
name: "user",
method: http.MethodGet,
path: "/db/_user/foo",
expectedLog: "/db/_user/<ud>foo</ud>",
admin: true,
},
{
name: "userSession",
method: http.MethodDelete,
path: "/db/_user/foo/_session",
expectedLog: "/db/_user/<ud>foo</ud>/_session",
admin: true,
},
{
name: "role",
method: http.MethodGet,
path: "/db/_role/foo",
expectedLog: "/db/_role/<ud>foo</ud>",
admin: true,
},
{
name: "CBG-2059",
method: http.MethodGet,
path: fmt.Sprintf("/%s/db", keyspace),
expectedLog: fmt.Sprintf("/%s/<ud>db</ud>", keyspace),
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
base.AssertLogContains(t, tc.expectedLog, func() {
if tc.admin {
_ = rt.SendAdminRequest(tc.method, tc.path, "")
} else {
_ = rt.SendRequest(tc.method, tc.path, "")
}
})
})
}
}

func Test_parseKeyspace(t *testing.T) {
tests := []struct {
ks string
Expand Down
64 changes: 64 additions & 0 deletions rest/importtest/import_no_race_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
// Copyright 2024-Present Couchbase, Inc.
//
// Use of this software is governed by the Business Source License included
// in the file licenses/BSL-Couchbase.txt. As of the Change Date specified
// in that file, in accordance with the Business Source License, use of this
// software will be governed by the Apache License, Version 2.0, included in
// the file licenses/APL2.txt.

//go:build !race

package importtest

import (
"net/http"
"strconv"
"testing"

"github.com/couchbase/sync_gateway/base"
"github.com/couchbase/sync_gateway/rest"
"github.com/stretchr/testify/assert"
)

// AssertLogContains can hit the race detector due to swapping the global loggers

func TestImportFilterLogging(t *testing.T) {
const errorMessage = `ImportFilterError`
importFilter := `function (doc) { console.error("` + errorMessage + `"); return doc.type == "mobile"; }`
rtConfig := rest.RestTesterConfig{
SyncFn: `function(doc, oldDoc) { channel(doc.channels) }`,
DatabaseConfig: &rest.DatabaseConfig{DbConfig: rest.DbConfig{
ImportFilter: &importFilter,
AutoImport: false,
}},
}
rt := rest.NewRestTesterDefaultCollection(t, &rtConfig) // use default collection since we are using default sync function
defer rt.Close()

// Add document to bucket
key := "ValidImport"
body := make(map[string]interface{})
body["type"] = "mobile"
body["channels"] = "A"
ok, err := rt.GetSingleDataStore().Add(key, 0, body)
assert.NoError(t, err)
assert.True(t, ok)

// Get number of errors before
numErrors, err := strconv.Atoi(base.SyncGatewayStats.GlobalStats.ResourceUtilizationStats().ErrorCount.String())
assert.NoError(t, err)

// Attempt to get doc will trigger import
base.AssertLogContains(t, errorMessage, func() {
response := rt.SendAdminRequest("GET", "/db/"+key, "")
assert.Equal(t, http.StatusOK, response.Code)
})

// Get number of errors after
numErrorsAfter, err := strconv.Atoi(base.SyncGatewayStats.GlobalStats.ResourceUtilizationStats().ErrorCount.String())
assert.NoError(t, err)

// Make sure at least one error was logged
assert.GreaterOrEqual(t, numErrors+1, numErrorsAfter)

}
41 changes: 0 additions & 41 deletions rest/importtest/import_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -583,47 +583,6 @@ func TestXattrImportFilterOptIn(t *testing.T) {
assertDocProperty(t, response, "rev", "1-25c26cdf9d7771e07f00be1d13f7fb7c")
}

func TestImportFilterLogging(t *testing.T) {
const errorMessage = `ImportFilterError`
importFilter := `function (doc) { console.error("` + errorMessage + `"); return doc.type == "mobile"; }`
rtConfig := rest.RestTesterConfig{
SyncFn: `function(doc, oldDoc) { channel(doc.channels) }`,
DatabaseConfig: &rest.DatabaseConfig{DbConfig: rest.DbConfig{
ImportFilter: &importFilter,
AutoImport: false,
}},
}
rt := rest.NewRestTesterDefaultCollection(t, &rtConfig) // use default collection since we are using default sync function
defer rt.Close()

// Add document to bucket
key := "ValidImport"
body := make(map[string]interface{})
body["type"] = "mobile"
body["channels"] = "A"
ok, err := rt.GetSingleDataStore().Add(key, 0, body)
assert.NoError(t, err)
assert.True(t, ok)

// Get number of errors before
numErrors, err := strconv.Atoi(base.SyncGatewayStats.GlobalStats.ResourceUtilizationStats().ErrorCount.String())
assert.NoError(t, err)

// Attempt to get doc will trigger import
base.AssertLogContains(t, errorMessage, func() {
response := rt.SendAdminRequest("GET", "/db/"+key, "")
assert.Equal(t, http.StatusOK, response.Code)
})

// Get number of errors after
numErrorsAfter, err := strconv.Atoi(base.SyncGatewayStats.GlobalStats.ResourceUtilizationStats().ErrorCount.String())
assert.NoError(t, err)

// Make sure at least one error was logged
assert.GreaterOrEqual(t, numErrors+1, numErrorsAfter)

}

// Test scenario where another actor updates a different xattr on a document. Sync Gateway
// should detect and not import/create new revision during read-triggered import
func TestXattrImportMultipleActorOnDemandGet(t *testing.T) {
Expand Down

0 comments on commit 7268258

Please sign in to comment.