test: add Neo4j integration tests for workflow run creation #1455
Conversation
WalkthroughAdds an integration test suite for WorkflowRun creation against the Neo4j StorageAdapter and a test utility to clean up WorkflowRun-related test data (repositories, issues, users, github users, commits, and runs). Changes
Possibly related issues
✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Comment |
youngchingjui
left a comment
youngchingjui
left a comment
There was a problem hiding this comment.
@coderabbitai is this safe? what are some concerns that I should think about?
youngchingjui
left a comment
youngchingjui
left a comment
There was a problem hiding this comment.
@coderabbitai review
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
__tests__/shared/adapters/neo4j/StorageAdapter.workflowRun.create.neo4j.test.ts (1)
39-48: Consider deferring dataSource creation tobeforeAll.
createTestDataSource()at module scope throws during test file loading if env vars are missing, which can interfere with test discovery. Moving it tobeforeAllwould allow the test runner to list tests even when the database is unavailable.🔎 Proposed refactor
describe("StorageAdapter - WorkflowRun creation", () => { let adapter: StorageAdapter - const dataSource = createTestDataSource() + let dataSource: ReturnType<typeof createTestDataSource> const runIds: string[] = [] const repoIds: string[] = [] const issues: { repoFullName: string; number: number }[] = [] const userIds: string[] = [] const githubUserIds: string[] = [] const commitShas: string[] = [] beforeAll(async () => { + dataSource = createTestDataSource() await verifyConnection(dataSource) adapter = new StorageAdapter(dataSource) })
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
__tests__/shared/adapters/neo4j/StorageAdapter.workflowRun.create.neo4j.test.ts__tests__/shared/adapters/neo4j/testUtils.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js,jsx,mts,mjs}
📄 CodeRabbit inference engine (.cursor/rules/code-structure.mdc)
Avoid barrel files: do not create files whose sole purpose is to re-export symbols from multiple modules; prefer importing from original module paths to maintain clear dependency graphs and better tree-shaking
Files:
__tests__/shared/adapters/neo4j/testUtils.ts__tests__/shared/adapters/neo4j/StorageAdapter.workflowRun.create.neo4j.test.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: youngchingjui
Repo: youngchingjui/issue-to-pr PR: 1282
File: shared/src/ports/events/publisher.ts:16-19
Timestamp: 2025-09-22T09:24:26.840Z
Learning: youngchingjui prefers to manage PR scope tightly and defer technical debt improvements to future PRs rather than expanding the current PR's scope, even for related consistency issues.
📚 Learning: 2026-01-02T09:21:36.652Z
Learnt from: youngchingjui
Repo: youngchingjui/issue-to-pr PR: 1450
File: shared/src/adapters/neo4j/types.ts:12-17
Timestamp: 2026-01-02T09:21:36.652Z
Learning: In the Neo4j data model at shared/src/adapters/neo4j/types.ts, the WorkflowRun node intentionally does not include a `state` field. The workflow run state is derived from the chain of events rather than being stored directly on the WorkflowRun node, following an event sourcing pattern where state is computed by replaying events.
Applied to files:
__tests__/shared/adapters/neo4j/testUtils.ts__tests__/shared/adapters/neo4j/StorageAdapter.workflowRun.create.neo4j.test.ts
📚 Learning: 2025-09-22T09:34:43.580Z
Learnt from: youngchingjui
Repo: youngchingjui/issue-to-pr PR: 1282
File: lib/neo4j/convert.ts:69-74
Timestamp: 2025-09-22T09:34:43.580Z
Learning: The file lib/neo4j/convert.ts is being moved away from/deprecated, so fixes in this file may not be worth implementing since it will be replaced.
Applied to files:
__tests__/shared/adapters/neo4j/testUtils.ts
📚 Learning: 2025-09-22T09:28:41.886Z
Learnt from: youngchingjui
Repo: youngchingjui/issue-to-pr PR: 1282
File: lib/adapters/PersistingEventBusAdapter.ts:55-58
Timestamp: 2025-09-22T09:28:41.886Z
Learning: The project is migrating away from neo4j/services/event adapters, so issues in files like lib/adapters/PersistingEventBusAdapter.ts that use these adapters may be temporary and lower priority.
Applied to files:
__tests__/shared/adapters/neo4j/testUtils.ts
📚 Learning: 2026-01-02T09:20:53.213Z
Learnt from: youngchingjui
Repo: youngchingjui/issue-to-pr PR: 1450
File: shared/src/adapters/neo4j/types.ts:272-283
Timestamp: 2026-01-02T09:20:53.213Z
Learning: The plan schema and llmResponseWithPlanSchema in shared/src/adapters/neo4j/types.ts are being migrated away/deprecated, so schema inconsistencies related to plan support do not need to be fixed.
Applied to files:
__tests__/shared/adapters/neo4j/testUtils.ts
🧬 Code graph analysis (1)
__tests__/shared/adapters/neo4j/testUtils.ts (1)
shared/src/adapters/neo4j/dataSource.ts (1)
Neo4jDataSource(10-13)
🔇 Additional comments (4)
__tests__/shared/adapters/neo4j/testUtils.ts (1)
78-166: LGTM - Clean and comprehensive test data cleanup utility.The function correctly:
- Uses parameterized Cypher queries to avoid injection
- Handles session cleanup in a
finallyblock- Skips empty arrays to avoid unnecessary queries
- Uses
DETACH DELETEto remove nodes with their relationshipsNote: Individual queries are not wrapped in a transaction, so partial cleanup may occur if an error is thrown mid-execution. This is acceptable for test cleanup purposes where atomicity isn't critical.
__tests__/shared/adapters/neo4j/StorageAdapter.workflowRun.create.neo4j.test.ts (3)
29-37: LGTM - Helper functions are well-designed.The
toNumberhelper correctly handles Neo4j integer types which can exceed JavaScript's safe integer range. ThebuildCommitShahelper generates valid 40-character hex strings suitable for test commit SHAs.
67-134: LGTM - Comprehensive test for user actor workflow run creation.The test properly:
- Generates unique identifiers to avoid test collisions
- Registers test data for cleanup before the operation (ensuring cleanup even if creation fails)
- Verifies all expected relationships and node properties
- Handles Neo4j integer types correctly with
toNumber()
136-224: LGTM - Thorough test for webhook actor with commit data.The test validates the more complex flow with:
- Webhook actor type correctly persisting as
GithubUsernode- Commit node creation with expected properties
BASED_ON_COMMITrelationship from WorkflowRunIN_REPOSITORYrelationship from Commit to RepositoryGood coverage of the optional commit payload path.
Motivation
StorageAdapter.workflow.run.createto increase confidence in DB writes.Description
__tests__/shared/adapters/neo4j/StorageAdapter.workflowRun.create.neo4j.test.tswith two integration tests that create workflow runs and assert repository/issue/actor/commit nodes and relationships.cleanupWorkflowRunTestDatahelper to__tests__/shared/adapters/neo4j/testUtils.tsto remove test-createdWorkflowRun,Repository,Issue,User,GithubUser, andCommitnodes after tests.StorageAdapterto exercise the creation code paths for bothuserandwebhookactor types and for optionalcommitpayloads.buildCommitShaand numeric conversion logic to normalize Neo4j integer values in assertions.Testing
pnpm test:neo4jand.env.localcontainingNEO4J_URI,NEO4J_USER, andNEO4J_PASSWORD.verifyConnectioncheck that will fail if the database is not reachable, preventing false positives.Codex Task
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.