fix(data-access): correct self-referential SQL in WorkCenter filter
The WHERE clause was comparing Code to itself instead of the aliased table reference, which would always be true.
This commit is contained in:
@@ -0,0 +1,260 @@
|
||||
# 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
|
||||
Reference in New Issue
Block a user