[Chronik] Add integration test for block_and_blocks endpoint into bitcoinsuite-chronik-client
Needs RevisionPublic
Actions

Authored by hazzarust on Sat, Feb 22, 14:02.

Details

Reviewers

Group Reviewers

Restricted Owners Package	(Owns No Changed Paths)
Restricted Project

Summary

Depends on D17595 - this diff uses features from D17595 such as the test_runner and handler.

Tests verify block and blocks range retrieval from the Chronik client. They check:

Block retrieval by height and hash.
Block order and consistency in ranges.
Error handling for invalid, parked, or invalidated blocks.
This ensures the Chronik client correctly handles block queries and edge cases.

Added detailed comments from TypeScript tests to Rust code, aligned test logic, and ensured error handling and block order validation.

Use the bitcoinsuite-chronik-client to return the tip height and tip hash length, thus we can compare with the python bitcoin tests and assert if they are equal.

Test Plan

Please set BUILD_DIR env to export BUILD_DIR="/path/to/build_dir
UNIX: ./contrib/teamcity/build-configurations.py build-bitcoinsuite-chronik-client

Diff Detail

Repository

rABC Bitcoin ABC

Branch

new_blocks

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 32466
Build 64420: Build Diff	build-bitcoinsuite-chronik-client
Build 64419: arc lint + arc unit

Event Timeline

hazzarust created this revision.Sat, Feb 22, 14:02

Owners added a reviewer: Restricted Owners Package.Sat, Feb 22, 14:02

Herald added a reviewer: Restricted Project. · View Herald TranscriptSat, Feb 22, 14:02

lint

Harbormaster completed remote builds in B32456: Diff 52724.Sat, Feb 22, 14:08

Harbormaster completed remote builds in B32457: Diff 52725.

To ensure no breaking changes from the updated IPC file, run ./contrib/teamcity/build-configurations.py chronik-client-integration-tests from bitcoin-abc to run typescript tests.

There is zero overlap between this and the typescript chronik client, so no reason for this to be part of the test plan.

Fabien requested changes to this revision.Mon, Feb 24, 08:43

Fabien added inline comments.

modules/bitcoinsuite-chronik-client/tests/block_and_blocks.rs
48 ↗	(On Diff #52725)	You're not testing the block, the name is bad. Also it would make sense to have get_block_by_height and get_block_by_hash so it's very clear what you're doing (instead of the dual Option params)
121 ↗	(On Diff #52725)	Check the error. If you get a 404 this test will pass but it's not what you expect
234 ↗	(On Diff #52725)	What error? Check it explicitly, it should be 404 here
397 ↗	(On Diff #52725)	dito
402 ↗	(On Diff #52725)	same you're not testing the blocks here
516 ↗	(On Diff #52725)	See comment in D17638 regarding this message
556 ↗	(On Diff #52725)	The BlockHash class from bitcoinsuite-core/src/block/block_hash.rs implements TryFrom, From and FromStr, you should use that instead

This revision now requires changes to proceed.Mon, Feb 24, 08:43

Updated the assert!() so that it will now anticipate 404 errors, renamed function names to be explicit

hazzarust edited the test plan for this revision. (Show Details)Mon, Feb 24, 17:12

Harbormaster completed remote builds in B32465: Diff 52736.Mon, Feb 24, 17:25

Merged imports

Harbormaster completed remote builds in B32466: Diff 52737.Mon, Feb 24, 17:45

hazzarust retitled this revision from [Chronik] Add block_and_blocks integration test into bitcoinsuite-chronik-client to [Chronik] Add integration test for block_and_blocks endpoint into bitcoinsuite-chronik-client.Mon, Feb 24, 17:52

Fabien requested changes to this revision.Mon, Feb 24, 19:49

Fabien added inline comments.

modules/bitcoinsuite-chronik-client/tests/block_and_blocks.rs
79	load ? The other functions are get_*, how is this one different ?
130	No. The error has a status code and a message, assert it's what you expect otherwise you're not testing much. Excluding a special case doesn't tell you if this is the correct error.
237	You want to check the status for the 404 error code, not the presence of the string "404" in the message
403	dito
519	Still needs to be updated

This revision now requires changes to proceed.Mon, Feb 24, 19:49

Revision Contents
Changeset List

Path

Size

modules/

bitcoinsuite-chronik-client/

tests/

block_and_blocks.rs

581 lines

Diff 52737

View Options

modules/bitcoinsuite-chronik-client/tests/block_and_blocks.rs

[Chronik] Add integration test for block_and_blocks endpoint into bitcoinsuite-chronik-clientNeeds RevisionPublicActions