CBG-4455 do not panic when calling FailNow (#7308)

* CBG-4455 fix FailNow calls to avoid panics

* Add license

.golangci-strict.yml

@@ -105,4 +105,5 @@ linters-settings:
       - wrapperFunc
     settings:
       ruleguard:
-        rules: '${configDir}/ruleguard/*.go'
+        rules: '${configDir}/ruleguard/rules-*.go'
+        failOn: all

.golangci.yml

@@ -64,7 +64,8 @@ linters-settings:
       - ruleguard
     settings:
       ruleguard:
-        rules: '${configDir}/ruleguard/*.go'
+        rules: '${configDir}/ruleguard/rules-*.go'
+        failOn: all
 
 # Disable goconst in test files, often we have duplicated strings across tests, but don't make sense as constants.
 issues:

base/collection_xattr_test.go

@@ -1324,11 +1324,8 @@ func TestWriteWithXattrsInsertAndDeleteError(t *testing.T) {
 func requireXattrsEqual(t testing.TB, expected map[string][]byte, actual map[string][]byte) {
 	require.Len(t, actual, len(expected), "Expected xattrs to be the same length %v, got %v", expected, actual)
 	for k, v := range expected {
-		actualV, ok := actual[k]
-		if !ok {
-			require.Fail(t, "Missing expected xattr %s", k)
-		}
-		require.JSONEq(t, string(v), string(actualV))
+		require.Contains(t, actual, k)
+		require.JSONEq(t, string(v), string(actual[k]))
 	}
 }
 

base/dcp_test.go

@@ -466,7 +466,7 @@ func TestConcurrentCBGTIndexCreation(t *testing.T) {
 			case <-terminatorChan:
 				context.Manager.Stop()
 			case <-time.After(20 * time.Second):
-				assert.Fail(t, "manager goroutine not terminated: %v", managerUUID)
+				require.Fail(t, fmt.Sprintf("manager goroutine not terminated: %v", managerUUID))
 			}
 
 		}(i, terminator)

db/design_doc_util_test.go

@@ -33,8 +33,6 @@ func assertDesignDocExists(t testing.TB, viewStore sgbucket.ViewStore, ddocName
 // assertDesignDocDoesNotExist ensures that the design doc does not exist in the dataStore.
 func assertDesignDocNotExists(t testing.TB, viewStore sgbucket.ViewStore, ddocName string) bool {
 	ddoc, err := viewStore.GetDDoc(ddocName)
-	if err == nil {
-		return assert.Failf(t, "Design doc %s should not exist but but it did: %v", ddocName, ddoc)
-	}
+	assert.Error(t, err, "Design doc %s should not exist but but it did: %v", ddocName, ddoc)
 	return assert.Truef(t, IsMissingDDocError(err), "Design doc %s should not exist but got a different error fetching it: %v", ddocName, err)
 }

rest/attachment_test.go

@@ -2443,11 +2443,10 @@ func TestAttachmentWithErroneousRevPos(t *testing.T) {
 		btcRunner.AttachmentsLock(btc.id).Unlock()
 
 		// Put doc with an erroneous revpos 1 but with a different digest, referring to the above attachment
-		_, err := btcRunner.PushRevWithHistory(btc.id, docID, &version, []byte(`{"_attachments": {"hello.txt": {"revpos":1,"stub":true,"length": 19,"digest":"sha1-l+N7VpXGnoxMm8xfvtWPbz2YvDc="}}}`), 1, 0)
+		updatedVersion, err := btcRunner.PushRevWithHistory(btc.id, docID, &version, []byte(`{"_attachments": {"hello.txt": {"revpos":1,"stub":true,"length": 19,"digest":"sha1-l+N7VpXGnoxMm8xfvtWPbz2YvDc="}}}`), 1, 0)
 		require.NoError(t, err)
 
-		// Ensure message and attachment is pushed up
-		btc.pushReplication.WaitForMessage(2)
+		rt.WaitForVersion(docID, *updatedVersion)
 
 		// Get the attachment and ensure the data is updated
 		resp := btc.rt.SendAdminRequest(http.MethodGet, "/{{.keyspace}}/doc/hello.txt", "")
@@ -2610,24 +2609,24 @@ func TestCBLRevposHandling(t *testing.T) {
 		btc := btcRunner.NewBlipTesterClientOptsWithRT(rt, &opts)
 		defer btc.Close()
 
-		doc1Version := btc.rt.PutDoc(doc1ID, `{}`)
-		doc2Version := btc.rt.PutDoc(doc2ID, `{}`)
+		doc1Version1 := btc.rt.PutDoc(doc1ID, `{}`)
+		doc2Version1 := btc.rt.PutDoc(doc2ID, `{}`)
 
 		btc.rt.WaitForPendingChanges()
 		btcRunner.StartOneshotPull(btc.id)
-		btcRunner.WaitForVersion(btc.id, doc1ID, doc1Version)
-		btcRunner.WaitForVersion(btc.id, doc2ID, doc2Version)
+		btcRunner.WaitForVersion(btc.id, doc1ID, doc1Version1)
+		btcRunner.WaitForVersion(btc.id, doc2ID, doc2Version1)
 
 		btcRunner.StartPush(btc.id)
 
 		attachmentAData := base64.StdEncoding.EncodeToString([]byte("attachmentA"))
 		attachmentBData := base64.StdEncoding.EncodeToString([]byte("attachmentB"))
 
-		doc1Version = btcRunner.AddRev(btc.id, doc1ID, &doc1Version, []byte(`{"key": "val", "_attachments": {"attachment": {"data": "`+attachmentAData+`"}}}`))
-		doc2Version = btcRunner.AddRev(btc.id, doc2ID, &doc2Version, []byte(`{"key": "val", "_attachments": {"attachment": {"data": "`+attachmentBData+`"}}}`))
+		doc1Version2 := btcRunner.AddRev(btc.id, doc1ID, &doc1Version1, []byte(`{"key": "val", "_attachments": {"attachment": {"data": "`+attachmentAData+`"}}}`))
+		doc2Version2 := btcRunner.AddRev(btc.id, doc2ID, &doc2Version1, []byte(`{"key": "val", "_attachments": {"attachment": {"data": "`+attachmentBData+`"}}}`))
 
-		btc.rt.WaitForVersion(doc1ID, doc1Version)
-		btc.rt.WaitForVersion(doc2ID, doc2Version)
+		btc.rt.WaitForVersion(doc1ID, doc1Version2)
+		btc.rt.WaitForVersion(doc2ID, doc2Version2)
 
 		collection, ctx := btc.rt.GetSingleTestDatabaseCollection()
 		_, err := collection.GetDocument(ctx, "doc1", db.DocUnmarshalAll)
@@ -2636,12 +2635,12 @@ func TestCBLRevposHandling(t *testing.T) {
 		require.NoError(t, err)
 
 		// Update doc1, don't change attachment, use correct revpos
-		doc1Version = btcRunner.AddRev(btc.id, doc1ID, &doc1Version, []byte(`{"key": "val", "_attachments":{"attachment":{"digest":"sha1-wzp8ZyykdEuZ9GuqmxQ7XDrY7Co=","length":11,"content_type":"","stub":true,"revpos":2}}}`))
-		btc.rt.WaitForVersion(doc1ID, doc1Version)
+		doc1Version3 := btcRunner.AddRev(btc.id, doc1ID, &doc1Version2, []byte(`{"key": "val", "_attachments":{"attachment":{"digest":"sha1-wzp8ZyykdEuZ9GuqmxQ7XDrY7Co=","length":11,"content_type":"","stub":true,"revpos":2}}}`))
+		btc.rt.WaitForVersion(doc1ID, doc1Version3)
 
 		// Update doc1, don't change attachment, use revpos=generation of revid, as CBL 2.x does.  Should not proveAttachment on digest match.
-		doc1Version = btcRunner.AddRev(btc.id, doc1ID, &doc1Version, []byte(`{"key": "val", "_attachments":{"attachment":{"digest":"sha1-wzp8ZyykdEuZ9GuqmxQ7XDrY7Co=","length":11,"content_type":"","stub":true,"revpos":4}}}`))
-		rt.WaitForVersion(doc1ID, doc1Version)
+		doc1Version4 := btcRunner.AddRev(btc.id, doc1ID, &doc1Version3, []byte(`{"key": "val", "_attachments":{"attachment":{"digest":"sha1-wzp8ZyykdEuZ9GuqmxQ7XDrY7Co=","length":11,"content_type":"","stub":true,"revpos":4}}}`))
+		rt.WaitForVersion(doc1ID, doc1Version4)
 
 		// Validate attachment exists
 		attResponse := btc.rt.SendAdminRequest("GET", "/{{.keyspace}}/doc1/attachment", "")
@@ -2650,8 +2649,8 @@ func TestCBLRevposHandling(t *testing.T) {
 
 		attachmentPushCount := btc.rt.GetDatabase().DbStats.CBLReplicationPushStats.AttachmentPushCount.Value()
 		// Update doc1, change attachment digest with CBL revpos=generation.  Should getAttachment
-		doc1Version = btcRunner.AddRev(btc.id, doc1ID, &doc1Version, []byte(`{"key": "val", "_attachments":{"attachment":{"digest":"sha1-SKk0IV40XSHW37d3H0xpv2+z9Ck=","length":11,"content_type":"","stub":true,"revpos":5}}}`))
-		rt.WaitForVersion(doc1ID, doc1Version)
+		doc1Version5 := btcRunner.AddRev(btc.id, doc1ID, &doc1Version4, []byte(`{"key": "val", "_attachments":{"attachment":{"digest":"sha1-SKk0IV40XSHW37d3H0xpv2+z9Ck=","length":11,"content_type":"","stub":true,"revpos":5}}}`))
+		rt.WaitForVersion(doc1ID, doc1Version5)
 
 		// Validate attachment exists and is updated
 		attResponse = btc.rt.SendAdminRequest("GET", "/{{.keyspace}}/doc1/attachment", "")

rest/blip_api_attachment_test.go

@@ -64,21 +64,18 @@ func TestBlipPushPullV2AttachmentV2Client(t *testing.T) {
 
 		// Create doc revision with attachment on SG.
 		bodyText := `{"greetings":[{"hi": "alice"}],"_attachments":{"hello.txt":{"data":"aGVsbG8gd29ybGQ="}}}`
-		version := btc.rt.PutDoc(docID, bodyText)
+		version1 := btc.rt.PutDoc(docID, bodyText)
 
-		data := btcRunner.WaitForVersion(btc.id, docID, version)
+		data := btcRunner.WaitForVersion(btc.id, docID, version1)
 		bodyTextExpected := `{"greetings":[{"hi":"alice"}],"_attachments":{"hello.txt":{"revpos":1,"length":11,"stub":true,"digest":"sha1-Kq5sNclPz7QV2+lfQIuc6R7oRu0="}}}`
 		require.JSONEq(t, bodyTextExpected, string(data))
 
 		// Update the replicated doc at client along with keeping the same attachment stub.
 		bodyText = `{"greetings":[{"hi":"bob"}],"_attachments":{"hello.txt":{"revpos":1,"length":11,"stub":true,"digest":"sha1-Kq5sNclPz7QV2+lfQIuc6R7oRu0="}}}`
-		version = btcRunner.AddRev(btc.id, docID, &version, []byte(bodyText))
+		version2 := btcRunner.AddRev(btc.id, docID, &version1, []byte(bodyText))
 
-		// TODO: Replace with rt.WaitForVersion
-		// Wait for the document to be replicated at SG
-		btc.pushReplication.WaitForMessage(2)
-
-		respBody := btc.rt.GetDocVersion(docID, version)
+		rt.WaitForVersion(docID, version2)
+		respBody := btc.rt.GetDocVersion(docID, version2)
 
 		assert.Equal(t, docID, respBody[db.BodyId])
 		greetings := respBody["greetings"].([]interface{})
@@ -135,20 +132,19 @@ func TestBlipPushPullV2AttachmentV3Client(t *testing.T) {
 
 		// Create doc revision with attachment on SG.
 		bodyText := `{"greetings":[{"hi": "alice"}],"_attachments":{"hello.txt":{"data":"aGVsbG8gd29ybGQ="}}}`
-		version := btc.rt.PutDoc(docID, bodyText)
+		version1 := btc.rt.PutDoc(docID, bodyText)
 
-		data := btcRunner.WaitForVersion(btc.id, docID, version)
+		data := btcRunner.WaitForVersion(btc.id, docID, version1)
 		bodyTextExpected := `{"greetings":[{"hi":"alice"}],"_attachments":{"hello.txt":{"revpos":1,"length":11,"stub":true,"digest":"sha1-Kq5sNclPz7QV2+lfQIuc6R7oRu0="}}}`
 		require.JSONEq(t, bodyTextExpected, string(data))
 
 		// Update the replicated doc at client along with keeping the same attachment stub.
 		bodyText = `{"greetings":[{"hi":"bob"}],"_attachments":{"hello.txt":{"revpos":1,"length":11,"stub":true,"digest":"sha1-Kq5sNclPz7QV2+lfQIuc6R7oRu0="}}}`
-		version = btcRunner.AddRev(btc.id, docID, &version, []byte(bodyText))
+		version2 := btcRunner.AddRev(btc.id, docID, &version1, []byte(bodyText))
 
-		// Wait for the document to be replicated at SG
-		btc.pushReplication.WaitForMessage(2)
+		rt.WaitForVersion(docID, version2)
 
-		respBody := btc.rt.GetDocVersion(docID, version)
+		respBody := btc.rt.GetDocVersion(docID, version2)
 
 		assert.Equal(t, docID, respBody[db.BodyId])
 		greetings := respBody["greetings"].([]interface{})

rest/blip_api_delta_sync_test.go

@@ -130,20 +130,18 @@ func TestBlipDeltaSyncPushPullNewAttachment(t *testing.T) {
 
 		// Create doc1 rev 1-77d9041e49931ceef58a1eef5fd032e8 on SG with an attachment
 		bodyText := `{"greetings":[{"hi": "alice"}],"_attachments":{"hello.txt":{"data":"aGVsbG8gd29ybGQ="}}}`
-		version := rt.PutDoc(docID, bodyText)
-		data := btcRunner.WaitForVersion(btc.id, docID, version)
+		version1 := rt.PutDoc(docID, bodyText)
+		data := btcRunner.WaitForVersion(btc.id, docID, version1)
 
 		bodyTextExpected := `{"greetings":[{"hi":"alice"}],"_attachments":{"hello.txt":{"revpos":1,"length":11,"stub":true,"digest":"sha1-Kq5sNclPz7QV2+lfQIuc6R7oRu0="}}}`
 		require.JSONEq(t, bodyTextExpected, string(data))
 
 		// Update the replicated doc at client by adding another attachment.
 		bodyText = `{"greetings":[{"hi":"alice"}],"_attachments":{"hello.txt":{"revpos":1,"length":11,"stub":true,"digest":"sha1-Kq5sNclPz7QV2+lfQIuc6R7oRu0="},"world.txt":{"data":"bGVsbG8gd29ybGQ="}}}`
-		version = btcRunner.AddRev(btc.id, docID, &version, []byte(bodyText))
+		version2 := btcRunner.AddRev(btc.id, docID, &version1, []byte(bodyText))
 
-		// Wait for the document to be replicated at SG
-		btc.pushReplication.WaitForMessage(2)
-
-		respBody := rt.GetDocVersion(docID, version)
+		rt.WaitForVersion(docID, version2)
+		respBody := rt.GetDocVersion(docID, version2)
 
 		assert.Equal(t, docID, respBody[db.BodyId])
 		greetings := respBody["greetings"].([]interface{})
@@ -848,6 +846,8 @@ func TestBlipDeltaSyncPush(t *testing.T) {
 			assert.NotEqual(t, `{"greetings":{"2-":[{"howdy":"bob"}]}}`, string(msgBody))
 			assert.Equal(t, `{"greetings":[{"hello":"world!"},{"hi":"alice"},{"howdy":"bob"}]}`, string(msgBody))
 		}
+		// wait for response body, indicating rev was written to server
+		_ = msg.Response()
 
 		respBody := rt.GetDocVersion(docID, newRev)
 		assert.Equal(t, "doc1", respBody[db.BodyId])
@@ -922,24 +922,28 @@ func TestBlipNonDeltaSyncPush(t *testing.T) {
 		btcRunner.StartPush(client.id)
 
 		// create doc1 rev 1-0335a345b6ffed05707ccc4cbc1b67f4
-		version := rt.PutDoc(docID, `{"greetings": [{"hello": "world!"}, {"hi": "alice"}]}`)
+		version1 := rt.PutDoc(docID, `{"greetings": [{"hello": "world!"}, {"hi": "alice"}]}`)
 
-		data := btcRunner.WaitForVersion(client.id, docID, version)
+		data := btcRunner.WaitForVersion(client.id, docID, version1)
 		assert.Equal(t, `{"greetings":[{"hello":"world!"},{"hi":"alice"}]}`, string(data))
 		// create doc1 rev 2-abcxyz on client
-		newRev := btcRunner.AddRev(client.id, docID, &version, []byte(`{"greetings":[{"hello":"world!"},{"hi":"alice"},{"howdy":"bob"}]}`))
-		// Check EE is delta, and CE is full-body replication
+		version2 := btcRunner.AddRev(client.id, docID, &version1, []byte(`{"greetings":[{"hello":"world!"},{"hi":"alice"},{"howdy":"bob"}]}`))
+		// MSG1: proposeChanges
+		// MSG2: rev
 		msg := client.waitForReplicationMessage(collection, 2)
+		require.Equal(t, db.MessageRev, msg.Profile())
+
+		// wait for the reply, indicating the message was written
+		_ = msg.Response()
 
 		// Check the request was NOT sent with a deltaSrc property
 		assert.Equal(t, "", msg.Properties[db.RevMessageDeltaSrc])
 		// Check the request body was NOT the delta
 		msgBody, err := msg.Body()
 		assert.NoError(t, err)
-		assert.NotEqual(t, `{"greetings":{"2-":[{"howdy":"bob"}]}}`, string(msgBody))
 		assert.Equal(t, `{"greetings":[{"hello":"world!"},{"hi":"alice"},{"howdy":"bob"}]}`, string(msgBody))
 
-		body := rt.GetDocVersion("doc1", newRev)
+		body := rt.GetDocVersion("doc1", version2)
 		require.Equal(t, "bob", body["greetings"].([]interface{})[2].(map[string]interface{})["howdy"])
 	})
 }