Run user-facing indexer tests from config + source strings - #1488
Merged
Conversation
Run handler source in-process against an isolated per-instance registration so independent test indexers over different configs don't share the global registry. - EnvioGlobal: registrationScopeOverride slot resolved ahead of the global activeRegistration. - HandlerRegister: make/finish/withScope build and run a registration record detached from the global; isWildcard/getSimulateOnEventRegistrations take it explicitly; a scoped registration for an unconfigured event throws. - SimulateItems.patchConfig threads the active registration through simulate. - Main: indexer getters resolve the active scope's config when set. - TestIndexer: extract parameterized make; public createTestIndexer wraps it. - InternalTestIndexer.createTestIndexer writes the handler source to a unique temp file and imports it under the scope, guarding stray pre-registrations. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014pvXzzaAX22qYRmGCKbbL6
Contributor
📝 WalkthroughWalkthroughChangesInternal test indexers now type-check and execute optional user test modules through temporary files and asynchronous Vitest suites. New fixtures cover token balances, simulation behavior, block parameters, wildcard routing, and slot resumption. Handler registration reports missing configured contracts or events. ChangesIndexer testing and validation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Fixture
participant InternalTestIndexer
participant TypeChecker
participant HandlerRegister
participant Vitest
Fixture->>InternalTestIndexer: Provide config, handlers, and test source
InternalTestIndexer->>TypeChecker: Validate virtual handler and test modules
InternalTestIndexer->>HandlerRegister: Start registration with parsed config
InternalTestIndexer->>Vitest: Import temporary modules
Vitest->>HandlerRegister: Register event handlers
Vitest->>Vitest: Execute simulation and assertion cases
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e885fcf4e1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Replace the scoped-registration approach with `defineIndexerTest`, which evaluates a real user-facing test module against a config parsed from YAML. Priming the config (`Config.prime`) is enough to make the public `createTestIndexer` from "envio" work with no project on disk, so handlers register through the normal global registry and none of the previous envio production changes are needed. This reverts the registration scope override, `withScope`, the indexer getter changes, and the SimulateItems/TestIndexer threading. `handlers` and `test` are ordinary user modules, type-checked against the config's generated types and then executed. The test module's `it()` calls register into the calling file's suite, so failures get real vitest names, code frames and diffs; the suite is named for the `defineIndexerTest` call site, and setup failures surface as a named test rather than a collection crash. Generated modules persist for the run so code frames resolve, and `globalSetup` clears them beforehand. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014pvXzzaAX22qYRmGCKbbL6
A registration that matches no configured chain can never be dispatched, so the handler silently never runs. Report it at the registration call site instead, naming the configured contracts or events so the typo is obvious. Applies to onEvent, onInstruction and contractRegister alike. Also move defineIndexerTest into InternalTestIndexer.res — only the TS compiler harness needs to be TypeScript. The fixture file stays .test.ts since collection-time registration needs top-level await. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014pvXzzaAX22qYRmGCKbbL6
One entry point: `fromUserApi(~test)` type-checks and runs a user-facing test module, instead of a separate `defineIndexerTest`. Vitest awaits a suite callback while collecting, so registering the imports inside `describe` keeps `fromUserApi` synchronous — no top-level await, which means fixtures are ordinary `_test.res` files rather than `.test.ts`. Type errors and setup failures are reported as a named test rather than a collection crash, since the parse already succeeded and the config is still returned. Parse-only callers keep throwing, as their assertions expect. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014pvXzzaAX22qYRmGCKbbL6
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/envio-tests/test/helpers/InternalTestIndexer.res (2)
34-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStale function name in comment.
The comment refers to
defineIndexerTest, but the actual public entrypoint in this diff isfromUserApi. This looks like a leftover from an earlier naming iteration and could mislead readers about which call this stack-walk is attributing.✏️ Suggested fix
-// `file:line` of the `defineIndexerTest` call — the first stack frame outside +// `file:line` of the `fromUserApi` call — the first stack frame outside // this helper — so a failing suite names the fixture that produced it.🤖 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/envio-tests/test/helpers/InternalTestIndexer.res` around lines 34 - 46, Update the explanatory comment above callSite to refer to the actual public entrypoint fromUserApi instead of defineIndexerTest, leaving the stack-walking implementation unchanged.
48-48: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicated
tmpDirpath computation across ReScript and TS. Both files independently re-derive the sametest/.tmplocation relative totest/helpers/; they currently agree, but nothing keeps them in sync if the directory name or relative depth changes.
packages/envio-tests/test/helpers/InternalTestIndexer.res#L48: consider sourcing the.tmpdirectory name from a single constant (e.g. exported fromTypeChecker.tsor a small shared JSON/config) that both this file andglobalSetup.tsimport.packages/envio-tests/test/helpers/globalSetup.ts#L10: same — derive from the same shared constant instead of recomputing the relative path independently.🤖 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/envio-tests/test/helpers/InternalTestIndexer.res` at line 48, Centralize the temporary directory path used by InternalTestIndexer and globalSetup instead of computing it independently. Update packages/envio-tests/test/helpers/InternalTestIndexer.res:48 and packages/envio-tests/test/helpers/globalSetup.ts:10 to consume one shared exported constant or configuration value for the test/.tmp location, preserving the existing resolved directory.
🤖 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.
Nitpick comments:
In `@packages/envio-tests/test/helpers/InternalTestIndexer.res`:
- Around line 34-46: Update the explanatory comment above callSite to refer to
the actual public entrypoint fromUserApi instead of defineIndexerTest, leaving
the stack-walking implementation unchanged.
- Line 48: Centralize the temporary directory path used by InternalTestIndexer
and globalSetup instead of computing it independently. Update
packages/envio-tests/test/helpers/InternalTestIndexer.res:48 and
packages/envio-tests/test/helpers/globalSetup.ts:10 to consume one shared
exported constant or configuration value for the test/.tmp location, preserving
the existing resolved directory.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3e2711c0-8817-4e1a-be1d-3c136739738d
📒 Files selected for processing (8)
packages/envio-tests/.gitignorepackages/envio-tests/test/HandlerRegister_test.respackages/envio-tests/test/TokenIndexer_test.respackages/envio-tests/test/helpers/InternalTestIndexer.respackages/envio-tests/test/helpers/TypeChecker.tspackages/envio-tests/test/helpers/globalSetup.tspackages/envio-tests/vitest.config.tspackages/envio/src/HandlerRegister.res
Chain 1 and 137 resolve a HyperSync endpoint automatically, so the rpc block was noise in configs whose tests are about handlers, entities, addresses or event signatures. Kept where the block is the subject: env-var interpolation, RPC URL and WebSocket validation, dual-source rejection, "unavailable via RPC" field selections, and chain 1337, which has no automatic endpoint. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014pvXzzaAX22qYRmGCKbbL6
The imported test module only registers if vitest awaits the suite callback during collection. If that ever stops holding the suite shrinks silently, which no reporter flags, so register a sentinel test outside the callback that asserts the import ran. Move the scenario tests that cover test-indexer mechanics rather than a specific project's handlers, which no longer need a codegen'd project and now get their handler and test source type-checked: - SimulateDynamicAddress (7) — contractRegister, dynamic addresses, repeated process(), address normalization, dead simulate inputs - OptionalBlockParams (9) — block-range defaults, progress carryover, auto-exit, invalid ranges - WildcardSimulate (2) — wildcard routing regardless of srcAddress - SlotResume (1, svm) — onSlot-only indexer progressing after resume Left behind: CustomSelection asserts generated ReScript types, and LoadLinkedEntities and EventHandler.test.ts exercise test_codegen's own handlers — none are test-indexer mechanics. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014pvXzzaAX22qYRmGCKbbL6
checkHandlerTypes lost its callers when checkSources landed, the publicConfigJson field on parsed was never read outside the helper, and the name override was never passed. Trim the comments that outlived the code they described. Unifying the two type-check paths means the thrown prefix is now "Type errors:" for handlers as well, and the throw assertions move to main's stricter toThrowErrorEqual. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014pvXzzaAX22qYRmGCKbbL6
DZakh
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c1a0254364
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Nothing matched on any chain, so the callback could never be dispatched. | ||
| // Reported at the registration call site, where the stack still points at the | ||
| // offending `onEvent`/`contractRegister`. | ||
| if !matched.contents { |
There was a problem hiding this comment.
When a contract or event exists only on a chain marked skip: true while another chain remains active, to_public_config_json removes that chain from the runtime chainMap but retains the global contract definitions and their handler files. The handler is therefore still loaded, yet this check sees no active-chain match and aborts indexer startup. This breaks the supported workflow of temporarily disabling a chain without deleting its handlers; no-match registrations should remain lenient for the global loader or account for skipped-chain definitions.
Useful? React with 👍 / 👎.
| // sibling worker's files mid-run. | ||
| export default function setup(): void { | ||
| const tmpDir = path.join(path.dirname(fileURLToPath(import.meta.url)), "..", ".tmp"); | ||
| fs.rmSync(tmpDir, { recursive: true, force: true }); |
There was a problem hiding this comment.
When two Vitest commands run concurrently in the same checkout, both use this shared test/.tmp directory and each global setup recursively deletes it. One run can therefore remove handler or test modules after the other run writes them but before its deferred describe callback imports them, producing intermittent module-not-found collection failures; use a per-run directory or coordinate cleanup instead of deleting the shared directory.
Useful? React with 👍 / 👎.
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
2 participants
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.
Summary
InternalTestIndexer.fromUserApigains a~testargument: a real user-facing test module, type-checked against the config's generated types and then executed. Combined with~handlers, a fixture can exercise an indexer end to end from YAML alone — no codegen'd project on disk.How it works
envio codegen.handlersandtestsources against those declarations, so they are bound to this fixture's chains, contracts, events and entities.Config.primeputs the parsed config whereConfig.load()finds it, which is what lets the realcreateTestIndexerfromenviorun with no project on disk.test/.tmpand imported. Handlers register through the ordinary global registry.describecallback, which vitest awaits while building the suite tree, so the test module'sit()calls become real tests with proper names, code frames and diffs. Nothing needs to be awaited by the caller, so fixtures stay ordinary_test.resfiles.Setup failures (type errors, a duplicate fixture in one file) are reported as a named test rather than a collection crash, and a sentinel test asserts the imports actually ran — if vitest ever stops awaiting suite callbacks the suite would otherwise shrink silently.
Production change
One, independent of the test machinery: registering a handler for an event that no configured chain defines now throws at the registration call site instead of silently never running. Applies to
onEvent,onInstructionandcontractRegister.This is breaking for a project with a dead handler — one registered for a contract absent from every chain, or an event since removed from config — which goes from silently ignored to a startup failure. Worth a changelog note, and it argues for a minor rather than a patch. Happy to split it into its own PR if preferred.
Migrated scenarios
Scenario tests covering test-indexer mechanics (rather than a specific project's handlers) moved to fixtures. They no longer need a codegen'd project, and their handler and test source are now type-checked:
SimulateDynamicAddress(7)contractRegister, dynamic addresses, repeatedprocess(), address normalization, dead simulate inputsOptionalBlockParams(9)WildcardSimulate(2)srcAddressSlotResume(1, svm)onSlot-only indexer progressing after a resumeLeft in place deliberately:
CustomSelectionasserts generated ReScript types (which this path doesn't cover), andLoadLinkedEntities/EventHandler.test.tsexercise test_codegen's own handler graph.Also dropped
rpc:blocks from configs whose tests aren't about sources — chains 1 and 137 resolve a HyperSync endpoint automatically. Kept wherever the block is the subject (env interpolation, URL validation, "unavailable via RPC" field selections, and chain 1337, which has no automatic endpoint).Known sharp edge
handlersandtestare ReScript template strings, so a literal${in the TS source must be escaped as\${. Two of the migrated fixtures build entity ids from template literals and do exactly that.Verification
envio-tests363 ·test_codegen606 ·svm_test3 — all passing, plus green CI.Summary by CodeRabbit
New Features
Bug Fixes
Tests
https://claude.ai/code/session_014pvXzzaAX22qYRmGCKbbL6