code review remarks

FerretDB · Mar 5, 2024 · efd071b · efd071b
1 parent 0173295
commit efd071b
Show file tree

Hide file tree

Showing 7 changed files with 15 additions and 42 deletions.
diff --git a/integration/commands_administration_test.go b/integration/commands_administration_test.go
@@ -252,7 +252,6 @@ func TestCommandsAdministrationListDatabases(t *testing.T) {
 	}
 
 	for name, tc := range testCases {
-		tc, name := tc, name
 		t.Run(name, func(t *testing.T) {
 			t.Parallel()
 

diff --git a/integration/create_test.go b/integration/create_test.go
@@ -333,7 +333,6 @@ func TestCreateCappedCommandInvalidSpec(t *testing.T) {
 			},
 		},
 	} {
-		tc, name := tc, name
 		t.Run(name, func(t *testing.T) {
 			t.Parallel()
 

diff --git a/integration/hello_command_test.go b/integration/hello_command_test.go
@@ -133,8 +133,6 @@ func TestHelloWithSupportedMechs(t *testing.T) {
 	}
 
 	for name, tc := range testCases {
-		tc, name := tc, name
-
 		t.Run(name, func(t *testing.T) {
 			t.Parallel()
 
@@ -156,13 +154,6 @@ func TestHelloWithSupportedMechs(t *testing.T) {
 
 			actual := ConvertDocument(t, res)
 
-			if tc.mechs != nil {
-				mechanisms := must.NotFail(actual.Get("saslSupportedMechs"))
-				assert.True(t, mechanisms.(*types.Array).ContainsAll(tc.mechs))
-			} else {
-				assert.False(t, actual.Has("saslSupportedMechs"))
-			}
-
 			assert.Equal(t, must.NotFail(actual.Get("isWritablePrimary")), true)
 			assert.Equal(t, must.NotFail(actual.Get("maxBsonObjectSize")), int32(16777216))
 			assert.Equal(t, must.NotFail(actual.Get("maxMessageSizeBytes")), int32(48000000))
@@ -174,6 +165,14 @@ func TestHelloWithSupportedMechs(t *testing.T) {
 			assert.Equal(t, must.NotFail(actual.Get("maxWireVersion")), int32(21))
 			assert.Equal(t, must.NotFail(actual.Get("readOnly")), false)
 			assert.Equal(t, must.NotFail(actual.Get("ok")), float64(1))
+
+			if tc.mechs == nil {
+				assert.False(t, actual.Has("saslSupportedMechs"))
+				return
+			}
+
+			mechanisms := must.NotFail(actual.Get("saslSupportedMechs"))
+			assert.True(t, mechanisms.(*types.Array).ContainsAll(tc.mechs))
 		})
 	}
 }
diff --git a/integration/query_test.go b/integration/query_test.go
@@ -728,7 +728,6 @@ func TestQueryCommandLimitPushDown(t *testing.T) {
 			limitPushdown: noPushdown,
 		},
 	} {
-		tc, name := tc, name
 		t.Run(name, func(t *testing.T) {
 			t.Parallel()
 

diff --git a/integration/setup/helpers.go b/integration/setup/helpers.go
@@ -89,7 +89,7 @@ func FailsForMongoDB(tb testtb.TB, reason string) testtb.TB {
 
 // SkipForMongoDB skips the current test for MongoDB.
 //
-// Use [FailsForMongoDB] in new code.
+// Deprecated: Use [FailsForMongoDB] in new code.
 func SkipForMongoDB(tb testtb.TB, reason string) {
 	tb.Helper()
 

diff --git a/integration/users/usersinfo_test.go b/integration/users/usersinfo_test.go
@@ -532,7 +532,6 @@ func TestUsersinfo(t *testing.T) {
 	}
 
 	for name, tc := range testCases {
-		name, tc := name, tc
 		t.Run(name, func(t *testing.T) {
 			t.Parallel()
 
@@ -575,28 +574,23 @@ func TestUsersinfo(t *testing.T) {
 
 				require.True(t, (tc.hasUser == nil) != (tc.expected == nil))
 
-				payload := integration.ConvertDocument(t, tc.payload)
-
 				if tc.showCredentials != nil {
-					cred, ok := actualUser.Get("credentials")
-					assert.Nil(t, ok, "credentials not found")
+					cred := must.NotFail(actualUser.Get("credentials")).(*types.Document)
 
 					for _, typ := range tc.showCredentials {
 						switch typ {
 						case "PLAIN":
-							assertPlainCredentials(t, "PLAIN", cred.(*types.Document))
+							assertPlainCredentials(t, "PLAIN", cred)
 						case "SCRAM-SHA-1":
-							assertSCRAMSHA1Credentials(t, "SCRAM-SHA-1", cred.(*types.Document))
+							assertSCRAMSHA1Credentials(t, "SCRAM-SHA-1", cred)
 						case "SCRAM-SHA-256":
-							assertSCRAMSHA256Credentials(t, "SCRAM-SHA-256", cred.(*types.Document))
+							assertSCRAMSHA256Credentials(t, "SCRAM-SHA-256", cred)
 						}
 					}
-				} else {
-					assert.False(t, actualUser.Has("credentials"))
 				}
 
-				if payload.Has("mechanisms") {
-					assertUsersinfoMechanisms(t, payload, actualUser)
+				if len(tc.showCredentials) == 0 {
+					assert.False(t, actualUser.Has("credentials"))
 				}
 
 				foundUsers[must.NotFail(actualUser.Get("_id")).(string)] = struct{}{}
@@ -623,19 +617,3 @@ func TestUsersinfo(t *testing.T) {
 		})
 	}
 }
-
-func assertUsersinfoMechanisms(t *testing.T, payload, user *types.Document) {
-	payloadMechanisms := must.NotFail(payload.Get("mechanisms")).(*types.Array)
-
-	if payloadMechanisms.Contains("PLAIN") {
-		assertPlainCredentials(t, "PLAIN", must.NotFail(user.Get("credentials")).(*types.Document))
-	}
-
-	if payloadMechanisms.Contains("SCRAM-SHA-1") {
-		assertSCRAMSHA1Credentials(t, "SCRAM-SHA-1", must.NotFail(user.Get("credentials")).(*types.Document))
-	}
-
-	if payloadMechanisms.Contains("SCRAM-SHA-256") {
-		assertSCRAMSHA256Credentials(t, "SCRAM-SHA-256", must.NotFail(user.Get("credentials")).(*types.Document))
-	}
-}
diff --git a/internal/handler/handlerparams/typecode_test.go b/internal/handler/handlerparams/typecode_test.go
@@ -51,7 +51,6 @@ func TestHasSameTypeElements(t *testing.T) {
 			same:  true,
 		},
 	} {
-		tc, name := tc, name
 		t.Run(name, func(t *testing.T) {
 			t.Parallel()