Skip to content

Commit

Permalink
Remove test for hardcoded ABCI version
Browse files Browse the repository at this point in the history
Hardcoding a test for the version in this way makes it impossible to run the
integration tests against different ABCI versions.

This change proposes one solution to address this problem and to the
underlying issue behind, e.g.,

- informalsystems#249
- informalsystems#238
- informalsystems#233

My sense is that, if we want to set a strict abci version requirement
for the rpc client, then we should put that in the source code itself.

E.g., we might put a check on the client that ensures the abci version
is within a specified version range known to be supported. If the
version is outside that range, we could either error out or log
errors/warning to alert users that we don't guarantee compatibility.

However, the current approach of hardcoding in a version in the
integration test seems to create a lot of busy work due to uninformative
test failures and it's not obvious what value it delivers. If the
integration tests are meant to test that the RPC client integrates
correctly with ACBI, should we really consider integration to have
failed when everything works as expected while interfacing with an
older (or newer) version?

Signed-off-by: Shon Feder <shon@informal.systems>
  • Loading branch information
Shon Feder committed Jun 15, 2020
1 parent 271edeb commit 5585b41
Showing 1 changed file with 0 additions and 1 deletion.
1 change: 0 additions & 1 deletion tendermint/tests/integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ mod rpc {
async fn abci_info() {
let abci_info = localhost_rpc_client().abci_info().await.unwrap();

assert_eq!(&abci_info.version, "0.17.0");
assert_eq!(abci_info.app_version, 1u64);
// the kvstore app's reply will contain "{\"size\":0}" as data right from the start
assert_eq!(&abci_info.data, "{\"size\":0}");
Expand Down

0 comments on commit 5585b41

Please sign in to comment.