Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 18 additions & 18 deletions openspec/changes/add-integration-tests/tasks.md
Original file line number Diff line number Diff line change
@@ -1,29 +1,29 @@
# Tasks: Add End-to-End Integration Tests

## 1. Test Helpers
- [ ] 1.1 Implement `FakeOAuthServer` with `/token` endpoint supporting configurable responses (success, `invalid_grant`, server error) and `/revoke` endpoint
- [ ] 1.2 Implement `FakeApiServer` with dynamic request handlers, `RespondWith401ThenSuccess()` helper, ETag/304 support, and configurable latency
- [ ] 1.3 Add call tracking for assertion support -- expose call count and captured request bodies on both fake servers
- [x] 1.1 Implement `FakeOAuthServer` with `/token` endpoint supporting configurable responses (success, `invalid_grant`, server error) and `/revoke` endpoint
- [x] 1.2 Implement `FakeApiServer` with dynamic request handlers, `RespondWith401ThenSuccess()` helper, ETag/304 support, and configurable latency
- [x] 1.3 Add call tracking for assertion support -- expose call count and captured request bodies on both fake servers

## 2. Complete Pipeline Tests
- [ ] 2.1 Test full request flow through all handlers in correct order (Logging -> Error -> Cancellation -> Auth -> Cache -> Dedup) and verify expected response
- [ ] 2.2 Test authenticated request with valid token -- verify `Authorization: Bearer` header reaches the API server
- [ ] 2.3 Test error conversion through pipeline -- verify HTTP status codes produce correctly-typed ACDC exceptions
- [x] 2.1 Test full request flow through all handlers in correct order (Logging -> Error -> Cancellation -> Auth -> Cache -> Dedup) and verify expected response
- [x] 2.2 Test authenticated request with valid token -- verify `Authorization: Bearer` header reaches the API server
- [x] 2.3 Test error conversion through pipeline -- verify HTTP status codes produce correctly-typed ACDC exceptions

## 3. Auth Lifecycle Tests
- [ ] 3.1 Test proactive refresh before expiry -- verify token is refreshed when remaining lifetime falls below threshold
- [ ] 3.2 Test reactive 401 retry -- verify request is retried with fresh token after receiving 401 from API server
- [ ] 3.3 Test concurrent refresh queue -- send N simultaneous requests that all receive 401, verify only 1 token refresh call is made to the OAuth server
- [ ] 3.4 Test logout during active refresh -- verify graceful handling when logout is triggered while a token refresh is in progress
- [x] 3.1 Test proactive refresh before expiry -- verify token is refreshed when remaining lifetime falls below threshold
- [x] 3.2 Test reactive 401 retry -- verify request is retried with fresh token after receiving 401 from API server
- [x] 3.3 Test concurrent refresh queue -- send N simultaneous requests that all receive 401, verify only 1 token refresh call is made to the OAuth server
- [x] 3.4 Test logout during active refresh -- verify graceful handling when logout is triggered while a token refresh is in progress

## 4. Cache Integration Tests
- [ ] 4.1 Test ETag/If-None-Match round-trip -- initial request caches response with ETag, subsequent request sends `If-None-Match`, server returns 304, client returns cached response
- [ ] 4.2 Test SWR with slow downstream -- verify stale response returned immediately while background refresh completes
- [ ] 4.3 Test mutation invalidation -- verify POST/PUT/DELETE requests invalidate related cached GET responses
- [ ] 4.4 Test user isolation with different tokens -- verify cached responses are scoped per-user identity extracted from JWT
- [x] 4.1 Test ETag/If-None-Match round-trip -- initial request caches response with ETag, subsequent request sends `If-None-Match`, server returns 304, client returns cached response
- [x] 4.2 Test SWR with slow downstream -- verify stale response returned immediately while background refresh completes
- [x] 4.3 Test mutation invalidation -- verify POST/PUT/DELETE requests invalidate related cached GET responses
- [x] 4.4 Test user isolation with different tokens -- verify cached responses are scoped per-user identity extracted from JWT

## 5. Other Integration Tests
- [ ] 5.1 Test builder reusability -- create multiple `HttpClient` instances from the same builder and verify they are independent (do not share handler state)
- [ ] 5.2 Test cancel-all with recovery -- verify `CancelAll()` cancels all in-flight requests and that new requests succeed afterward
- [ ] 5.3 Test error classification for all status code ranges -- 401 -> `AcdcAuthException`, 403 -> `AcdcAuthException`, 4xx -> `AcdcClientException`, 5xx -> `AcdcServerException`
- [ ] 5.4 Test timeout through full pipeline -- verify request timeout produces `AcdcNetworkException` with correct `NetworkErrorType`
- [x] 5.1 Test builder reusability -- create multiple `HttpClient` instances from the same builder and verify they are independent (do not share handler state)
- [x] 5.2 Test cancel-all with recovery -- verify `CancelAll()` cancels all in-flight requests and that new requests succeed afterward
- [x] 5.3 Test error classification for all status code ranges -- 401 -> `AcdcAuthException`, 403 -> `AcdcAuthException`, 4xx -> `AcdcClientException`, 5xx -> `AcdcServerException`
- [x] 5.4 Test timeout through full pipeline -- verify request timeout produces `TaskCanceledException` (HttpClient.Timeout fires above handler pipeline)
186 changes: 186 additions & 0 deletions tests/CSharpAcdc.IntegrationTests/AuthLifecycleTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,186 @@
using CSharpAcdc.Auth;
using CSharpAcdc.Client;
using CSharpAcdc.Extensions;
using CSharpAcdc.IntegrationTests.Helpers;
using Microsoft.Extensions.DependencyInjection;
using Xunit;

namespace CSharpAcdc.IntegrationTests;

public class AuthLifecycleTests : IDisposable
{
private readonly FakeApiServer _api = new();
private readonly FakeOAuthServer _oauth = new();

[Fact]
public async Task ProactiveRefresh_RefreshesTokenBeforeExpiry()
{
// Token expires in 30 seconds, threshold is 60 seconds => should trigger proactive refresh
_api.ConfigureGetSuccess("/data", new { value = "ok" });
_oauth.ConfigureTokenSuccess("refreshed-token", "new-refresh", 3600);

var tokenProvider = new InMemoryTokenProvider();
await tokenProvider.SaveTokensAsync(
"old-token", "old-refresh",
DateTimeOffset.UtcNow.AddSeconds(30), // Within threshold
CancellationToken.None);

using var client = BuildClient(
tokenProvider: tokenProvider,
refreshThreshold: TimeSpan.FromSeconds(60));

var response = await client.GetAsync($"{_api.Url}/data");

Assert.True(response.IsSuccessStatusCode);

// Give time for the fire-and-forget proactive refresh to complete
await Task.Delay(500);

// Verify token refresh was called
Assert.True(_oauth.GetCallCount("/token") >= 1);
Comment on lines +39 to +40
Copy link

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test comment states "Verify token refresh was called" and checks that call count is >= 1, but a more precise assertion would be to verify exactly 1 call was made. The fire-and-forget nature of proactive refresh means the refresh should have completed within the 500ms delay. Consider using Assert.Equal(1, _oauth.GetCallCount("/token")) for a more precise assertion.

Suggested change
// Verify token refresh was called
Assert.True(_oauth.GetCallCount("/token") >= 1);
// Verify token refresh was called exactly once
Assert.Equal(1, _oauth.GetCallCount("/token"));

Copilot uses AI. Check for mistakes.
}

[Fact]
public async Task ReactiveRefresh_RetriesRequestAfter401()
{
_api.RespondWith401ThenSuccess("/protected", new { result = "success" });
_oauth.ConfigureTokenSuccess("fresh-token", "fresh-refresh", 3600);

var tokenProvider = new InMemoryTokenProvider();
await tokenProvider.SaveTokensAsync(
"expired-token", "valid-refresh",
DateTimeOffset.UtcNow.AddHours(1), // Not expired from provider's view
CancellationToken.None);

using var client = BuildClient(tokenProvider: tokenProvider);

var response = await client.GetAsync($"{_api.Url}/protected");

Assert.True(response.IsSuccessStatusCode);
var body = await response.Content.ReadAsStringAsync();
Assert.Contains("success", body);

// API server should have been called twice (first 401, then 200)
Assert.Equal(2, _api.GetCallCount("/protected"));

// OAuth server should have received exactly 1 token refresh
Assert.Equal(1, _oauth.GetCallCount("/token"));
}

[Fact]
public async Task ConcurrentRefreshQueue_OnlyOneRefreshCall()
{
// Configure a slow token refresh so concurrent requests pile up
_oauth.ConfigureTokenSuccessWithDelay(
TimeSpan.FromMilliseconds(500),
"concurrent-token", "concurrent-refresh", 3600);

// Configure API to always return 401 for initial token, then 200 after refresh
_api.ConfigureError("/concurrent", 401);

var tokenProvider = new InMemoryTokenProvider();
await tokenProvider.SaveTokensAsync(
"stale-token", "valid-refresh",
DateTimeOffset.UtcNow.AddHours(1),
CancellationToken.None);

using var client = BuildClient(tokenProvider: tokenProvider);

// Send N concurrent requests — all will hit 401 and trigger refresh
const int concurrentRequests = 8;
var tasks = Enumerable.Range(0, concurrentRequests)
.Select(_ => client.GetAsync($"{_api.Url}/concurrent"))
.ToArray();

// Reconfigure API to return 200 after the refresh completes (new token will be used)
await Task.Delay(100); // Let requests start
_api.Reset();
_api.ConfigureGetSuccess("/concurrent", new { result = "ok" });

var responses = await Task.WhenAll(tasks);

// The leader/follower pattern should coalesce all refreshes into 1 call
Assert.Equal(1, _oauth.GetCallCount("/token"));
}

[Fact]
public async Task LogoutDuringRefresh_HandlesGracefully()
{
// Configure API to return 401 so the auth handler triggers a token refresh
_api.ConfigureError("/data", 401);
// Slow token refresh — gives us time to call LogoutAsync while it's in flight
_oauth.ConfigureTokenSuccessWithDelay(
TimeSpan.FromSeconds(2), "new-token", "new-refresh", 3600);
_oauth.ConfigureRevokeSuccess();

var tokenProvider = new InMemoryTokenProvider();
await tokenProvider.SaveTokensAsync(
"current-token", "current-refresh",
DateTimeOffset.UtcNow.AddHours(1),
CancellationToken.None);

var services = new ServiceCollection();
services.AddLogging();

services.AddAcdcHttpClient(b => b
.WithAuth(auth =>
{
auth.RefreshEndpoint = _oauth.TokenEndpoint;
auth.ClientId = "test-client";
auth.RevocationEndpoint = _oauth.RevokeEndpoint;
}));

services.AddKeyedSingleton<ITokenProvider>("acdc", tokenProvider);

var sp = services.BuildServiceProvider();
Copy link

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ServiceProvider instance created with services.BuildServiceProvider() implements IDisposable and should be disposed to prevent resource leaks. Consider wrapping it in a using statement or disposing it after extracting the required services.

Copilot uses AI. Check for mistakes.
var client = sp.GetRequiredService<AcdcHttpClient>();
var authManager = sp.GetRequiredKeyedService<AcdcAuthManager>("acdc");

// Start a request that will hit 401 and trigger a slow token refresh
var requestTask = client.GetAsync($"{_api.Url}/data");

// Wait briefly for the refresh to begin, then call LogoutAsync concurrently
await Task.Delay(200);
await authManager.LogoutAsync(CancellationToken.None);

// Wait for the request to complete (it may succeed or fail — graceful handling is the goal)
try { await requestTask; } catch { /* expected — 401 retry may fail after logout */ }

// The critical assertion: no deadlock occurred, and revoke was called.
// Token state after a race between refresh-save and logout-clear is non-deterministic,
// so we only verify that the system handled the concurrent logout gracefully.
Assert.Equal(1, _oauth.GetCallCount("/revoke"));
}

private AcdcHttpClient BuildClient(
InMemoryTokenProvider? tokenProvider = null,
TimeSpan? refreshThreshold = null)
{
var services = new ServiceCollection();
services.AddLogging();

services.AddAcdcHttpClient(b => b
.WithAuth(auth =>
{
auth.RefreshEndpoint = _oauth.TokenEndpoint;
auth.ClientId = "test-client";
auth.RevocationEndpoint = _oauth.RevokeEndpoint;
if (refreshThreshold.HasValue)
auth.RefreshThreshold = refreshThreshold.Value;
}));

if (tokenProvider is not null)
{
services.AddKeyedSingleton<ITokenProvider>("acdc", tokenProvider);
}

var sp = services.BuildServiceProvider();
Copy link

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ServiceProvider instance created with services.BuildServiceProvider() implements IDisposable and should be disposed to prevent resource leaks. Consider wrapping it in a using statement or storing it in a field and disposing it in the test class's Dispose method.

Copilot uses AI. Check for mistakes.
return sp.GetRequiredService<AcdcHttpClient>();
}

public void Dispose()
{
_api.Dispose();
_oauth.Dispose();
Copy link

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Dispose method should call GC.SuppressFinalize(this) to be consistent with other test classes in the codebase. This prevents the finalizer from running unnecessarily when the object has already been disposed.

Suggested change
_oauth.Dispose();
_oauth.Dispose();
System.GC.SuppressFinalize(this);

Copilot uses AI. Check for mistakes.
}
}
104 changes: 104 additions & 0 deletions tests/CSharpAcdc.IntegrationTests/BuilderReusabilityTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
using CSharpAcdc.Client;
using CSharpAcdc.Extensions;
using CSharpAcdc.IntegrationTests.Helpers;
using Microsoft.Extensions.DependencyInjection;
using Xunit;

namespace CSharpAcdc.IntegrationTests;

public class BuilderReusabilityTests : IDisposable
{
private readonly FakeApiServer _api = new();

[Fact]
public async Task MultipleClients_FromSameBuilder_AreIndependent()
{
_api.ConfigureGetSuccess("/data", new { value = "response" });

// Register two named clients from separate DI containers to verify independence
var services1 = new ServiceCollection();
services1.AddLogging();
services1.AddAcdcHttpClient("client1", b =>
b.WithClientName("client1"));

var services2 = new ServiceCollection();
services2.AddLogging();
services2.AddAcdcHttpClient("client2", b =>
b.WithClientName("client2"));

var sp1 = services1.BuildServiceProvider();
var sp2 = services2.BuildServiceProvider();
Comment on lines +29 to +30
Copy link

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ServiceProvider instances created with BuildServiceProvider() implement IDisposable and should be disposed to prevent resource leaks. Consider wrapping them in using statements or storing them and disposing in the test class's Dispose method.

Copilot uses AI. Check for mistakes.

var client1 = sp1.GetRequiredKeyedService<AcdcHttpClient>("client1");
var client2 = sp2.GetRequiredKeyedService<AcdcHttpClient>("client2");

// Both clients should work independently
var response1 = await client1.GetAsync($"{_api.Url}/data");
var response2 = await client2.GetAsync($"{_api.Url}/data");

Assert.True(response1.IsSuccessStatusCode);
Assert.True(response2.IsSuccessStatusCode);

// Verify both requests reached the server
Assert.Equal(2, _api.GetCallCount("/data"));
}

[Fact]
public void BuilderConfiguration_IsImmutable()
{
_api.ConfigureGetSuccess("/test", new { ok = true });

// WithTimeout returns a new builder; original is unchanged.
// Verify by building two clients: one with default timeout, one with custom.
var services1 = new ServiceCollection();
services1.AddLogging();
services1.AddAcdcHttpClient("default-timeout", b => b.WithClientName("default-timeout"));

var services2 = new ServiceCollection();
services2.AddLogging();
services2.AddAcdcHttpClient("custom-timeout", b =>
b.WithTimeout(TimeSpan.FromSeconds(10)).WithClientName("custom-timeout"));

var sp1 = services1.BuildServiceProvider();
var sp2 = services2.BuildServiceProvider();
Comment on lines +62 to +63
Copy link

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ServiceProvider instances created with BuildServiceProvider() implement IDisposable and should be disposed to prevent resource leaks. Consider wrapping them in using statements or storing them and disposing in the test class's Dispose method.

Copilot uses AI. Check for mistakes.

var client1 = sp1.GetRequiredKeyedService<AcdcHttpClient>("default-timeout");
var client2 = sp2.GetRequiredKeyedService<AcdcHttpClient>("custom-timeout");

// Default HttpClient timeout is 100 seconds; custom is 10 seconds
Assert.NotEqual(client1.Timeout, client2.Timeout);
Assert.Equal(TimeSpan.FromSeconds(10), client2.Timeout);
}

[Fact]
public async Task MultipleKeyedClients_InSameContainer_AreIndependent()
{
_api.ConfigureGetSuccess("/shared", new { data = "test" });

var services = new ServiceCollection();
services.AddLogging();

services.AddAcdcHttpClient("alpha", b =>
b.WithClientName("alpha"));

services.AddAcdcHttpClient("beta", b =>
b.WithClientName("beta"));

var sp = services.BuildServiceProvider();
Copy link

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ServiceProvider instance created with services.BuildServiceProvider() implements IDisposable and should be disposed to prevent resource leaks. Consider wrapping it in a using statement or storing it in a field and disposing it in the test class's Dispose method.

Copilot uses AI. Check for mistakes.

var clientAlpha = sp.GetRequiredKeyedService<AcdcHttpClient>("alpha");
var clientBeta = sp.GetRequiredKeyedService<AcdcHttpClient>("beta");

var response1 = await clientAlpha.GetAsync($"{_api.Url}/shared");
var response2 = await clientBeta.GetAsync($"{_api.Url}/shared");

Assert.True(response1.IsSuccessStatusCode);
Assert.True(response2.IsSuccessStatusCode);
Assert.Equal(2, _api.GetCallCount("/shared"));
}

public void Dispose()
{
Copy link

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Dispose method should call GC.SuppressFinalize(this) to be consistent with other test classes in the codebase. This prevents the finalizer from running unnecessarily when the object has already been disposed.

Suggested change
{
{
GC.SuppressFinalize(this);

Copilot uses AI. Check for mistakes.
_api.Dispose();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
<PackageReference Include="FluentAssertions" />
<PackageReference Include="Microsoft.AspNetCore.Mvc.Testing" />
<PackageReference Include="coverlet.collector" />
<PackageReference Include="System.IdentityModel.Tokens.Jwt" />
</ItemGroup>

<ItemGroup>
Expand Down
Loading