Skip to content

Commit 94f2bc3

Browse files
jrmccannonjrmccannon
authored
[PM-25056] - Deadlock testing fix (#7478)
* This wraps the delete method in a retry loop in order to protect the delete calls when cleaning up test data. * Removing database test clean up as these databases should be ephemeral
1 parent 327290c commit 94f2bc3

13 files changed

Lines changed: 3 additions & 220 deletions

‎test/Infrastructure.IntegrationTest/AdminConsole/OrganizationTestHelpers.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
using Bit.Core.Entities;
55
using Bit.Core.Enums;
66
using Bit.Core.Repositories;
7-
7+
using Bit.Core.Utilities;
88
namespace Bit.Infrastructure.IntegrationTest.AdminConsole;
99

1010
/// <summary>
@@ -16,7 +16,7 @@ public static class OrganizationTestHelpers
1616
{
1717
public static Task<User> CreateTestUserAsync(this IUserRepository userRepository, string identifier = "test")
1818
{
19-
var id = Guid.NewGuid();
19+
var id = CoreHelpers.GenerateComb();
2020
return userRepository.CreateAsync(new User
2121
{
2222
Id = id,
@@ -34,7 +34,7 @@ public static Task<Organization> CreateTestOrganizationAsync(this IOrganizationR
3434
int? seatCount = null,
3535
string identifier = "test")
3636
{
37-
var id = Guid.NewGuid();
37+
var id = CoreHelpers.GenerateComb();
3838
return organizationRepository.CreateAsync(new Organization
3939
{
4040
Name = $"{identifier}-{id}",

‎test/Infrastructure.IntegrationTest/AdminConsole/Repositories/CollectionRepository/CollectionRepositoryCreateTests.cs‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -61,13 +61,6 @@ await collectionRepository.CreateAsync(collection,
6161
Assert.Equal(2, users.Length);
6262
Assert.Single(users, u => u.Id == orgUser1.Id && u.Manage && !u.HidePasswords && u.ReadOnly);
6363
Assert.Single(users, u => u.Id == orgUser2.Id && !u.Manage && u.HidePasswords && !u.ReadOnly);
64-
65-
// Clean up data
66-
await userRepository.DeleteAsync(user1);
67-
await userRepository.DeleteAsync(user2);
68-
await organizationRepository.DeleteAsync(organization);
69-
await groupRepository.DeleteManyAsync([group1.Id, group2.Id]);
70-
await organizationUserRepository.DeleteManyAsync([orgUser1.Id, orgUser2.Id]);
7164
}
7265

7366
/// <remarks>
@@ -98,8 +91,5 @@ public async Task CreateAsync_WithNoAccess_Works(
9891

9992
Assert.Empty(actualAccess.Groups);
10093
Assert.Empty(actualAccess.Users);
101-
102-
// Clean up
103-
await organizationRepository.DeleteAsync(organization);
10494
}
10595
}

‎test/Infrastructure.IntegrationTest/AdminConsole/Repositories/CollectionRepository/CollectionRepositoryReplaceTests.cs‎

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -87,12 +87,6 @@ await collectionRepository.ReplaceAsync(collection,
8787
Assert.Equal(2, users.Length);
8888
Assert.Single(users, u => u.Id == orgUser2.Id && !u.Manage && !u.HidePasswords && u.ReadOnly);
8989
Assert.Single(users, u => u.Id == orgUser3.Id && u.Manage && !u.HidePasswords && u.ReadOnly);
90-
91-
// Clean up data
92-
await userRepository.DeleteAsync(user1);
93-
await userRepository.DeleteAsync(user2);
94-
await userRepository.DeleteAsync(user3);
95-
await organizationRepository.DeleteAsync(organization);
9690
}
9791

9892
/// <remarks>
@@ -141,10 +135,6 @@ await collectionRepository.CreateAsync(collection,
141135

142136
Assert.Empty(actualAccess.Groups);
143137
Assert.Empty(actualAccess.Users);
144-
145-
// Clean up
146-
await userRepository.DeleteAsync(user);
147-
await organizationRepository.DeleteAsync(organization);
148138
}
149139

150140
[Theory, DatabaseData]
@@ -205,10 +195,5 @@ await collectionRepository.CreateAsync(collection,
205195
Assert.Equal(2, users.Length);
206196
Assert.Single(users, u => u.Id == orgUser1.Id && u.Manage && !u.HidePasswords && u.ReadOnly);
207197
Assert.Single(users, u => u.Id == orgUser2.Id && !u.Manage && u.HidePasswords && !u.ReadOnly);
208-
209-
// Clean up data
210-
await userRepository.DeleteAsync(user1);
211-
await userRepository.DeleteAsync(user2);
212-
await organizationRepository.DeleteAsync(organization);
213198
}
214199
}

‎test/Infrastructure.IntegrationTest/AdminConsole/Repositories/CollectionRepository/CreateDefaultCollectionsSharedTests.cs‎

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,6 @@ public static async Task CreatesDefaultCollections_Success(
3535

3636
// Assert
3737
await AssertAllUsersHaveOneDefaultCollectionAsync(collectionRepository, resultOrganizationUsers, organization.Id);
38-
39-
await CleanupAsync(organizationRepository, userRepository, organization, resultOrganizationUsers);
4038
}
4139

4240
public static async Task CreatesForNewUsersOnly_AndIgnoresExistingUsers(
@@ -72,8 +70,6 @@ await CreateUserForOrgAsync(userRepository, organizationUserRepository, organiza
7270

7371
// Assert
7472
await AssertAllUsersHaveOneDefaultCollectionAsync(collectionRepository, affectedOrgUsers, organization.Id);
75-
76-
await CleanupAsync(organizationRepository, userRepository, organization, affectedOrgUsers);
7773
}
7874

7975
public static async Task IgnoresAllExistingUsers(
@@ -101,8 +97,6 @@ public static async Task IgnoresAllExistingUsers(
10197

10298
// Assert - Original collections should remain unchanged, still only one per user
10399
await AssertAllUsersHaveOneDefaultCollectionAsync(collectionRepository, resultOrganizationUsers, organization.Id);
104-
105-
await CleanupAsync(organizationRepository, userRepository, organization, resultOrganizationUsers);
106100
}
107101

108102
private static async Task CreateUsersWithExistingDefaultCollectionsAsync(
@@ -141,18 +135,4 @@ private static async Task<OrganizationUser> CreateUserForOrgAsync(IUserRepositor
141135

142136
return orgUser;
143137
}
144-
145-
private static async Task CleanupAsync(IOrganizationRepository organizationRepository,
146-
IUserRepository userRepository,
147-
Organization organization,
148-
IEnumerable<OrganizationUser> organizationUsers)
149-
{
150-
await organizationRepository.DeleteAsync(organization);
151-
152-
await userRepository.DeleteManyAsync(
153-
organizationUsers
154-
.Where(organizationUser => organizationUser.UserId != null)
155-
.Select(organizationUser => new User() { Id = organizationUser.UserId.Value })
156-
);
157-
}
158138
}

‎test/Infrastructure.IntegrationTest/AdminConsole/Repositories/OrganizationRepositoryTests.cs‎

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,6 @@ public async Task GetManyByIdsAsync_ExistingOrganizations_ReturnsOrganizations(I
3636
Assert.Equal(2, result.Count);
3737
Assert.Contains(result, org => org.Id == organization1.Id);
3838
Assert.Contains(result, org => org.Id == organization2.Id);
39-
40-
// Clean up
41-
await organizationRepository.DeleteAsync(organization1);
42-
await organizationRepository.DeleteAsync(organization2);
4339
}
4440

4541
[Theory, DatabaseData]
@@ -212,9 +208,6 @@ public async Task IncrementSeatCountAsync_GivenOrganizationHasNotChangedSeatCoun
212208
Assert.Equal(organization.Id, updateResult.Id);
213209
Assert.True(updateResult.SyncSeats);
214210
Assert.Equal(requestDate.ToString("yyyy-MM-dd HH:mm:ss"), updateResult.RevisionDate.ToString("yyyy-MM-dd HH:mm:ss"));
215-
216-
// Annul
217-
await sutRepository.DeleteAsync(organization);
218211
}
219212

220213
[DatabaseData, Theory]
@@ -237,9 +230,6 @@ public async Task IncrementSeatCountAsync_GivenOrganizationHasChangedSeatCountBe
237230
Assert.Equal(organization.Id, updateResult.Id);
238231
Assert.True(updateResult.SyncSeats);
239232
Assert.Equal(requestDate.ToString("yyyy-MM-dd HH:mm:ss"), updateResult.RevisionDate.ToString("yyyy-MM-dd HH:mm:ss"));
240-
241-
// Annul
242-
await sutRepository.DeleteAsync(organization);
243233
}
244234

245235
[DatabaseData, Theory]
@@ -260,9 +250,6 @@ public async Task GetOrganizationsForSubscriptionSyncAsync_GivenOrganizationHasC
260250
Assert.Equal(organization.Id, updateResult.Id);
261251
Assert.True(updateResult.SyncSeats);
262252
Assert.Equal(requestDate.ToString("yyyy-MM-dd HH:mm:ss"), updateResult.RevisionDate.ToString("yyyy-MM-dd HH:mm:ss"));
263-
264-
// Annul
265-
await sutRepository.DeleteAsync(organization);
266253
}
267254

268255
[DatabaseData, Theory]
@@ -281,9 +268,6 @@ public async Task UpdateSuccessfulOrganizationSyncStatusAsync_GivenOrganizationH
281268
// Assert
282269
var result = (await sutRepository.GetOrganizationsForSubscriptionSyncAsync()).ToArray();
283270
Assert.Null(result.FirstOrDefault(x => x.Id == organization.Id));
284-
285-
// Annul
286-
await sutRepository.DeleteAsync(organization);
287271
}
288272

289273
[DatabaseTheory, DatabaseData]
@@ -400,9 +384,6 @@ public async Task GetAbilityAsync_WithExistingOrganization_ReturnsCorrectAbility
400384
Assert.Equal(organization.UseDisableSmAdsForUsers, result.UseDisableSmAdsForUsers);
401385
Assert.Equal(organization.UsePhishingBlocker, result.UsePhishingBlocker);
402386
Assert.Equal(organization.UseMyItems, result.UseMyItems);
403-
404-
// Clean up
405-
await organizationRepository.DeleteAsync(organization);
406387
}
407388

408389
[Theory, DatabaseData]

‎test/Infrastructure.IntegrationTest/AdminConsole/Repositories/OrganizationUserRepository/GetManyConfirmedAcceptedDetailsByUserAsyncTests.cs‎

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,6 @@ public async Task ReturnsDetails_WhenUserIsConfirmed(
2626
Assert.Equal(organization.Id, result.OrganizationId);
2727
Assert.Equal(user.Id, result.UserId);
2828
Assert.Equal(OrganizationUserStatusType.Confirmed, result.Status);
29-
30-
// Annul
31-
await organizationRepository.DeleteAsync(organization);
32-
await userRepository.DeleteAsync(user);
3329
}
3430

3531
[Theory, DatabaseData]
@@ -52,10 +48,6 @@ public async Task ReturnsDetails_WhenUserIsAccepted(
5248
Assert.Equal(organization.Id, result.OrganizationId);
5349
Assert.Equal(user.Id, result.UserId);
5450
Assert.Equal(OrganizationUserStatusType.Accepted, result.Status);
55-
56-
// Annul
57-
await organizationRepository.DeleteAsync(organization);
58-
await userRepository.DeleteAsync(user);
5951
}
6052

6153
[Theory, DatabaseData]
@@ -80,11 +72,6 @@ public async Task ReturnsDetailsAcrossMultipleOrganizations_WhenUserIsConfirmedO
8072
Assert.Equal(2, results.Count);
8173
Assert.Contains(results, r => r.OrganizationId == confirmedOrg.Id && r.Status == OrganizationUserStatusType.Confirmed);
8274
Assert.Contains(results, r => r.OrganizationId == acceptedOrg.Id && r.Status == OrganizationUserStatusType.Accepted);
83-
84-
// Annul
85-
await organizationRepository.DeleteAsync(confirmedOrg);
86-
await organizationRepository.DeleteAsync(acceptedOrg);
87-
await userRepository.DeleteAsync(user);
8875
}
8976

9077
[Theory, DatabaseData]
@@ -103,10 +90,6 @@ public async Task DoesNotReturnDetails_WhenUserIsInvited(
10390

10491
// Assert
10592
Assert.DoesNotContain(results, r => r.OrganizationId == organization.Id);
106-
107-
// Annul
108-
await organizationRepository.DeleteAsync(organization);
109-
await userRepository.DeleteAsync(user);
11093
}
11194

11295
[Theory, DatabaseData]
@@ -125,10 +108,6 @@ public async Task DoesNotReturnDetails_WhenUserIsRevoked(
125108

126109
// Assert
127110
Assert.DoesNotContain(results, r => r.OrganizationId == organization.Id);
128-
129-
// Annul
130-
await organizationRepository.DeleteAsync(organization);
131-
await userRepository.DeleteAsync(user);
132111
}
133112

134113
[Theory, DatabaseData]
@@ -149,9 +128,5 @@ public async Task DoesNotReturnDetails_ForOtherUsers(
149128

150129
// Assert
151130
Assert.DoesNotContain(results, r => r.OrganizationId == organization.Id);
152-
153-
// Annul
154-
await organizationRepository.DeleteAsync(organization);
155-
await userRepository.DeleteManyAsync([targetUser, otherUser]);
156131
}
157132
}

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

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1397,10 +1397,6 @@ public async Task ConfirmOrganizationUserAsync_WhenUserIsAccepted_ReturnsTrue(IO
13971397
Assert.NotNull(updatedUser);
13981398
Assert.Equal(OrganizationUserStatusType.Confirmed, updatedUser.Status);
13991399
Assert.Equal(key, updatedUser.Key);
1400-
1401-
// Annul
1402-
await organizationRepository.DeleteAsync(organization);
1403-
await userRepository.DeleteAsync(user);
14041400
}
14051401

14061402
[Theory, DatabaseData]
@@ -1430,10 +1426,6 @@ public async Task ConfirmOrganizationUserAsync_WhenUserIsAlreadyConfirmed_Return
14301426
var unchangedUser = await organizationUserRepository.GetByIdAsync(orgUser.Id);
14311427
Assert.NotNull(unchangedUser);
14321428
Assert.Equal(OrganizationUserStatusType.Confirmed, unchangedUser.Status);
1433-
1434-
// Annul
1435-
await organizationRepository.DeleteAsync(organization);
1436-
await userRepository.DeleteAsync(user);
14371429
}
14381430

14391431
[Theory, DatabaseData]
@@ -1464,10 +1456,6 @@ public async Task ConfirmOrganizationUserAsync_IsIdempotent_WhenCalledMultipleTi
14641456
var finalUser = await organizationUserRepository.GetByIdAsync(orgUser.Id);
14651457
Assert.NotNull(finalUser);
14661458
Assert.Equal(OrganizationUserStatusType.Confirmed, finalUser.Status);
1467-
1468-
// Annul
1469-
await organizationRepository.DeleteAsync(organization);
1470-
await userRepository.DeleteAsync(user);
14711459
}
14721460

14731461
[Theory, DatabaseData]

‎test/Infrastructure.IntegrationTest/AdminConsole/Repositories/PolicyRepository/GetManyConfirmedAcceptedByUserIdAsyncTests.cs‎

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,6 @@ public async Task ReturnsPolicies_WhenUserIsConfirmed(
3333

3434
// Assert
3535
Assert.Contains(results, p => p.Id == policy.Id);
36-
37-
// Annul
38-
await organizationRepository.DeleteAsync(organization);
39-
await userRepository.DeleteAsync(user);
4036
}
4137

4238
[Theory, DatabaseData]
@@ -62,10 +58,6 @@ public async Task ReturnsPolicies_WhenUserIsAccepted(
6258

6359
// Assert
6460
Assert.Contains(results, p => p.Id == policy.Id);
65-
66-
// Annul
67-
await organizationRepository.DeleteAsync(organization);
68-
await userRepository.DeleteAsync(user);
6961
}
7062

7163
[Theory, DatabaseData]
@@ -102,11 +94,6 @@ public async Task ReturnsPoliciesAcrossMultipleOrganizations_WhenUserIsConfirmed
10294
// Assert
10395
Assert.Contains(results, p => p.Id == confirmedPolicy.Id);
10496
Assert.Contains(results, p => p.Id == acceptedPolicy.Id);
105-
106-
// Annul
107-
await organizationRepository.DeleteAsync(confirmedOrg);
108-
await organizationRepository.DeleteAsync(acceptedOrg);
109-
await userRepository.DeleteAsync(user);
11097
}
11198

11299
[Theory, DatabaseData]
@@ -139,10 +126,6 @@ await organizationUserRepository.CreateAsync(new OrganizationUser
139126

140127
// Assert
141128
Assert.DoesNotContain(results, p => p.Id == policy.Id);
142-
143-
// Annul
144-
await organizationRepository.DeleteAsync(organization);
145-
await userRepository.DeleteAsync(user);
146129
}
147130

148131
[Theory, DatabaseData]
@@ -168,10 +151,6 @@ public async Task DoesNotReturnPolicies_WhenUserIsRevoked(
168151

169152
// Assert
170153
Assert.DoesNotContain(results, p => p.Id == policy.Id);
171-
172-
// Annul
173-
await organizationRepository.DeleteAsync(organization);
174-
await userRepository.DeleteAsync(user);
175154
}
176155

177156
[Theory, DatabaseData]
@@ -199,9 +178,5 @@ public async Task DoesNotReturnPolicies_ForOtherUsers(
199178

200179
// Assert
201180
Assert.DoesNotContain(results, p => p.Id == policy.Id);
202-
203-
// Annul
204-
await organizationRepository.DeleteAsync(organization);
205-
await userRepository.DeleteManyAsync([targetUser, otherUser]);
206181
}
207182
}

0 commit comments

Comments
 (0)