Add integration tests for ScanJobs and Scanner services#56
Conversation
Add landing page, fix CI failures, and update dependencies
…d-to-end workflow tests - Implement ScanJobsIntegrationTests to validate the ScanJobs API endpoints. - Create ScanJobsWebFactory for setting up an in-memory database and authenticated client. - Add ScannerIntegrationTests to test the Scanner service endpoints with stub HTTP handlers. - Introduce ScannerWebFactory for configuring the Scanner service with in-memory database and stub clients. - Develop EndToEndWorkflowTests to verify the complete workflow from scan initiation to analytics. - Implement WorkflowBackgroundServiceConcurrencyTests to ensure proper handling of concurrent workflows. - Create ConcurrencyOrchestratorFactory to control concurrency limits for deterministic tests.
Merge pull request #56 from DeeDee1103/develop
There was a problem hiding this comment.
Pull request overview
This PR expands the repository’s integration testing surface (ScanJobs/Scanner/Orchestrator workflows) and adjusts service startup/build/development configuration (git SCM queries, docker-compose cleanup, Key Vault config, and EF Core database initialization behavior).
Changes:
- Added new integration test project coverage for ScanJobs, Scanner, Orchestrator end-to-end workflows, and workflow background-service concurrency.
- Switched database startup initialization toward EF Core migrations for relational providers and added initial migration artifacts across multiple services.
- Tweaked build/dev/runtime configuration (disable SCM git queries, remove duplicate docker-compose deps, add local dev commands, add Key Vault configuration calls).
Reviewed changes
Copilot reviewed 59 out of 68 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/StorageLens.Integration.Tests/Workflow/WorkflowBackgroundServiceConcurrencyTests.cs | New workflow concurrency tests |
| tests/StorageLens.Integration.Tests/Workflow/EndToEndWorkflowTests.cs | New orchestrator E2E tests |
| tests/StorageLens.Integration.Tests/StorageLens.Integration.Tests.csproj | Add extern-alias project refs |
| tests/StorageLens.Integration.Tests/Scanner/ScannerWebFactory.cs | Scanner test web factory |
| tests/StorageLens.Integration.Tests/Scanner/ScannerIntegrationTests.cs | Scanner integration tests |
| tests/StorageLens.Integration.Tests/ScanJobs/ScanJobsWebFactory.cs | ScanJobs test web factory |
| tests/StorageLens.Integration.Tests/ScanJobs/ScanJobsIntegrationTests.cs | ScanJobs integration tests |
| tests/StorageLens.Integration.Tests/Orchestrator/OrchestratorWebFactory.cs | Orchestrator stubbed upstreams |
| tests/StorageLens.Integration.Tests/Orchestrator/OrchestratorIntegrationTests.cs | Orchestrator integration tests |
| tests/StorageLens.Integration.Tests/Infrastructure/StorageLensWebFactory.cs | Update Locations factory for alias |
| src/StorageLens.Web/StorageLens.Web.csproj | Align EF Core packages + Design |
| src/StorageLens.Web/Program.cs | Add KeyVault configuration |
| src/StorageLens.Web/Migrations/ClientsDbContextModelSnapshot.cs | Clients DB snapshot |
| src/StorageLens.Web/Migrations/20260321200512_InitialCreate.Designer.cs | Clients migration designer |
| src/StorageLens.Web/Migrations/20260321200512_InitialCreate.cs | Clients initial migration |
| src/StorageLens.Shared.Infrastructure/StartupDatabaseInitializer.cs | MigrateAsync for relational DBs |
| src/StorageLens.Shared.Infrastructure/GlobalExceptionHandler.cs | Simplify exception mapping |
| src/StorageLens.Shared.Contracts/LocationContracts.cs | Split location contracts |
| src/StorageLens.Shared.Contracts/ScanJobContracts.cs | Split scanjob contracts |
| src/StorageLens.Shared.Contracts/FileInventoryContracts.cs | Split file inventory contracts |
| src/StorageLens.Shared.Contracts/DuplicatesContracts.cs | Split duplicates contracts |
| src/StorageLens.Shared.Contracts/ScannerContracts.cs | Split scanner contracts |
| src/StorageLens.Shared.Contracts/HashingContracts.cs | Split hashing contracts |
| src/StorageLens.Shared.Contracts/OrchestratorContracts.cs | Split orchestrator contracts |
| src/StorageLens.Shared.Contracts/AnalyticsContracts.cs | Split analytics contracts |
| src/StorageLens.Shared.Contracts/Class1.cs | Replace monolithic contracts file |
| src/StorageLens.Services.Locations/StorageLens.Services.Locations.csproj | Align EF Core packages + Design |
| src/StorageLens.Services.Locations/Program.cs | Add KeyVault + Program partial + seeder change |
| src/StorageLens.Services.Locations/Migrations/LocationsDbContextModelSnapshot.cs | Locations snapshot |
| src/StorageLens.Services.Locations/Migrations/20260321195700_InitialCreate.Designer.cs | Locations migration designer |
| src/StorageLens.Services.Locations/Migrations/20260321195700_InitialCreate.cs | Locations initial migration |
| src/StorageLens.Services.ScanJobs/StorageLens.Services.ScanJobs.csproj | Align EF Core packages + Design |
| src/StorageLens.Services.ScanJobs/Program.cs | Add KeyVault + Program partial + seeder change |
| src/StorageLens.Services.ScanJobs/Migrations/ScanJobsDbContextModelSnapshot.cs | ScanJobs snapshot |
| src/StorageLens.Services.ScanJobs/Migrations/20260321195945_InitialCreate.Designer.cs | ScanJobs migration designer |
| src/StorageLens.Services.ScanJobs/Migrations/20260321195945_InitialCreate.cs | ScanJobs initial migration |
| src/StorageLens.Services.Scanner/StorageLens.Services.Scanner.csproj | Align EF Core packages + Design |
| src/StorageLens.Services.Scanner/Program.cs | Add KeyVault + Program partial |
| src/StorageLens.Services.Scanner/Migrations/ScannerDbContextModelSnapshot.cs | Scanner snapshot |
| src/StorageLens.Services.Scanner/Migrations/20260321200339_InitialCreate.Designer.cs | Scanner migration designer |
| src/StorageLens.Services.Scanner/Migrations/20260321200339_InitialCreate.cs | Scanner initial migration |
| src/StorageLens.Services.Orchestrator/StorageLens.Services.Orchestrator.csproj | Align EF Core packages + Design |
| src/StorageLens.Services.Orchestrator/Program.cs | Add KeyVault + Program partial |
| src/StorageLens.Services.Orchestrator/Migrations/OrchestratorDbContextModelSnapshot.cs | Orchestrator snapshot |
| src/StorageLens.Services.Orchestrator/Migrations/20260321200443_InitialCreate.Designer.cs | Orchestrator migration designer |
| src/StorageLens.Services.Orchestrator/Migrations/20260321200443_InitialCreate.cs | Orchestrator initial migration |
| src/StorageLens.Services.FileInventory/StorageLens.Services.FileInventory.csproj | Align EF Core packages + Design |
| src/StorageLens.Services.FileInventory/Program.cs | Add KeyVault + seeder change |
| src/StorageLens.Services.FileInventory/Migrations/FileInventoryDbContextModelSnapshot.cs | FileInventory snapshot |
| src/StorageLens.Services.FileInventory/Migrations/20260321200137_InitialCreate.Designer.cs | FileInventory migration designer |
| src/StorageLens.Services.FileInventory/Migrations/20260321200137_InitialCreate.cs | FileInventory initial migration |
| src/StorageLens.Services.Duplicates/StorageLens.Services.Duplicates.csproj | Align EF Core packages + Design |
| src/StorageLens.Services.Duplicates/Program.cs | Add KeyVault + seeder change |
| src/StorageLens.Services.Duplicates/Migrations/DuplicatesDbContextModelSnapshot.cs | Duplicates snapshot |
| src/StorageLens.Services.Duplicates/Migrations/20260321200257_InitialCreate.Designer.cs | Duplicates migration designer |
| src/StorageLens.Services.Duplicates/Migrations/20260321200257_InitialCreate.cs | Duplicates initial migration |
| src/StorageLens.Services.Hashing/StorageLens.Services.Hashing.csproj | Align EF Core packages + Design |
| src/StorageLens.Services.Hashing/Program.cs | Add KeyVault configuration |
| src/StorageLens.Services.Hashing/Migrations/HashingDbContextModelSnapshot.cs | Hashing snapshot |
| src/StorageLens.Services.Hashing/Migrations/20260321200802_InitialCreate.Designer.cs | Hashing migration designer |
| src/StorageLens.Services.Hashing/Migrations/20260321200802_InitialCreate.cs | Hashing initial migration |
| src/StorageLens.Services.Analytics/Program.cs | Add KeyVault configuration |
| Directory.Build.props | Disable SCM git queries in build |
| docker-compose.yml | Remove duplicate redis depends |
| .claude/settings.local.json | Add local build/status commands |
| runtime-web.log | Updated runtime log output |
| runtime-ps.json | Updated docker ps capture |
| ERROR | Added PowerShell error artifact |
Files not reviewed (8)
- src/StorageLens.Services.Duplicates/Migrations/20260321200257_InitialCreate.Designer.cs: Language not supported
- src/StorageLens.Services.FileInventory/Migrations/20260321200137_InitialCreate.Designer.cs: Language not supported
- src/StorageLens.Services.Hashing/Migrations/20260321200802_InitialCreate.Designer.cs: Language not supported
- src/StorageLens.Services.Locations/Migrations/20260321195700_InitialCreate.Designer.cs: Language not supported
- src/StorageLens.Services.Orchestrator/Migrations/20260321200443_InitialCreate.Designer.cs: Language not supported
- src/StorageLens.Services.ScanJobs/Migrations/20260321195945_InitialCreate.Designer.cs: Language not supported
- src/StorageLens.Services.Scanner/Migrations/20260321200339_InitialCreate.Designer.cs: Language not supported
- src/StorageLens.Web/Migrations/20260321200512_InitialCreate.Designer.cs: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (dbContext.Database.IsRelational()) | ||
| { | ||
| await dbContext.Database.MigrateAsync(cancellationToken); | ||
| } | ||
| else | ||
| { | ||
| await EnsureTablesCreatedForCurrentContextAsync(dbContext, logger, cancellationToken); | ||
| var created = await dbContext.Database.EnsureCreatedAsync(cancellationToken); | ||
| if (!created) | ||
| await EnsureTablesCreatedForCurrentContextAsync(dbContext, logger, cancellationToken); |
There was a problem hiding this comment.
Switching relational initialization from EnsureCreated/CreateTables to Database.MigrateAsync can break existing deployments where schemas were previously created without migrations. If tables already exist but __EFMigrationsHistory does not, the first migration may fail with “object already exists” and prevent startup. Consider gating migrations behind a config flag, or attempting MigrateAsync and falling back to the previous EnsureCreated/CreateTables path when the database already contains tables but has no migration history.
| app, | ||
| "StorageLens.Services.Locations", | ||
| static (_, context, cancellationToken) => LocationsSeeder.SeedAsync(context, cancellationToken)); | ||
| static (_, context, _) => LocationsSeeder.SeedAsync(context)); |
There was a problem hiding this comment.
The seeder delegate is discarding the ApplicationStopping cancellation token, but LocationsSeeder.SeedAsync accepts a CancellationToken and uses it for EF async operations. Passing the token through helps ensure startup seeding cancels promptly during shutdown and avoids hanging the host.
| static (_, context, _) => LocationsSeeder.SeedAsync(context)); | |
| static (_, context, cancellationToken) => LocationsSeeder.SeedAsync(context, cancellationToken)); |
| BackgroundDatabaseInitialization.Start<ScanJobsDbContext>( | ||
| app, | ||
| "StorageLens.Services.ScanJobs", | ||
| static (_, context, cancellationToken) => ScanJobsSeeder.SeedAsync(context, cancellationToken)); | ||
| static (_, context, _) => ScanJobsSeeder.SeedAsync(context)); | ||
|
|
There was a problem hiding this comment.
The seeder delegate is discarding the ApplicationStopping cancellation token, but ScanJobsSeeder.SeedAsync accepts a CancellationToken and uses it for EF async operations. Pass the token through so seeding can be cancelled cleanly during shutdown.
| app, | ||
| "StorageLens.Services.FileInventory", | ||
| static (_, context, cancellationToken) => FileInventorySeeder.SeedAsync(context, cancellationToken)); | ||
| static (_, context, _) => FileInventorySeeder.SeedAsync(context)); |
There was a problem hiding this comment.
The seeder delegate is discarding the ApplicationStopping cancellation token, but FileInventorySeeder.SeedAsync accepts a CancellationToken and uses it for EF async operations. Pass the token through to avoid seeding work continuing during shutdown.
| static (_, context, _) => FileInventorySeeder.SeedAsync(context)); | |
| static (_, context, stoppingToken) => FileInventorySeeder.SeedAsync(context, stoppingToken)); |
| app, | ||
| "StorageLens.Services.Duplicates", | ||
| static (_, context, cancellationToken) => DuplicatesSeeder.SeedAsync(context, cancellationToken)); | ||
| static (_, context, _) => DuplicatesSeeder.SeedAsync(context)); |
There was a problem hiding this comment.
The seeder delegate is discarding the ApplicationStopping cancellation token, but DuplicatesSeeder.SeedAsync accepts a CancellationToken and uses it for EF async operations. Pass the token through so seeding can be cancelled promptly during shutdown.
| static (_, context, _) => DuplicatesSeeder.SeedAsync(context)); | |
| static (_, context, cancellationToken) => DuplicatesSeeder.SeedAsync(context, cancellationToken)); |
| At line:1 char:639 | ||
| + ... agelens-dev-key' }; $result += $u = $($_.Exception.Message) } } }; $r ... | ||
| + ~ | ||
| The Try statement is missing its Catch or Finally block. | ||
| At line:1 char:642 | ||
| + ... ens-dev-key' }; $result += $u = $($_.Exception.Message) } } }; $resul ... | ||
| + ~ | ||
| Unexpected token '}' in expression or statement. | ||
| + CategoryInfo : ParserError: (:) [], ParentContainsErrorRecordException | ||
| + FullyQualifiedErrorId : MissingCatchOrFinally | ||
|
|
There was a problem hiding this comment.
This appears to be a local tooling/log artifact (PowerShell parser error) with a very generic filename. Keeping it in the repo/PR makes it hard to understand intent and can create noise in future diffs. Consider removing it from source control or moving it under a clearly named diagnostics/logs directory with context in a README if it’s intentionally preserved.
| At line:1 char:639 | |
| + ... agelens-dev-key' }; $result += $u = $($_.Exception.Message) } } }; $r ... | |
| + ~ | |
| The Try statement is missing its Catch or Finally block. | |
| At line:1 char:642 | |
| + ... ens-dev-key' }; $result += $u = $($_.Exception.Message) } } }; $resul ... | |
| + ~ | |
| Unexpected token '}' in expression or statement. | |
| + CategoryInfo : ParserError: (:) [], ParentContainsErrorRecordException | |
| + FullyQualifiedErrorId : MissingCatchOrFinally | |
| This file previously contained a raw PowerShell parser error log that was | |
| accidentally committed to source control. | |
| The original content looked like a transient error message (for example: | |
| "The Try statement is missing its Catch or Finally block" and | |
| "CategoryInfo : ParserError"), and did not represent intentional source | |
| code or configuration used by this project. | |
| To reduce noise in code review and static analysis (CodeQL) results, and to | |
| avoid confusion about its purpose, the log output has been removed. | |
| If you need to preserve debugging logs in the future, prefer storing them | |
| under a clearly named diagnostics/logs directory with an accompanying | |
| README that explains their intent and context. |
| public enum ScanJobStatus | ||
| { | ||
| Pending = 1, | ||
| Queued = 2, | ||
| Scanning = 3, | ||
| Hashing = 4, | ||
| DetectingDuplicates = 5, | ||
| RefreshingAnalytics = 6, | ||
| Completed = 7, | ||
| Failed = 8, | ||
| PartiallyCompleted = 9, | ||
| Running = 10 | ||
| } |
There was a problem hiding this comment.
These contracts files are indented with hard tabs, while most C# files in this repo (including adjacent changes like StartupDatabaseInitializer.cs) use spaces. Mixing tabs/spaces tends to create inconsistent diffs and formatting across editors. Consider reformatting to the repo’s prevailing spacing style (e.g., 4-space indentation).
| public record ScannerExecuteRequest( | ||
| Guid StorageLocationId, | ||
| Guid ScanJobId, | ||
| string CorrelationId, | ||
| string LocationName, | ||
| string RootPath, | ||
| int BatchSize = 250, | ||
| int ProgressUpdateInterval = 200, | ||
| bool IncludeHidden = false); |
There was a problem hiding this comment.
These contracts files are indented with hard tabs, while most C# files in this repo use spaces. Consider reformatting to consistent spacing to avoid editor-dependent alignment issues and noisy diffs.
| <RepositoryUrl>https://github.com/DeeDee1103/StorageLens</RepositoryUrl> | ||
| <!-- | ||
| Prevent Microsoft.Build.Tasks.Git from trying to read .git/config during build. | ||
| Without this, dotnet build fails when another process (e.g. VS Code) holds a | ||
| lock on .git/config. Source-link is not used in this project, so disabling | ||
| the query has no functional impact. | ||
| --> | ||
| <EnableSourceControlManagerQueries>false</EnableSourceControlManagerQueries> | ||
| </PropertyGroup> |
There was a problem hiding this comment.
This PR changes source/test code and build/deployment config, but there are no accompanying docs/** or docs/CHANGELOG.md updates shown here. The repo’s docs-guard workflow requires at least one docs/** change and a docs/CHANGELOG.md entry whenever src/** or tests/** (or key config like docker-compose/Directory.Build.props) change.
| if (dbContext.Database.IsRelational()) | ||
| { | ||
| await dbContext.Database.MigrateAsync(cancellationToken); | ||
| } |
There was a problem hiding this comment.
PR title/description focus on integration tests + build/compose cleanup, but the diff also introduces a broad shift to EF Core migrations (new Migrations folders across multiple services + StartupDatabaseInitializer now calls MigrateAsync) and adds KeyVault configuration calls in several services. Either the PR description should be updated to reflect these additional, higher-impact changes, or the changes should be split into separate PRs for easier review and safer rollout.
This pull request introduces two main improvements: it fixes a potential build issue with Git source control queries in the build configuration, and it cleans up duplicate service dependencies in the Docker Compose file. Additionally, it enhances local development scripts by adding build and status commands.
Build system improvements:
<EnableSourceControlManagerQueries>false</EnableSourceControlManagerQueries>toDirectory.Build.propsto prevent build failures caused by locked.git/configfiles when SourceLink is not used.Docker Compose cleanup:
- redisentries from thedepends_onlists for multiple services indocker-compose.yml, reducing clutter and potential confusion. [1] [2] [3] [4] [5] [6] [7] [8] [9]Development tooling:
.claude/settings.local.jsonfor runningdotnet buildand echoing the exit code, aiding in local build automation and diagnostics.