Page MenuHomePhabricator

[chronik] Add test runner to bitcoinsuite-chronik-client
Needs ReviewPublic

Authored by hazzarust on Sun, Jan 19, 11:54.

Details

Reviewers
Fabien
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Restricted Project
Summary

Every Rust test that will trigger a child Python IPC test will use the test runner to execute, and to handle stderr / stdout.
The test runner takes in a script (to run the child Python IPC test) and a bespoke socket, giving the opportunity to run multiple
tests at once without worrying about socket collision.

This diff ensures sockets are passed to the child process correctly, setting up for future IPC communication.

Test Plan

cd modules/bitcoinsuite-chronik-client && cargo test -- --nocapture

Event Timeline

Owners added a reviewer: Restricted Owners Package.Sun, Jan 19, 11:54
Fabien requested changes to this revision.Mon, Jan 20, 08:57
Fabien added a subscriber: Fabien.

The test plan doesn't work

modules/bitcoinsuite-chronik-client/src/test_runner.rs
16 ↗(On Diff #52306)

This code runs in the context of testing, so it's fine (and actually useful) to be a bit verbose to help with debugging.
E.g. you should print the file name here if you failed to remove it.

32 ↗(On Diff #52306)

Same here, you should print something like "Starting test_runner for <script name>" before calling the command

36 ↗(On Diff #52306)

Please use some less generic environment variable name here, there is a high risk of collision. E.g. CHRONIK_CLIENT_RUST_IPC_SOCKET

37 ↗(On Diff #52306)

I don't think you need to pass this, it's already determined by the test_runner.py. This will also avoid a copy because you can remove the above clone()

66 ↗(On Diff #52306)

Let's bump the timeout to something absurdly large.
You don't want the test to fail randomly because the machine running it is slow. OTOH it's fine to wait say 1 minute if there is a real error.

88 ↗(On Diff #52306)

This is unreachable due to the above assert

This revision now requires changes to proceed.Mon, Jan 20, 08:57
hazzarust edited the test plan for this revision. (Show Details)