feat: run typed real resources in test kernels - #1565
Merged
Conversation
📝 WalkthroughWalkthrough테스트 커널에 typed PostgreSQL/Redis 리소스의 시작, provider 등록, evidence 수집, 격리, 의무 검증 및 정리 기능이 추가되었습니다. 새 ChangesTyped test resources
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant TestKernel
participant PostgreSQLRedisResources
participant CrocoProviders
participant Application
participant Cleanup
TestKernel->>PostgreSQLRedisResources: start containers and collect evidence
PostgreSQLRedisResources->>CrocoProviders: register typed connections
TestKernel->>Application: bootstrap after resource startup
Application->>Cleanup: shutdown
Cleanup->>PostgreSQLRedisResources: dispose clients and containers
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
kang-heewon
force-pushed
the
fix/1482-production-parity-test-kernel
branch
from
July 27, 2026 07:39
03ef264 to
2641fee
Compare
kang-heewon
force-pushed
the
feat/1484-typed-testing-resources
branch
from
July 27, 2026 08:53
0620e99 to
0c4cedf
Compare
📊 Benchmark Results✅ All benchmarks passed
Updated: 2026-07-27T13:41:29.519Z · Commit: 1c0b2db |
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/testing/src/libs/TestKernel.ts (1)
475-480: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick wincleanupOperations 구성 로직이 성공 경로와 catch 경로에 중복됨.
두 블록이
bootstrapCleanup/registeredCleanups/dispose/resourceCleanups를 거의 동일한 순서로 조합하며, 이번 PR에서도resourceCleanups추가를 두 곳 모두에 반영해야 했다. 향후 한쪽만 수정하면 리소스(예: 실제 Postgres 컨테이너) dispose가 정리 시퀀스에서 누락되어 리소스 누수로 이어질 위험이 있다.공통 헬퍼로 추출하는 것을 권장한다.
♻️ 제안하는 리팩터링
+ function buildCleanupOperations( + disposeApp: CrocoApp | undefined, + ): TestKernelCleanupOperation[] { + return [ + ...(bootstrapCleanup ? [bootstrapCleanup] : []), + ...[...registeredCleanups].reverse(), + ...(disposeApp && options.dispose ? [() => options.dispose?.(disposeApp)] : []), + ...[...resourceCleanups].reverse(), + ]; + } ... - const cleanupOperations = [ - ...(bootstrapCleanup ? [bootstrapCleanup] : []), - ...[...registeredCleanups].reverse(), - ...(options.dispose ? [() => options.dispose?.(app as CrocoApp)] : []), - ...[...resourceCleanups].reverse(), - ]; + const cleanupOperations = buildCleanupOperations(app); ... - const cleanupOperations = [ - ...(bootstrapCleanup ? [bootstrapCleanup] : []), - ...[...registeredCleanups].reverse(), - ...(app && options.dispose ? [() => options.dispose?.(app as CrocoApp)] : []), - ...[...resourceCleanups].reverse(), - ]; + const cleanupOperations = buildCleanupOperations(app);Also applies to: 496-501
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/testing/src/libs/TestKernel.ts` around lines 475 - 480, Extract the duplicated cleanupOperations assembly in the success and catch paths of TestKernel into a shared helper, preserving the order of bootstrapCleanup, reversed registeredCleanups, optional dispose, and reversed resourceCleanups. Use the helper in both paths so future cleanup additions remain synchronized.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/testing-resources/src/libs/PostgresResource.ts`:
- Around line 173-207: Update the dispose cleanup flow to preserve every
rollback, pool shutdown, and container-stop failure with bounded logs and
recovery metadata. Add individual diagnostics for activeClient rollback and
activePool.end failures, while recognizing that stopContainer already records
its own cleanup diagnostic so it is not duplicated. When failures are reported,
retain the original failure information and avoid re-wrapping an existing
TestResourceLifecycleProblem; ensure multiple failures are not reduced to
failures[0].
In `@packages/testing-resources/src/libs/RedisResource.ts`:
- Line 169: Update the Redis resource helper around lastError so it returns a
typed failure result instead of throwing the unknown ioredis error directly. In
start, consume that result, add the relevant diagnostic context, and throw only
TestResourceLifecycleProblem; preserve successful startup behavior and ensure no
native Error or other generic value escapes the helper.
In `@packages/testing-resources/src/libs/shared.ts`:
- Around line 35-46: Update appendContainerLogs so the target buffer retains at
most the last 200 non-empty container log lines as each chunk is processed.
After appending lines, remove older entries from target while preserving the
newest 200 for passedDiagnostic and failedDiagnostic consumers.
In `@packages/testing-resources/src/tests/RealResources.spec.ts`:
- Around line 128-137: 정리: RealResources 테스트에서 Drizzle 타입 캐스팅을 제거하세요.
`DrizzleTransactionalEventStore` 구성 주변에서 `db as unknown as
DrizzleTransactionalEventStoreDb`와 `createDrizzleTxAdapter(db as never) as
unknown as ...` 우회를 삭제하고, 실제 `drizzle(connection.pool)` 반환값인 `db`를 직접 사용해 타입이
추론되도록 수정하세요.
In `@packages/testing-resources/src/tests/ResourceConfiguration.spec.ts`:
- Around line 1-15: Update the “testing resource configuration” describe block
in ResourceConfiguration.spec.ts to add a Vitest beforeEach hook that resets the
DI Container before every test. Import the required beforeEach hook and use the
existing Token-based container reset mechanism, ensuring each test starts with
isolated provider state.
In `@packages/testing/src/libs/TestKernel.ts`:
- Around line 406-442: Expose an optional static fidelity mode hint on the
TestResource contract and populate it for resources such as postgresResource and
redisResource. Extend the obligation validation loop before resource.start() to
reject rollback obligations when the hint already indicates rollback, preserving
the existing post-start fidelity check as a fallback. Use the existing
TestKernelResourceFidelityProblem for failures.
---
Outside diff comments:
In `@packages/testing/src/libs/TestKernel.ts`:
- Around line 475-480: Extract the duplicated cleanupOperations assembly in the
success and catch paths of TestKernel into a shared helper, preserving the order
of bootstrapCleanup, reversed registeredCleanups, optional dispose, and reversed
resourceCleanups. Use the helper in both paths so future cleanup additions
remain synchronized.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 011ce58a-9a64-4da3-b0c4-0a6723416039
⛔ Files ignored due to path filters (2)
packages/problems-core/src/generated/problem-code-registry.tsis excluded by!**/generated/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (59)
.changeset/typed-test-resources.md.github/workflows/ci.ymlREADME.mdcroco.arch.jsondocs/package-catalog.jsondocs/package-docs-report.mddocs/problem-code-registry.jsonpackages/docs/astro.config.mjspackages/docs/package.jsonpackages/docs/src/content/docs/api/problems-core/src/classes/Problem.mdpackages/docs/src/content/docs/api/problems-core/src/variables/CROCO_PROBLEM_CODE_REGISTRY.mdpackages/docs/src/content/docs/api/testing-resources/src/classes/TestResourceConfigurationProblem.mdpackages/docs/src/content/docs/api/testing-resources/src/classes/TestResourceLifecycleProblem.mdpackages/docs/src/content/docs/api/testing-resources/src/functions/postgresResource.mdpackages/docs/src/content/docs/api/testing-resources/src/functions/redisResource.mdpackages/docs/src/content/docs/api/testing-resources/src/functions/testResourceProvider.mdpackages/docs/src/content/docs/api/testing-resources/src/type-aliases/PostgresResourceOptions.mdpackages/docs/src/content/docs/api/testing-resources/src/type-aliases/PostgresTestConnection.mdpackages/docs/src/content/docs/api/testing-resources/src/type-aliases/RedisResourceOptions.mdpackages/docs/src/content/docs/api/testing-resources/src/type-aliases/RedisTestConnection.mdpackages/docs/src/content/docs/api/testing-resources/src/type-aliases/ResourceImageOptions.mdpackages/docs/src/content/docs/api/testing-resources/src/type-aliases/TestResourceProvider.mdpackages/docs/src/content/docs/api/testing-resources/src/variables/DEFAULT_POSTGRES_IMAGE.mdpackages/docs/src/content/docs/api/testing-resources/src/variables/DEFAULT_REDIS_IMAGE.mdpackages/docs/src/content/docs/api/testing/src/classes/TestKernel.mdpackages/docs/src/content/docs/api/testing/src/classes/TestKernelResourceFidelityProblem.mdpackages/docs/src/content/docs/api/testing/src/classes/TestKernelResourceNotFoundProblem.mdpackages/docs/src/content/docs/api/testing/src/classes/TestKernelResourceRegistrationProblem.mdpackages/docs/src/content/docs/api/testing/src/type-aliases/StartedTestResource.mdpackages/docs/src/content/docs/api/testing/src/type-aliases/TestKernelResourceEvidence.mdpackages/docs/src/content/docs/api/testing/src/type-aliases/TestKernelResourceObligation.mdpackages/docs/src/content/docs/api/testing/src/type-aliases/TestResource.mdpackages/docs/src/content/docs/api/testing/src/type-aliases/TestResourceDiagnostic.mdpackages/docs/src/content/docs/api/testing/src/type-aliases/TestResourceDiagnosticStage.mdpackages/docs/src/content/docs/api/testing/src/type-aliases/TestResourceFidelity.mdpackages/docs/src/content/docs/api/testing/src/type-aliases/TestResourceIsolation.mdpackages/docs/src/content/docs/api/testing/src/type-aliases/TestResourceMode.mdpackages/docs/src/content/docs/api/testing/src/type-aliases/TestResourceStartContext.mdpackages/docs/src/content/docs/en/reference/problem-recovery-cookbook.mdpackages/docs/tsconfig.typedoc.jsonpackages/testing-resources/README.mdpackages/testing-resources/package.jsonpackages/testing-resources/src/index.tspackages/testing-resources/src/libs/PostgresResource.tspackages/testing-resources/src/libs/RedisResource.tspackages/testing-resources/src/libs/problems.tspackages/testing-resources/src/libs/providers.tspackages/testing-resources/src/libs/shared.tspackages/testing-resources/src/tests/RealResources.spec.tspackages/testing-resources/src/tests/ResourceConfiguration.spec.tspackages/testing-resources/tsconfig.jsonpackages/testing-resources/vitest.config.tspackages/testing/src/index.tspackages/testing/src/libs/TestKernel.tspackages/testing/src/tests/TestKernel.spec.tspnpm-workspace.yamlpublic-api-surface.snapshot.jsonscripts/tests/ci-workflow.spec.tsscripts/workflow-verification-contract.mts
Member
Author
|
Addressed the outside-diff cleanup assembly finding in 4a3ae7a: success and failure paths now share buildCleanupOperations, preserving bootstrap cleanup, reverse registered cleanup, optional app disposal, and reverse resource cleanup order. Focused @croco/testing tests (109), typecheck, and lint pass; pre-push also passed all 232 test tasks and 231 typecheck tasks. |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/testing-resources/src/tests/ResourceConfiguration.spec.ts`:
- Around line 1-3: Reorder the imports in ResourceConfiguration.spec.ts so the
external imports, including vitest and node:stream, appear before the internal
`@croco/framework-context` import; keep the imported symbols unchanged.
- Around line 101-115: Update the “preserves every cleanup failure as structured
evidence” test to use the repository’s asynchronous error-testing convention:
wrap the synchronous throwCleanupFailures invocation in
Promise.resolve().then(...), make the test async, and assert with
expect(...).rejects.toThrow while preserving the existing structured error
expectations.
In `@pnpm-workspace.yaml`:
- Line 49: pnpm-workspace.yaml의 testcontainers>archiver 오버라이드를 제거하거나 CommonJS를
지원하는 ^7.0.1로 변경하세요. testcontainers@12.0.4와 Jest의 tar 복사 경로가 archiver 8.0.0을 로드하지
않도록 유지하고, CVE 패치 목적이라면 호환되는 testcontainers 버전도 함께 조정하세요.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 94c24637-d680-4562-8f33-ac5b16caabc2
⛔ Files ignored due to path filters (2)
packages/problems-core/src/generated/problem-code-registry.tsis excluded by!**/generated/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (16)
docs/problem-code-registry.jsonpackages/docs/src/content/docs/api/problems-core/src/variables/CROCO_PROBLEM_CODE_REGISTRY.mdpackages/docs/src/content/docs/api/testing-resources/src/classes/TestResourceLifecycleProblem.mdpackages/docs/src/content/docs/api/testing/src/type-aliases/TestResource.mdpackages/docs/src/content/docs/en/guides/getting-started.mdxpackages/docs/src/content/docs/en/index.mdxpackages/docs/src/content/docs/en/reference/problem-recovery-cookbook.mdpackages/testing-resources/src/libs/PostgresResource.tspackages/testing-resources/src/libs/RedisResource.tspackages/testing-resources/src/libs/problems.tspackages/testing-resources/src/libs/shared.tspackages/testing-resources/src/tests/RealResources.spec.tspackages/testing-resources/src/tests/ResourceConfiguration.spec.tspackages/testing/src/libs/TestKernel.tspackages/testing/src/tests/TestKernel.spec.tspnpm-workspace.yaml
kang-heewon
force-pushed
the
feat/1484-typed-testing-resources
branch
from
July 27, 2026 13:32
eeca7c0 to
98ccf29
Compare
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.
Outcome
@croco/testingTestKernel can now start typed resources before production application bootstrap, inject their connections into kernel-scoped Croco DI, report structured fidelity and lifecycle evidence, reject rollback resources for commit-semantic obligations, and dispose resources exactly once after application shutdown.A new optional
@croco/testing-resourcespackage owns digest-pinned PostgreSQL and Redis Testcontainers without adding Docker dependencies to@croco/testing. PostgreSQL supports rollback, commit, and migration modes with database-per-worker isolation; Redis uses prefix-per-test isolation. Startup, migration, health, provider registration, and cleanup failures retain bounded container logs and recovery metadata.CI now runs a credential-free real-resource suite covering Drizzle rollback isolation, migration from empty, transactional outbox commit semantics, concurrent PostgreSQL/Redis isolation, and cleanup after provider registration failure.
Fixes #1484
Dependency
The production-parity TestKernel dependency from #1559 is merged. This branch is rebased directly onto the current
trunkcommitb07ae3a.Verification
pnpm --filter @croco/testing-resources test:real— 5/5 passed after the rebase; the suite had also passed three consecutive runs during readiness hardening@croco/testingfocused suite — 109/109 passed, including 20 TestKernel cases@croco/testing-resourcesunit suite — 9/9 passedpnpm docs:api:check— 115/115 tasks; generated API docs matchedpnpm problem-registry:check— 510 codes matched 510 discoveriespnpm public-api:check— 114 package snapshots matchedtrunkReview gates
@croco/testing; Docker clients and resource implementations stay in the optional package; CI commands are explicitly allowlisted.Residual risk
Real-resource tests require a healthy Docker-compatible daemon and first-run image pulls. Custom image overrides and provider factories remain caller-controlled; unpinned images require an explicit local-only opt-out, and provider failures are surfaced as startup Problems after partial resources are closed.
Summary by CodeRabbit