fix(api): the container could not start, and nothing tested that it could - #25
Merged
Conversation
…ould A04 merged with green CI and the deploy failed at boot. AccessModule injected IngestionService, which GitHubModule did not export, and a RateBudget, which is an interface and therefore has no runtime token for Nest to resolve. Production stayed on the previous build, so the site never went down, but A04 was not deployed. The wiring is a one-line export and an @Inject. The reason it reached production is the part worth fixing: every suite in this app constructs classes directly with fakes, which is the right way to test behaviour and says nothing about whether the container can build them. The dependency graph is a real artifact and had no test at all. app.module.test.ts compiles the actual graph, in both shapes an operator can deploy: with and without a delegated-query key, which are different graphs because Authorizations is null in one of them. It also pins the route list, since a module that fails to register takes its controllers with it silently while /health keeps answering. That test could not have been written before this commit. Vitest transpiles with esbuild, which does not emit decorator metadata, so Nest resolved every constructor parameter as undefined and the failure looked like a bug inside the service. The API's vitest config now uses SWC with decoratorMetadata, which is why this class of test was missing rather than merely forgotten. Verified by reverting each half of the original break: dropping the export, dropping the @Inject, and unregistering AccessModule each turn the new test red. Also removed a DatabaseModule import added while diagnosing, along with the comment claiming it was required. DatabaseModule is @global, so the comment was false, and a false reason in a comment is worse than none.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A04 merged green and the deploy failed at boot. Production stayed on the previous build, so nothing went down, but A04 is not live.
What broke
AccessModuleinjectedIngestionService, whichGitHubModuledid not export, and aRateBudget, which is an interface and has no runtime token for Nest to resolve.Why it reached production
Every suite in this app constructs classes directly with fakes. That is the right way to test behaviour and it says nothing about whether the container can build them. The dependency graph is a real artifact and had no test.
app.module.test.tscompiles the actual graph, in both shapes an operator can deploy: with and without a delegated-query key, which are genuinely different graphs becauseAuthorizationsis null in one of them. It also pins the route list, because a module that fails to register takes its controllers with it silently while/healthkeeps answering, so a smoke test on one route proves nothing.Why the test did not exist
Vitest transpiles with esbuild, which does not emit decorator metadata, so Nest resolved every constructor parameter as
undefinedand the failure looked like a bug inside the service. The API's vitest config now uses SWC withdecoratorMetadata. This class of test was unavailable rather than merely forgotten.Verified
Reverting each half of the original break turns the new test red: dropping the export, dropping the
@Inject, and unregisteringAccessModule.