test(api): cover Production fail-fast on missing health-check connection string#519
Conversation
…ion string Adds a regression test for the #210 fail-fast in Api/Program.cs: booting the host in Production without ConnectionStrings:TrueMain must throw at startup rather than silently dropping the Npgsql "ready" check (which would leave /readyz green while Postgres is unreachable). No Postgres fixture is needed — the host throws during service registration — so the class stays out of the serialised integration collection.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| // configuration up to the health-check branch. The discard keeps this an | ||
| // Action: a bare property access can't be a statement-bodied lambda, so | ||
| // dropping it would infer Func<IServiceProvider> and bind a different | ||
| // assertion overload that doesn't observe the startup throw. | ||
| var act = () => _ = factory.Services; | ||
|
|
There was a problem hiding this comment.
NON BLOQUANT — factory.CreateClient() déclencherait EnsureServer() de la même façon, retourne HttpClient (type concret ≠ IServiceProvider), donc la lambda () => factory.CreateClient() est inférée Action sans avoir besoin du discard ni du commentaire explicatif. C'est le pattern utilisé dans CorsStartupIntegrationTests et serait plus lisible ici :
| // configuration up to the health-check branch. The discard keeps this an | |
| // Action: a bare property access can't be a statement-bodied lambda, so | |
| // dropping it would infer Func<IServiceProvider> and bind a different | |
| // assertion overload that doesn't observe the startup throw. | |
| var act = () => _ = factory.Services; | |
| var act = () => factory.CreateClient(); |
La logique est identique : les deux chemins remontent l'exception levée à la ligne 52 de Program.cs pendant la phase de construction du host, bien avant app.Run().
There was a problem hiding this comment.
Review
Le PR est propre. Un seul fichier, aucun changement de code applicatif — uniquement un nouveau test qui couvre le fail-fast deja en place.
Ce qui est verifie :
- Le throw a la ligne 52 de Program.cs se produit bien avant builder.Build() (pendant l enregistrement des services), donc il precede les guards CORS (ValidateOnStart), AddDbContext, AddMongoLogging, etc. — la classe n a pas besoin d injecter Cors:Origins ni de Postgres.
- L exclusion de [Collection(IntegrationCollection.Name)] est intentionnelle et correcte : pas de fixture Postgres requise, pas de risque de contention sur le reset de la base.
- Le pattern WithMessage avec la sous-chaine correspond bien au message reel du throw.
- La gestion using var factory garantit le Dispose() meme lorsque l acces a Services leve l exception ; WebApplicationFactory.Dispose() gere proprement le cas ou _host est null.
- La reference a TrueMainWebApplicationFactory.DefaultOpsApiKey pour satisfaire [MinLength(32)] est correcte (meme pattern que CorsStartupIntegrationTests).
Suggestions (non bloquantes) :
- Voir le commentaire inline sur les lignes 26-31 : factory.CreateClient() est un declencheur plus simple et coherent avec CorsStartupIntegrationTests — il evite le discard _ = et le long commentaire explicatif.
Adopts review feedback: factory.CreateClient() triggers host build the same way and infers a clean Action lambda, matching CorsStartupIntegrationTests — drops the discard and the explanatory comment.
| public void Build_ShouldThrow_WhenConnectionStringMissingInProduction() | ||
| { | ||
| using var factory = new MissingConnectionStringFactory(); | ||
|
|
There was a problem hiding this comment.
NON BLOQUANT — CorsStartupIntegrationTests utilise le pattern async Task + await using var factory pour laisser WebApplicationFactory<T> effectuer son nettoyage asynchrone. Ici la factory ne démarre jamais (elle lève pendant l'enregistrement des services), donc la version synchrone fonctionne en pratique — mais pour rester cohérent avec le reste de la suite, on pourrait écrire :
| public async Task Build_ShouldThrow_WhenConnectionStringMissingInProduction() |
…et remplacer using var factory par await using var factory à la ligne suivante.
There was a problem hiding this comment.
Review
Le PR ajoute un unique fichier de test couvrant le garde-fou fail-fast introduit par #210. La logique est correcte et bien documentée.
Ce qui a été vérifié :
- Le wildcard
"*readiness health check can be registered in Production*"correspond exactement au message levé dansApi/Program.cs(lignes 52-54). ✓ - L'environnement "Production" déclenche bien la branche
else if— l'ordre dans Program.cs garantit que ce throw intervient avant toute validation CORS. ✓ - Mettre
ConnectionStrings:TrueMain = nulldansAddInMemoryCollection(source la plus prioritaire) fait retournernullàGetConnectionString, satisfaisantstring.IsNullOrWhiteSpace. ✓ Ops:ApiKeyinjecté pour éviter que la validation[MinLength(32)]d'OpsOptions ne pollue l'exception attendue. ✓- Absence intentionnelle de
[Collection(IntegrationCollection.Name)]: aucun accès à Postgres n'est requis puisque la levée survient pendant l'enregistrement des services. ✓
Suggestions (non bloquantes) :
- Voir commentaire inline ligne 24 : rendre le test
async Task+await using var factorypour rester cohérent avec le pattern deCorsStartupIntegrationTests.
Adopts review feedback: matches CorsStartupIntegrationTests' async cleanup pattern (async Task + await using var factory).
| // Null out the connection string in the highest-precedence source | ||
| // so any value inherited from earlier providers is overridden, and | ||
| // satisfy OpsOptions' [MinLength(32)] so the missing-connection | ||
| // branch is the only thing left to fail the boot. |
There was a problem hiding this comment.
NON BLOQUANT — commentaire légèrement trompeur
Le commentaire affirme que fournir Ops:ApiKey est nécessaire pour « laisser le missing-connection branch être le seul échec au boot ». Ce n'est pas tout à fait exact : le throw au fail-fast (Program.cs ligne 52) se produit pendant l'enregistrement des services, soit avant l'appel à builder.Build(). Or, ValidateOnStart() sur OpsOptions ne s'exécute qu'après Build(), lors du démarrage du host. En environnement Production sans connection string, le throw de la garde santé se déclenche donc systématiquement avant que la validation des options soit atteinte — Ops:ApiKey est de la défense en profondeur, pas une contrainte fonctionnelle pour isoler la bonne branche.
Suggestion (non obligatoire) :
| // branch is the only thing left to fail the boot. | |
| // Null out the connection string in the highest-precedence source | |
| // so any value inherited from earlier providers is overridden. | |
| // Ops:ApiKey is provided defensively (satisfies [MinLength(32)]) even | |
| // though the health-check throw fires during service registration — before | |
| // builder.Build() and therefore before ValidateOnStart() runs for any option. |
Addresses review feedback: the health-check throw fires during service registration, before Build() and thus before any ValidateOnStart — so Ops:ApiKey is defense-in-depth, not functionally required to isolate the branch.
There was a problem hiding this comment.
Review
Le test couvre exactement le guard-fail qu'il vise : démarrage en Production sans ConnectionStrings:TrueMain → InvalidOperationException. La logique est saine, le pattern correspond à celui de CorsStartupIntegrationTests, et les choix de conception sont correctement documentés (isolation hors collection, Ops:ApiKey défensif, scope Production uniquement).
Points vérifiés
- Message pattern :
"*readiness health check can be registered in Production*"correspond exactement à la chaîne lancée ligne 52-54 deApi/Program.cs. ✓ - Isolation : absence de
[Collection(IntegrationCollection.Name)]intentionnelle — le host lève l'exception pendant l'enregistrement des services, avant tout accès Postgres. ✓ - Précédence de config :
AddInMemoryCollectionajouté en dernier → priorité maximale, écrase toute valeur héritée. ✓ async Task+ assertion synchrone :await usingjustifie leasync;CreateClient()est synchrone donc.Should().Throw<>()est correct — même pattern queCorsStartupIntegrationTests. ✓- Primary constructor vide : syntaxe C# 12 valide, cohérente avec la base de code. ✓
Aucun point bloquant.
2 participants
Contexte
Suite à #210 (déjà corrigé sur
developpar 6418a25 + bc76c44). Le code du fail-fast est en place, mais aucun test ne le couvrait : si quelqu'un casse/supprime ce garde-fou plus tard, rien ne le signalerait.Changement
backend/tests/TrueMain.IntegrationTests/HealthCheckBootstrapIntegrationTests.cs(test uniquement, aucun changement de code applicatif) : démarre le host en environnementProductionsansConnectionStrings:TrueMainet vérifie qu'uneInvalidOperationExceptionest levée au démarrage (au lieu de laisser/readyzvert alors que Postgres est injoignable).Le test cible uniquement
Production, le seul environnement où le fail-fast s'applique (tous les déploiements tournent en Production — cf.compose*.yaml). Pas de fixture Postgres requise : le host lève l'exception pendant l'enregistrement des services, bien avant de toucher la base — la classe reste donc hors de la collection d'intégration sérialisée.Notes
dotnetn'étant pas disponible dans l'environnement d'exécution, le build/les tests n'ont pas pu être lancés localement — la CI les validera.https://claude.ai/code/session_01B1tjxyzK9zDzMWcD8aitxX
Generated by Claude Code