Skip to content

Commit 474fefd

Browse files
committed
Fix adversarial review findings in SignalR hub tests
- Replace generic Exception assertions with HttpRequestException + 401 check in auth tests (prevents false positives from config errors) - WaitForEventsAsync now throws TimeoutException when expected events do not arrive, making timeout failures immediately diagnosable - Use DateTimeOffset.UtcNow instead of DateTime.UtcNow for consistency - Add missing status code assertions on column/card creation in ClearEditingCard test - Fix XML doc to not claim boardMutation coverage that does not exist - Use await using for conn2 in DisconnectOne test to prevent resource leak on assertion failure
1 parent 304af98 commit 474fefd

File tree

2 files changed

+22
-8
lines changed

2 files changed

+22
-8
lines changed

backend/tests/Taskdeck.Api.Tests/BoardsHubIntegrationTests.cs

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ namespace Taskdeck.Api.Tests;
1313

1414
/// <summary>
1515
/// Integration tests for the BoardsHub SignalR hub covering presence lifecycle,
16-
/// authentication enforcement, board mutation event delivery, and edge cases.
16+
/// authentication enforcement, and edge cases.
1717
/// Uses WebApplicationFactory with the real SignalR pipeline (in-memory transport).
1818
/// </summary>
1919
public class BoardsHubIntegrationTests : IClassFixture<TestWebApplicationFactory>
@@ -35,7 +35,9 @@ public async Task Unauthenticated_Connection_ShouldFailToStart()
3535
await using var connection = SignalRTestHelper.CreateBoardsHubConnection(_factory, accessToken: null);
3636

3737
var act = () => connection.StartAsync();
38-
await act.Should().ThrowAsync<Exception>();
38+
// SignalR negotiate returns 401; the client wraps this in HttpRequestException
39+
await act.Should().ThrowAsync<HttpRequestException>()
40+
.Where(ex => ex.Message.Contains("401") || ex.StatusCode == HttpStatusCode.Unauthorized);
3941
}
4042

4143
[Fact]
@@ -56,7 +58,9 @@ public async Task InvalidToken_Connection_ShouldFailToStart()
5658
await using var connection = SignalRTestHelper.CreateBoardsHubConnection(_factory, "not-a-valid-jwt");
5759

5860
var act = () => connection.StartAsync();
59-
await act.Should().ThrowAsync<Exception>();
61+
// Invalid JWT causes 401 on the negotiate endpoint
62+
await act.Should().ThrowAsync<HttpRequestException>()
63+
.Where(ex => ex.Message.Contains("401") || ex.StatusCode == HttpStatusCode.Unauthorized);
6064
}
6165

6266
// ────────────────────────────────────────────────────────────────
@@ -131,10 +135,12 @@ public async Task ClearEditingCard_ShouldBroadcastPresenceWithNullEditingState()
131135
var colResponse = await client.PostAsJsonAsync(
132136
$"/api/boards/{board.Id}/columns",
133137
new CreateColumnDto(board.Id, "Backlog", null, null));
138+
colResponse.StatusCode.Should().Be(HttpStatusCode.Created);
134139
var column = await colResponse.Content.ReadFromJsonAsync<ColumnDto>();
135140
var cardResponse = await client.PostAsJsonAsync(
136141
$"/api/boards/{board.Id}/cards",
137142
new CreateCardDto(board.Id, column!.Id, "Test Card", null, null, null));
143+
cardResponse.StatusCode.Should().Be(HttpStatusCode.Created);
138144
var card = await cardResponse.Content.ReadFromJsonAsync<CardDto>();
139145

140146
var events = new EventCollector<BoardPresenceSnapshot>();
@@ -412,13 +418,13 @@ public async Task SameUser_TwoConnections_DisconnectOne_ShouldNotRemovePresence(
412418
await conn1.InvokeAsync("JoinBoard", board.Id);
413419
await SignalRTestHelper.WaitForEventsAsync(events1, 1);
414420

415-
var conn2 = SignalRTestHelper.CreateBoardsHubConnection(_factory, user.Token);
421+
await using var conn2 = SignalRTestHelper.CreateBoardsHubConnection(_factory, user.Token);
416422
conn2.On<BoardPresenceSnapshot>("boardPresence", _ => { });
417423
await conn2.StartAsync();
418424
await conn2.InvokeAsync("JoinBoard", board.Id);
419425
await SignalRTestHelper.WaitForEventsAsync(events1, 2);
420426

421-
// Disconnect conn2
427+
// Disconnect conn2 (DisposeAsync is idempotent; await using provides safety net)
422428
events1.Clear();
423429
await conn2.DisposeAsync();
424430

backend/tests/Taskdeck.Api.Tests/Support/SignalRTestHelper.cs

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,21 @@ public static async Task<IReadOnlyList<T>> WaitForEventsAsync<T>(
4242
int expectedCount,
4343
TimeSpan? timeout = null)
4444
{
45-
var deadline = DateTime.UtcNow + (timeout ?? TimeSpan.FromSeconds(5));
46-
while (collector.Count < expectedCount && DateTime.UtcNow < deadline)
45+
var effectiveTimeout = timeout ?? TimeSpan.FromSeconds(5);
46+
var deadline = DateTimeOffset.UtcNow + effectiveTimeout;
47+
while (collector.Count < expectedCount && DateTimeOffset.UtcNow < deadline)
4748
{
4849
await Task.Delay(50);
4950
}
5051

51-
return collector.ToList();
52+
var result = collector.ToList();
53+
if (result.Count < expectedCount)
54+
{
55+
throw new TimeoutException(
56+
$"Expected {expectedCount} event(s) but received {result.Count} within {effectiveTimeout.TotalSeconds}s.");
57+
}
58+
59+
return result;
5260
}
5361
}
5462

0 commit comments

Comments
 (0)