Fix Most Test Warnings (#4612)

* Add Collections Tests

* Update CollectionRepository Implementation

* Test Adding And Deleting Through Replace

* Format

* Fix Most Test Warnings

* Format

perf/MicroBenchmarks/Identity/IdentityServer/StaticClientStoreTests.cs

@@ -15,7 +15,7 @@ public class StaticClientStoreTests
     }
 
     [Params("mobile", "connector", "invalid", "a_much_longer_invalid_value_that_i_am_making_up", "WEB", "")]
-    public string? ClientId { get; set; }
+    public string ClientId { get; set; } = null!;
 
     [Benchmark]
     public Client? TryGetValue()

test/Api.IntegrationTest/AdminConsole/Public/Controllers/MembersControllerTests.cs

@@ -21,8 +21,10 @@ public class MembersControllerTests : IClassFixture<ApiApplicationFactory>, IAsy
     private readonly HttpClient _client;
     private readonly ApiApplicationFactory _factory;
     private readonly LoginHelper _loginHelper;
-    private Organization _organization;
-    private string _ownerEmail;
+
+    // These will get set in `InitializeAsync` which is ran before all tests
+    private Organization _organization = null!;
+    private string _ownerEmail = null!;
 
     public MembersControllerTests(ApiApplicationFactory factory)
     {
@@ -74,7 +76,7 @@ public class MembersControllerTests : IClassFixture<ApiApplicationFactory>, IAsy
             m.Email == _ownerEmail && m.Type == OrganizationUserType.Owner));
 
         // The custom user
-        var user1Result = result.Data.SingleOrDefault(m => m.Email == userEmail1);
+        var user1Result = result.Data.Single(m => m.Email == userEmail1);
         Assert.Equal(OrganizationUserType.Custom, user1Result.Type);
         AssertHelper.AssertPropertyEqual(
             new PermissionsModel { AccessImportExport = true, ManagePolicies = true, AccessReports = true },
@@ -156,6 +158,7 @@ public class MembersControllerTests : IClassFixture<ApiApplicationFactory>, IAsy
         var organizationUserRepository = _factory.GetService<IOrganizationUserRepository>();
         var orgUser = await organizationUserRepository.GetByIdAsync(result.Id);
 
+        Assert.NotNull(orgUser);
         Assert.Equal(email, orgUser.Email);
         Assert.Equal(OrganizationUserType.Custom, orgUser.Type);
         Assert.Equal("myCustomUser", orgUser.ExternalId);
@@ -201,6 +204,7 @@ public class MembersControllerTests : IClassFixture<ApiApplicationFactory>, IAsy
         var organizationUserRepository = _factory.GetService<IOrganizationUserRepository>();
         var updatedOrgUser = await organizationUserRepository.GetByIdAsync(result.Id);
 
+        Assert.NotNull(updatedOrgUser);
         Assert.Equal(OrganizationUserType.Custom, updatedOrgUser.Type);
         Assert.Equal("example", updatedOrgUser.ExternalId);
         Assert.Equal(OrganizationUserStatusType.Confirmed, updatedOrgUser.Status);
@@ -240,6 +244,7 @@ public class MembersControllerTests : IClassFixture<ApiApplicationFactory>, IAsy
         var organizationUserRepository = _factory.GetService<IOrganizationUserRepository>();
         var updatedOrgUser = await organizationUserRepository.GetByIdAsync(result.Id);
 
+        Assert.NotNull(updatedOrgUser);
         Assert.Equal(OrganizationUserType.Custom, updatedOrgUser.Type);
         AssertHelper.AssertPropertyEqual(
             new Permissions { CreateNewCollections = true, ManageScim = true, ManageGroups = true, ManageUsers = true },

test/Api.IntegrationTest/Helpers/OrganizationTestHelpers.cs

@@ -1,4 +1,5 @@
-using Bit.Api.IntegrationTest.Factories;
+using System.Diagnostics;
+using Bit.Api.IntegrationTest.Factories;
 using Bit.Core.AdminConsole.Entities;
 using Bit.Core.Billing.Enums;
 using Bit.Core.Entities;
@@ -38,6 +39,8 @@ public static class OrganizationTestHelpers
             PaymentMethodType = paymentMethod
         });
 
+        Debug.Assert(signUpResult.organizationUser is not null);
+
         return new Tuple<Organization, OrganizationUser>(signUpResult.organization, signUpResult.organizationUser);
     }
 
@@ -57,6 +60,7 @@ public static class OrganizationTestHelpers
         var organizationUserRepository = factory.GetService<IOrganizationUserRepository>();
 
         var user = await userRepository.GetByEmailAsync(userEmail);
+        Debug.Assert(user is not null);
 
         var orgUser = new OrganizationUser
         {

test/Api.IntegrationTest/SecretsManager/Helpers/SecretsManagerOrganizationHelper.cs

@@ -1,4 +1,5 @@
-using Bit.Api.IntegrationTest.Factories;
+using System.Diagnostics;
+using Bit.Api.IntegrationTest.Factories;
 using Bit.Api.IntegrationTest.Helpers;
 using Bit.Core.AdminConsole.Entities;
 using Bit.Core.Entities;
@@ -35,7 +36,8 @@ public class SecretsManagerOrganizationHelper
 
     public async Task<(Organization organization, OrganizationUser owner)> Initialize(bool useSecrets, bool ownerAccessSecrets, bool organizationEnabled)
     {
-        (_organization, _owner) = await OrganizationTestHelpers.SignUpAsync(_factory, ownerEmail: _ownerEmail, billingEmail: _ownerEmail);
+        (_organization, _owner!) = await OrganizationTestHelpers.SignUpAsync(_factory, ownerEmail: _ownerEmail, billingEmail: _ownerEmail);
+        Debug.Assert(_owner is not null);
 
         if (useSecrets || !organizationEnabled)
         {

test/Api.Test/AdminConsole/Controllers/GroupsControllerPutTests.cs

@@ -20,6 +20,8 @@ using Microsoft.AspNetCore.Authorization;
 using NSubstitute;
 using Xunit;
 
+#nullable enable
+
 namespace Bit.Api.Test.AdminConsole.Controllers;
 
 [ControllerCustomize(typeof(GroupsController))]
@@ -305,7 +307,7 @@ public class GroupsControllerPutTests
             .Returns(new Tuple<Group, ICollection<CollectionAccessSelection>>(group, currentCollectionAccess ?? []));
         if (savingUser != null)
         {
-            sutProvider.GetDependency<IOrganizationUserRepository>().GetByOrganizationAsync(orgId, savingUser.UserId.Value)
+            sutProvider.GetDependency<IOrganizationUserRepository>().GetByOrganizationAsync(orgId, savingUser.UserId!.Value)
                 .Returns(savingUser);
         }
 

test/Api.Test/Vault/Controllers/SyncControllerTests.cs

@@ -129,7 +129,7 @@ public class SyncControllerTests
         // Asserts
         // Assert that methods are called
         var hasEnabledOrgs = organizationUserDetails.Any(o => o.Enabled);
-        this.AssertMethodsCalledAsync(userService, organizationUserRepository, providerUserRepository, folderRepository,
+        await this.AssertMethodsCalledAsync(userService, organizationUserRepository, providerUserRepository, folderRepository,
             cipherRepository, sendRepository, collectionRepository, collectionCipherRepository, hasEnabledOrgs);
 
         Assert.IsType<SyncResponseModel>(result);
@@ -216,7 +216,7 @@ public class SyncControllerTests
         // Assert that methods are called
 
         var hasEnabledOrgs = organizationUserDetails.Any(o => o.Enabled);
-        this.AssertMethodsCalledAsync(userService, organizationUserRepository, providerUserRepository, folderRepository,
+        await this.AssertMethodsCalledAsync(userService, organizationUserRepository, providerUserRepository, folderRepository,
             cipherRepository, sendRepository, collectionRepository, collectionCipherRepository, hasEnabledOrgs);
 
         Assert.IsType<SyncResponseModel>(result);
@@ -293,7 +293,7 @@ public class SyncControllerTests
         // Assert that methods are called
 
         var hasEnabledOrgs = organizationUserDetails.Any(o => o.Enabled);
-        this.AssertMethodsCalledAsync(userService, organizationUserRepository, providerUserRepository, folderRepository,
+        await this.AssertMethodsCalledAsync(userService, organizationUserRepository, providerUserRepository, folderRepository,
             cipherRepository, sendRepository, collectionRepository, collectionCipherRepository, hasEnabledOrgs);
 
         Assert.IsType<SyncResponseModel>(result);

test/Core.Test/AdminConsole/Services/OrganizationServiceTests.cs

@@ -1115,7 +1115,7 @@ OrganizationUserInvite invite, SutProvider<OrganizationService> sutProvider)
 
         await sutProvider.Sut.InviteUsersAsync(organization.Id, savingUser.Id, systemUser: null, invites);
 
-        sutProvider.GetDependency<IUpdateSecretsManagerSubscriptionCommand>().Received(1)
+        await sutProvider.GetDependency<IUpdateSecretsManagerSubscriptionCommand>().Received(1)
             .UpdateSubscriptionAsync(Arg.Is<SecretsManagerSubscriptionUpdate>(update =>
                 update.SmSeats == organization.SmSeats + invitedSmUsers &&
                 !update.SmServiceAccountsChanged &&
@@ -1158,7 +1158,7 @@ OrganizationUserInvite invite, SutProvider<OrganizationService> sutProvider)
         // OrgUser is reverted
         // Note: we don't know what their guids are so comparing length is the best we can do
         var invitedEmails = invites.SelectMany(i => i.invite.Emails);
-        sutProvider.GetDependency<IOrganizationUserRepository>().Received(1).DeleteManyAsync(
+        await sutProvider.GetDependency<IOrganizationUserRepository>().Received(1).DeleteManyAsync(
             Arg.Is<IEnumerable<Guid>>(ids => ids.Count() == invitedEmails.Count()));
 
         Received.InOrder(() =>

test/Core.Test/OrganizationFeatures/OrganizationSubscriptionUpdate/AddSecretsManagerSubscriptionCommandTests.cs

@@ -57,7 +57,7 @@ public class AddSecretsManagerSubscriptionCommandTests
 
         // TODO: call ReferenceEventService - see AC-1481
 
-        sutProvider.GetDependency<IOrganizationService>().Received(1).ReplaceAndUpdateCacheAsync(Arg.Is<Organization>(c =>
+        await sutProvider.GetDependency<IOrganizationService>().Received(1).ReplaceAndUpdateCacheAsync(Arg.Is<Organization>(c =>
             c.SmSeats == plan.SecretsManager.BaseSeats + additionalSmSeats &&
             c.SmServiceAccounts == plan.SecretsManager.BaseServiceAccount + additionalServiceAccounts &&
             c.UseSecretsManager == true));

test/Core.Test/OrganizationFeatures/OrganizationSubscriptionUpdate/UpdateSecretsManagerSubscriptionCommandTests.cs

@@ -663,8 +663,8 @@ public class UpdateSecretsManagerSubscriptionCommandTests
             .SendOrganizationMaxSeatLimitReachedEmailAsync(Arg.Any<Organization>(), Arg.Any<int>(),
                 Arg.Any<IEnumerable<string>>());
 
-        sutProvider.GetDependency<IOrganizationRepository>().DidNotReceiveWithAnyArgs().ReplaceAsync(default);
-        sutProvider.GetDependency<IApplicationCacheService>().DidNotReceiveWithAnyArgs().UpsertOrganizationAbilityAsync(default);
+        await sutProvider.GetDependency<IOrganizationRepository>().DidNotReceiveWithAnyArgs().ReplaceAsync(default);
+        await sutProvider.GetDependency<IApplicationCacheService>().DidNotReceiveWithAnyArgs().UpsertOrganizationAbilityAsync(default);
     }
 
     private void AssertUpdatedOrganization(Func<Organization> organizationMatcher, SutProvider<UpdateSecretsManagerSubscriptionCommand> sutProvider)

test/Core.Test/OrganizationFeatures/OrganizationSubscriptionUpdate/UpgradeOrganizationPlanCommandTests.cs

@@ -165,7 +165,7 @@ public class UpgradeOrganizationPlanCommandTests
         var exception = await Assert.ThrowsAsync<BadRequestException>(() => sutProvider.Sut.UpgradePlanAsync(organization.Id, upgrade));
         Assert.Contains("Your organization currently has 2 Secrets Manager seats filled. Your new plan only has", exception.Message);
 
-        sutProvider.GetDependency<IOrganizationService>().DidNotReceiveWithAnyArgs().ReplaceAndUpdateCacheAsync(default);
+        await sutProvider.GetDependency<IOrganizationService>().DidNotReceiveWithAnyArgs().ReplaceAndUpdateCacheAsync(default);
     }
 
     [Theory, FreeOrganizationUpgradeCustomize]
@@ -194,6 +194,6 @@ public class UpgradeOrganizationPlanCommandTests
         var exception = await Assert.ThrowsAsync<BadRequestException>(() => sutProvider.Sut.UpgradePlanAsync(organization.Id, upgrade));
         Assert.Contains($"Your organization currently has {currentServiceAccounts} machine accounts. Your new plan only allows", exception.Message);
 
-        sutProvider.GetDependency<IOrganizationService>().DidNotReceiveWithAnyArgs().ReplaceAndUpdateCacheAsync(default);
+        await sutProvider.GetDependency<IOrganizationService>().DidNotReceiveWithAnyArgs().ReplaceAndUpdateCacheAsync(default);
     }
 }

test/Core.Test/Utilities/BulkAuthorizationHandlerTests.cs

@@ -62,11 +62,12 @@ public class BulkAuthorizationHandlerTests
 
     private class TestBulkAuthorizationHandler : BulkAuthorizationHandler<TestOperationRequirement, TestResource>
     {
-        protected override async Task HandleRequirementAsync(AuthorizationHandlerContext context,
+        protected override Task HandleRequirementAsync(AuthorizationHandlerContext context,
             TestOperationRequirement requirement,
             ICollection<TestResource> resources)
         {
             context.Succeed(requirement);
+            return Task.CompletedTask;
         }
     }
 }

test/Identity.Test/Controllers/AccountsControllerTests.cs

@@ -86,7 +86,7 @@ public class AccountsControllerTests : IDisposable
             Kdf = KdfType.PBKDF2_SHA256,
             KdfIterations = AuthConstants.PBKDF2_ITERATIONS.Default
         };
-        _userRepository.GetKdfInformationByEmailAsync(Arg.Any<string>()).Returns(Task.FromResult(userKdfInfo));
+        _userRepository.GetKdfInformationByEmailAsync(Arg.Any<string>()).Returns(userKdfInfo);
 
         var response = await _sut.PostPrelogin(new PreloginRequestModel { Email = "user@example.com" });
 
@@ -97,7 +97,7 @@ public class AccountsControllerTests : IDisposable
     [Fact]
     public async Task PostPrelogin_WhenUserDoesNotExist_ShouldDefaultToPBKDF()
     {
-        _userRepository.GetKdfInformationByEmailAsync(Arg.Any<string>()).Returns(Task.FromResult<UserKdfInformation>(null!));
+        _userRepository.GetKdfInformationByEmailAsync(Arg.Any<string>()).Returns(Task.FromResult<UserKdfInformation?>(null));
 
         var response = await _sut.PostPrelogin(new PreloginRequestModel { Email = "user@example.com" });
 

test/Infrastructure.IntegrationTest/AdminConsole/Repositories/OrganizationUserRepositoryTests.cs

@@ -38,6 +38,7 @@ public class OrganizationUserRepositoryTests
         await organizationUserRepository.DeleteAsync(orgUser);
 
         var newUser = await userRepository.GetByIdAsync(user.Id);
+        Assert.NotNull(newUser);
         Assert.NotEqual(newUser.AccountRevisionDate, user.AccountRevisionDate);
     }
 
@@ -90,7 +91,9 @@ public class OrganizationUserRepositoryTests
         });
 
         var updatedUser1 = await userRepository.GetByIdAsync(user1.Id);
+        Assert.NotNull(updatedUser1);
         var updatedUser2 = await userRepository.GetByIdAsync(user2.Id);
+        Assert.NotNull(updatedUser2);
 
         Assert.NotEqual(updatedUser1.AccountRevisionDate, user1.AccountRevisionDate);
         Assert.NotEqual(updatedUser2.AccountRevisionDate, user2.AccountRevisionDate);

test/Infrastructure.IntegrationTest/Auth/Repositories/AuthRequestRepositoryTests.cs

@@ -165,12 +165,15 @@ public class AuthRequestRepositoryTests
 
         // Assert that the unchanged auth request is still unchanged
         var skippedAuthRequest = await authRequestRepository.GetByIdAsync(authRequestNotToBeUpdated.Id);
+        Assert.NotNull(skippedAuthRequest);
         Assert.True(AuthRequestEquals(skippedAuthRequest, authRequestNotToBeUpdated));
 
         // Assert that the values updated on the changed auth requests were updated, and no others
         var updatedAuthRequest1 = await authRequestRepository.GetByIdAsync(authRequestToBeUpdated1.Id);
+        Assert.NotNull(updatedAuthRequest1);
         Assert.True(AuthRequestEquals(authRequestToBeUpdated1, updatedAuthRequest1));
         var updatedAuthRequest2 = await authRequestRepository.GetByIdAsync(authRequestToBeUpdated2.Id);
+        Assert.NotNull(updatedAuthRequest2);
         Assert.True(AuthRequestEquals(authRequestToBeUpdated2, updatedAuthRequest2));
 
         // Assert that the auth request we never created is not created by

test/Infrastructure.IntegrationTest/Auth/Repositories/EmergencyAccessRepositoryTests.cs

@@ -39,6 +39,7 @@ public class EmergencyAccessRepositoriesTests
 
         var updatedGrantee = await userRepository.GetByIdAsync(granteeUser.Id);
 
+        Assert.NotNull(updatedGrantee);
         Assert.NotEqual(updatedGrantee.AccountRevisionDate, granteeUser.AccountRevisionDate);
     }
 }

test/Infrastructure.IntegrationTest/ConfigurationExtensions.cs

@@ -21,9 +21,9 @@ public static class ConfigurationExtensions
     public static Database[] GetDatabases(this IConfiguration config)
     {
         var typedConfig = config.Get<TypedConfig>();
-        if (typedConfig.Databases == null)
+        if (typedConfig?.Databases == null)
         {
-            return Array.Empty<Database>();
+            return [];
         }
 
         return typedConfig.Databases.Where(d => d.Enabled).ToArray();

test/Infrastructure.IntegrationTest/Tools/SendRepositoryTests.cs

@@ -27,6 +27,7 @@ public class SendRepositoryTests
         Assert.Equal(expirationDate, createdSend.ExpirationDate!.Value, LaxDateTimeComparer.Default);
 
         var sendFromDatabase = await sendRepository.GetByIdAsync(createdSend.Id);
+        Assert.NotNull(sendFromDatabase);
         Assert.Equal(expirationDate, sendFromDatabase.ExpirationDate!.Value, LaxDateTimeComparer.Default);
         Assert.Equal(SendType.Text, sendFromDatabase.Type);
         Assert.Equal(0, sendFromDatabase.AccessCount);

test/Infrastructure.IntegrationTest/Vault/Repositories/CipherRepositoryTests.cs

@@ -40,6 +40,7 @@ public class CipherRepositoryTests
 
         Assert.Null(deletedCipher);
         var updatedUser = await userRepository.GetByIdAsync(user.Id);
+        Assert.NotNull(updatedUser);
         Assert.NotEqual(updatedUser.AccountRevisionDate, user.AccountRevisionDate);
     }
 
@@ -61,6 +62,7 @@ public class CipherRepositoryTests
         });
 
         user = await userRepository.GetByIdAsync(user.Id);
+        Assert.NotNull(user);
 
         var organization = await organizationRepository.CreateAsync(new Organization
         {
@@ -110,6 +112,7 @@ public class CipherRepositoryTests
 
         var updatedUser = await userRepository.GetByIdAsync(user.Id);
 
+        Assert.NotNull(updatedUser);
         Assert.True(updatedUser.AccountRevisionDate - user.AccountRevisionDate > TimeSpan.Zero,
             "The AccountRevisionDate is expected to be changed");
 
@@ -135,6 +138,7 @@ public class CipherRepositoryTests
 
 
         user = await userRepository.GetByIdAsync(user.Id);
+        Assert.NotNull(user);
 
         // Create cipher in personal vault
         var createdCipher = await cipherRepository.CreateAsync(new Cipher
@@ -176,6 +180,7 @@ public class CipherRepositoryTests
 
         var updatedCipher = await cipherRepository.GetByIdAsync(createdCipher.Id);
 
+        Assert.NotNull(updatedCipher);
         Assert.Null(updatedCipher.UserId);
         Assert.Equal(organization.Id, updatedCipher.OrganizationId);
         Assert.NotNull(updatedCipher.Folders);