Skip to content

test(checkpoint): replace vacuous interface tests with behavioural co… - #228

Open
N-otorious wants to merge 1 commit into
Miracle656:mainfrom
N-otorious:test/checkpoint-behavioural
Open

N-otorious wants to merge 1 commit into
Miracle656:mainfrom
N-otorious:test/checkpoint-behavioural

Conversation

@N-otorious

Copy link
Copy Markdown
Contributor

Replace vacuous checkpoint tests with behavioural coverage

Removes three interface-shape tests that exercised no runtime code and replaces them with 19 behavioural tests
against a mocked Prisma client, covering hasCheckpoint, getLastCheckpoint, commitBatch (fresh insert, replayed
batchId, transaction failure), and updateAccountSummaries. checkpoint.ts moves from 0% to 100%
statement/function/line coverage.

Note: commitBatch and hasCheckpoint are not yet wired into startIndexer().

…verage

Removes three interface-shape tests that exercised no runtime code.
Adds 19 behavioural tests against a mocked Prisma client covering
hasCheckpoint, getLastCheckpoint, commitBatch (fresh insert, replayed
batchId, transaction failure), and updateAccountSummaries.

checkpoint.ts: 0% -> 100% statement/function/line coverage.

Note: commitBatch and hasCheckpoint are not yet wired into startIndexer().
@Miracle656

Copy link
Copy Markdown
Owner

#229 has merged, and this is the same work on the same file — a second, longer version of src/__tests__/checkpoint.test.ts. Merging this now would replace what just landed (274 insertions against 184 deletions on that file), not add to it.

I am not going to close one of your PRs for you. Tell me which you want:

If this version has assertions #229 lacks, the second option is the better one and I would like to see them. From a skim the extra length is mostly the file header and structure rather than new cases, but you know the difference better than I do.

For what it is worth, the merged version earns its place for the right reason: it asserts what was passed, not just what came back —

expect(callArg.where.network_batchId.network).toBe("mainnet")

That is the assertion that catches a helper returning the right shape from the wrong network. We fixed exactly that class of bug in the indexer today: mainnet was reading a testnet contract, found no events, and reported itself healthy.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants