Skip to content

Commit

Permalink
test(sequencer-relayer): reinstate black box tests (#1033)
Browse files Browse the repository at this point in the history
## Summary
The black box tests were removed in a recent PR. This PR reinstates them
using the new mock gRPC framework `astria-grpc-mock`.

## Background
The tests would have needed heavy refactoring in #963, and we wanted to
avoid adding a lot more code to that already-large PR. We decided to
temporarily delete them and reinstate them using `astria-grpc-mock` to
mock responses from the Celestia app.

## Changes
The tests removed in #963 and a single test briefly added in #1001 then
removed again have been restored. I also added a new test to check the
relayer shuts down in a timely manner.

Previously the tests leaned heavily on counts of blobs received by the
mock Celestia app. The current tests retain these checks, but also query
the `/status` endpoint of the sequencer-relayer to confirm its state.
There was also a single test previously which checked the state of the
postsubmit.json file. I didn't think this was necessary given that we're
querying the state in a more "black box" way now (via the http server),
but I didn't remove the function to perform this check
(`TestSequencerRelayer::assert_state_files_are_as_expected`) pending a
decision on whether to reinstate that check or not inside the
`later_height_in_state_leads_to_expected_relay` test.

As @SuperFluffy predicted, all the new protobuf packages added in #963
had to be added to the `pbjson_build` builder to generate the serde
impls required for using them in `astria-grpc-mock`. This accounts for
the vast majority of the new LoC in this PR.

I made a few small changes to the mock framework:
- added `Mock::up_to_n_times` to avoid failures due to a single-use mock
response being sent multiple times
- added `DynamicResponse` to support constructing mock responses which
can be passed the relevant gRPC request
- changed `MockGuard::wait_until_satisfied` to take `self` by ref rather
than value, since if it consumes self and `wait_until_satisfied` times
out, the `MockGuard` panics in its `Drop` impl, meaning we don't get a
good indication from e.g. a `tokio::time::timeout` as to what went wrong

Most of the new functionality lives in `MockCelestiaAppServer` and
`TestSequencerRelayer`. For the former, all gRPCs except for
`BroadcastTx` and `GetTx` are set up to always return valid-enough
responses - i.e. these don't need to be mounted individually in every
test case. In the case of `MockCelestiaAppServer`, the new functionality
is a combination of support for mounting mock responses and functions
with timeouts to query the `/status`, `/healthz` and `/readyz` http
endpoints of the sequencer-relayer.

## Testing
These changes are tests.

Closes #1008.
  • Loading branch information
Fraser999 authored May 3, 2024
1 parent 7111792 commit 83d23ab
Show file tree
Hide file tree
Showing 42 changed files with 7,672 additions and 1,355 deletions.
39 changes: 3 additions & 36 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 0 additions & 4 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ exclude = ["tools/protobuf-compiler"]

members = [
"crates/astria-build-info",
"crates/astria-celestia-client",
"crates/astria-celestia-mock",
"crates/astria-cli",
"crates/astria-composer",
"crates/astria-conductor",
Expand All @@ -27,8 +25,6 @@ members = [
# not act on lints
default-members = [
"crates/astria-build-info",
"crates/astria-celestia-client",
"crates/astria-celestia-mock",
"crates/astria-cli",
"crates/astria-composer",
"crates/astria-conductor",
Expand Down
42 changes: 0 additions & 42 deletions crates/astria-celestia-client/Cargo.toml

This file was deleted.

Loading

0 comments on commit 83d23ab

Please sign in to comment.