Fix GitHub Actions test workflow and integration test infrastructure#77
Fix GitHub Actions test workflow and integration test infrastructure#77PhantomDave merged 4 commits intoadded-test-workflowfrom
Conversation
Co-authored-by: PhantomDave <34485699+PhantomDave@users.noreply.github.com>
… details for debugging Co-authored-by: PhantomDave <34485699+PhantomDave@users.noreply.github.com>
|
@copilot The tests should work at the end of this PR |
…QL API Co-authored-by: PhantomDave <34485699+PhantomDave@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
This PR fixes the GitHub Actions test workflow and resolves integration test infrastructure issues that prevented tests from running successfully. The changes correct GraphQL mutation/query naming to match HotChocolate's conventions and establish proper test database isolation using EF Core's InMemory provider.
Key changes:
- Updated GitHub Actions to modern versions and added database/backend startup for E2E test prerequisites
- Fixed EF Core database provider conflicts by using isolated service provider with InMemory database
- Corrected GraphQL operation names (
loginvsloginAccount,accountByEmailvsgetAccountByEmail) to match HotChocolate's automatic camelCase conversion
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
PhantomDave.BankTracking.IntegrationTests/Helpers/GraphQLTestFactory.cs |
Configures test infrastructure with InMemory database, removes hosted services to prevent conflicts, adds GraphQL exception details, and bypasses authorization for testing |
PhantomDave.BankTracking.IntegrationTests/GraphQL/AccountIntegrationTests.cs |
Updates GraphQL operation names to match backend API conventions, corrects JSON parsing paths for login responses, and simplifies test assertions |
|
|
||
| public class GraphQLTestFactory : WebApplicationFactory<PhantomDave.BankTracking.Api.Program> | ||
| { | ||
| private static readonly string DatabaseName = $"InMemoryTestDb_{Guid.NewGuid()}"; |
There was a problem hiding this comment.
The static DatabaseName field creates a single shared database for all test class instances. This can cause test interference when xUnit runs tests in parallel or when multiple test classes use the same factory. Consider generating a unique database name per factory instance:
private readonly string _databaseName = $"InMemoryTestDb_{Guid.NewGuid()}";Then use _databaseName instead of DatabaseName in line 73.
| // Create a new service provider for EF Core with only InMemory provider | ||
| var efServiceProvider = new ServiceCollection() | ||
| .AddEntityFrameworkInMemoryDatabase() | ||
| .BuildServiceProvider(); |
There was a problem hiding this comment.
The efServiceProvider is created on every call to ConfigureWebHost, which happens multiple times during test execution. Consider making this a static singleton to avoid creating duplicate service providers:
private static readonly IServiceProvider EfServiceProvider = new ServiceCollection()
.AddEntityFrameworkInMemoryDatabase()
.BuildServiceProvider();This improves performance and ensures consistent EF Core service configuration across all tests.
| // For integration tests, configure authorization to allow anonymous access | ||
| // This bypasses the [Authorize] attribute requirement |
There was a problem hiding this comment.
The comment describes bypassing [Authorize] attributes, but this configuration actually replaces the default authorization policy globally rather than bypassing the attribute. This means all endpoints still go through authorization checks, they just always succeed.
Consider clarifying the comment:
// Configure authorization policy to always succeed in tests
// This allows testing protected endpoints without JWT token validationThis better explains that authorization still runs but always returns true.
| // For integration tests, configure authorization to allow anonymous access | |
| // This bypasses the [Authorize] attribute requirement | |
| // Configure authorization policy to always succeed in tests | |
| // This allows testing protected endpoints without JWT token validation |
| .BuildServiceProvider(); | ||
|
|
||
| // Re-register with InMemory database using the isolated service provider | ||
| // Use a static database name so all tests in the class share the same database |
There was a problem hiding this comment.
The comment states "Use a static database name so all tests in the class share the same database", but this isn't accurate. The static DatabaseName field means all factory instances share the same database, not just tests within a single class. When multiple test classes use this factory, they'll all share the same database, which can cause unexpected test interference.
Update the comment to clarify:
// Use a static database name so all test classes using this factory share the same database
// WARNING: This can cause test interference across test classes| // Use a static database name so all tests in the class share the same database | |
| // Use a static database name so all test classes using this factory share the same database | |
| // WARNING: This can cause test interference across test classes |
| services.AddAuthorization(options => | ||
| { | ||
| options.UseInMemoryDatabase($"InMemoryTestDb_{Guid.NewGuid()}"); | ||
| options.DefaultPolicy = new Microsoft.AspNetCore.Authorization.AuthorizationPolicyBuilder() | ||
| .RequireAssertion(_ => true) // Always return true, effectively allowing all requests | ||
| .Build(); | ||
| }); |
There was a problem hiding this comment.
Calling AddAuthorization here creates a new authorization service registration, potentially conflicting with the existing one from the main application. This may not have the intended effect of replacing the default policy. Instead, use PostConfigure to modify the existing authorization options:
services.PostConfigure<Microsoft.AspNetCore.Authorization.AuthorizationOptions>(options =>
{
options.DefaultPolicy = new Microsoft.AspNetCore.Authorization.AuthorizationPolicyBuilder()
.RequireAssertion(_ => true)
.Build();
});This ensures the authorization configuration is modified after the main application setup runs.
The test workflow failed due to outdated GitHub Actions versions and integration tests having infrastructure issues that prevented them from running successfully.
Changes
Workflow Updates
checkout@v5,setup-dotnet@v5,setup-node@v6for consistency with other workflowscache: 'npm'andcache-dependency-pathto Node setup for faster buildsprestart)Integration Test Infrastructure Fixes
UseInternalServiceProviderwith isolated service provider containing only InMemory databaseloginvsloginAccount,userAccountvsgetUserAccount,accountByEmailvsgetAccountByEmail)data.loginvsdata.loginAccount)[Authorize]attributes in test environment (WebApplicationFactory limitation)Test Results
All tests now pass successfully:
Technical Details
The integration test infrastructure is now fully functional with proper database isolation and GraphQL API alignment.
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.