d4135e8ad3
The WHERE clause was comparing Code to itself instead of the aliased table reference, which would always be true.
261 lines
9.0 KiB
Markdown
261 lines
9.0 KiB
Markdown
# API Client Tests Design
|
||
|
||
## Purpose
|
||
|
||
Update unit tests for Api, Api.Integration, and Client projects to use the new API contracts (`ApiRoutes`, `ApiResult<T>`, `I*ApiClient` interfaces).
|
||
|
||
## Goals
|
||
|
||
1. Replace hardcoded route strings with `ApiRoutes.*` constants
|
||
2. Add comprehensive unit tests for API client base behavior
|
||
3. Add focused unit tests for each API client (success + representative error)
|
||
4. Add integration tests using actual `*ApiClient` classes against test server
|
||
5. Cover edge cases: malformed payloads, empty responses, network errors
|
||
|
||
## Test Structure
|
||
|
||
```
|
||
tests/
|
||
├── JdeScoping.Client.Tests/
|
||
│ └── Services/
|
||
│ ├── CryptoServiceTests.cs (existing)
|
||
│ ├── ApiClientBaseTests.cs (new - ALL 6 ApiResult cases + edge cases)
|
||
│ ├── SearchApiClientTests.cs (new - success + 1 error per method)
|
||
│ ├── LookupApiClientTests.cs (new - success + 1 error per method)
|
||
│ ├── AuthApiClientTests.cs (new - success + 1 error per method)
|
||
│ └── FileApiClientTests.cs (new - success + 1 error per method)
|
||
│
|
||
├── JdeScoping.Api.IntegrationTests/
|
||
│ ├── AuthenticationTests.cs (update routes to ApiRoutes.*)
|
||
│ ├── FileControllerIntegrationTests.cs (update routes to ApiRoutes.*)
|
||
│ ├── SignalRTests.cs (update routes if needed)
|
||
│ └── ClientIntegrationTests/ (new folder)
|
||
│ ├── SearchApiClientIntegrationTests.cs
|
||
│ ├── LookupApiClientIntegrationTests.cs
|
||
│ ├── AuthApiClientIntegrationTests.cs
|
||
│ └── FileApiClientIntegrationTests.cs
|
||
```
|
||
|
||
## Unit Test Approach
|
||
|
||
### ApiClientBaseTests - Full Coverage
|
||
|
||
Tests ALL 6 HTTP status code → ApiResult mappings plus edge cases:
|
||
|
||
```csharp
|
||
public class ApiClientBaseTests
|
||
{
|
||
// Core status code mappings (test once here, not repeated per client)
|
||
[Fact] Task GetAsync_Returns200_MapsToSuccessValue()
|
||
[Fact] Task GetAsync_Returns404_MapsToNotFound()
|
||
[Fact] Task GetAsync_Returns400_MapsToValidationError()
|
||
[Fact] Task GetAsync_Returns401_MapsToUnauthorized()
|
||
[Fact] Task GetAsync_Returns403_MapsToForbidden()
|
||
[Fact] Task GetAsync_Returns500_MapsToApiError()
|
||
|
||
// Edge cases (malformed responses)
|
||
[Fact] Task GetAsync_Returns200_EmptyBody_MapsToApiError()
|
||
[Fact] Task GetAsync_Returns200_InvalidJson_MapsToApiError()
|
||
[Fact] Task GetAsync_Returns204_MapsToApiError()
|
||
[Fact] Task GetAsync_Returns422_MapsToValidationError()
|
||
[Fact] Task GetAsync_NetworkException_MapsToApiError()
|
||
[Fact] Task GetAsync_Timeout_MapsToApiError()
|
||
|
||
// Also test PostAsync, GetBytesAsync, PostMultipartAsync
|
||
}
|
||
```
|
||
|
||
### Client Test Pattern - Lean Coverage
|
||
|
||
Each client tests:
|
||
1. Correct route called (verify path AND HTTP method)
|
||
2. Query string encoding (for lookup methods)
|
||
3. Success case with response deserialization
|
||
4. One representative error case (usually 401 for auth-required, 404 for not found)
|
||
|
||
```csharp
|
||
public class SearchApiClientTests
|
||
{
|
||
private readonly MockHttpMessageHandler _mockHttp;
|
||
private readonly SearchApiClient _client;
|
||
|
||
// Route + method verification
|
||
[Fact] Task GetUserSearchesAsync_CallsCorrectRoute_WithGetMethod()
|
||
[Fact] Task CreateSearchAsync_CallsCorrectRoute_WithPostMethod()
|
||
|
||
// Success cases
|
||
[Fact] Task GetUserSearchesAsync_Success_ReturnsSearchList()
|
||
[Fact] Task GetSearchAsync_Success_ReturnsSearch()
|
||
[Fact] Task CreateSearchAsync_Success_ReturnsId()
|
||
|
||
// Representative error (not all 6 - those are tested in ApiClientBaseTests)
|
||
[Fact] Task GetSearchAsync_404_ReturnsNotFound()
|
||
[Fact] Task GetUserSearchesAsync_401_ReturnsUnauthorized()
|
||
}
|
||
|
||
public class LookupApiClientTests
|
||
{
|
||
// Query string encoding is important for lookup methods
|
||
[Fact] Task FindItemsAsync_EncodesQueryString_Correctly()
|
||
[Fact] Task FindItemsAsync_WithSpecialChars_EncodesCorrectly()
|
||
|
||
[Fact] Task FindItemsAsync_Success_ReturnsItemList()
|
||
[Fact] Task FindOperatorsAsync_Success_ReturnsUserList()
|
||
}
|
||
```
|
||
|
||
## Integration Test Approach
|
||
|
||
### Shared HttpClient for Auth State
|
||
|
||
**Critical:** Use a single `HttpClient` instance with `HandleCookies = true` for all authenticated calls. Auth is cookie-based.
|
||
|
||
```csharp
|
||
public class ClientIntegrationTestBase : IClassFixture<TestWebApplicationFactory>
|
||
{
|
||
protected readonly TestWebApplicationFactory Factory;
|
||
protected readonly HttpClient SharedClient; // Single client, preserves cookies
|
||
|
||
// API clients share the authenticated HttpClient
|
||
protected readonly ISearchApiClient SearchClient;
|
||
protected readonly ILookupApiClient LookupClient;
|
||
protected readonly IAuthApiClient AuthClient;
|
||
protected readonly IFileApiClient FileClient;
|
||
|
||
public ClientIntegrationTestBase(TestWebApplicationFactory factory)
|
||
{
|
||
Factory = factory;
|
||
SharedClient = factory.CreateClient(new WebApplicationFactoryClientOptions
|
||
{
|
||
HandleCookies = true,
|
||
AllowAutoRedirect = false
|
||
});
|
||
|
||
// All clients share the same HttpClient (cookie container)
|
||
SearchClient = new SearchApiClient(SharedClient);
|
||
LookupClient = new LookupApiClient(SharedClient);
|
||
FileClient = new FileApiClient(SharedClient);
|
||
|
||
// AuthApiClient needs crypto service
|
||
var cryptoService = CreateCryptoService(SharedClient);
|
||
AuthClient = new AuthApiClient(SharedClient, cryptoService);
|
||
}
|
||
|
||
protected async Task LoginAsync(string username = "testuser", string password = "testpass")
|
||
{
|
||
var result = await AuthClient.LoginAsync(new EncryptedLoginRequest(...));
|
||
result.IsSuccess.ShouldBeTrue();
|
||
}
|
||
|
||
// For testing unauthorized scenarios, create a fresh client
|
||
protected HttpClient CreateFreshClient() => Factory.CreateClient(new WebApplicationFactoryClientOptions
|
||
{
|
||
HandleCookies = false, // No cookies = no auth
|
||
AllowAutoRedirect = false
|
||
});
|
||
}
|
||
```
|
||
|
||
### Integration Test Pattern
|
||
|
||
```csharp
|
||
public class SearchApiClientIntegrationTests : ClientIntegrationTestBase
|
||
{
|
||
public SearchApiClientIntegrationTests(TestWebApplicationFactory factory) : base(factory) { }
|
||
|
||
[Fact]
|
||
public async Task GetUserSearchesAsync_WithAuth_ReturnsSearchList()
|
||
{
|
||
// Arrange
|
||
await LoginAsync();
|
||
|
||
// Act
|
||
var result = await SearchClient.GetUserSearchesAsync();
|
||
|
||
// Assert
|
||
result.IsSuccess.ShouldBeTrue();
|
||
result.Value.ShouldNotBeNull();
|
||
}
|
||
|
||
[Fact]
|
||
public async Task GetUserSearchesAsync_WithoutAuth_ReturnsUnauthorized()
|
||
{
|
||
// Use fresh client without cookies
|
||
var freshClient = new SearchApiClient(CreateFreshClient());
|
||
|
||
var result = await freshClient.GetUserSearchesAsync();
|
||
|
||
result.IsUnauthorized.ShouldBeTrue();
|
||
}
|
||
}
|
||
```
|
||
|
||
### Test Data and Cleanup
|
||
|
||
Integration tests use:
|
||
- `UseFakeAuth = true` in test server (no real LDAP)
|
||
- In-memory or test database with seeded data
|
||
- xUnit collection fixtures to avoid parallelization issues with shared state
|
||
|
||
```csharp
|
||
[Collection("IntegrationTests")] // Prevents parallel execution
|
||
public class SearchApiClientIntegrationTests : ClientIntegrationTestBase
|
||
{
|
||
// Tests run sequentially within collection
|
||
}
|
||
```
|
||
|
||
### Update Existing Tests
|
||
|
||
Replace hardcoded routes:
|
||
|
||
```csharp
|
||
// Before:
|
||
var response = await _client.GetAsync("/api/search");
|
||
|
||
// After:
|
||
var response = await _client.GetAsync(ApiRoutes.Search.Base);
|
||
```
|
||
|
||
## Test Cases Per Client (Revised - Lean)
|
||
|
||
### ApiClientBaseTests (~18 tests)
|
||
- 6 status code mappings × 3 HTTP methods (GET, POST, multipart)
|
||
- 6 edge case tests (empty body, invalid JSON, 204, 422, network error, timeout)
|
||
|
||
### SearchApiClient (~12 tests)
|
||
- 6 route/method verifications
|
||
- 6 success cases (one per method)
|
||
- 2 representative error cases
|
||
|
||
### LookupApiClient (~10 tests)
|
||
- 4 route/method verifications
|
||
- 2 query string encoding tests
|
||
- 4 success cases
|
||
|
||
### AuthApiClient (~8 tests)
|
||
- 4 route/method verifications
|
||
- 4 success cases
|
||
|
||
### FileApiClient (~16 tests)
|
||
- 8 route/method verifications
|
||
- 8 success cases
|
||
|
||
### Integration Tests (~12 tests)
|
||
- 3 per client (with auth, without auth, specific scenario)
|
||
|
||
## Total Test Count (Revised)
|
||
|
||
- Unit tests: ~64 (down from 132)
|
||
- Integration tests: ~12
|
||
- Total: ~76 new tests
|
||
|
||
## Key Design Decisions
|
||
|
||
1. **All 6 ApiResult cases tested in ApiClientBaseTests only** - Not repeated per client method
|
||
2. **Shared HttpClient for auth** - Single client with cookies for authenticated integration tests
|
||
3. **Fresh client for unauthorized tests** - New HttpClient without cookies
|
||
4. **Query string encoding** - Explicitly tested for lookup methods
|
||
5. **Edge cases covered** - Malformed JSON, empty bodies, network errors
|
||
6. **No Activator.CreateInstance** - Direct instantiation with shared HttpClient
|