ABCI compatibility with 0.17.0 #249
Comments
This was referenced May 7, 2020
|
Closing this for now. The current version bump was fixed by @greg-szabo in #256 |
shonfeder
pushed a commit
to shonfeder/tendermint-rs
that referenced
this issue
Jun 15, 2020
Proposes one solution to the underlying issue behind - 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 the abci version is within a version range which is 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. The current approach of hardcoding in a version in the integration test tiself seems to create a lot of busy work due to uninformative test failures, and I'm not sure if delivers value. If the integration tests are meant to test that the RPC client integrates correctly with ACBI, should we really consider that integration has failed if everything works as expected while interfacing with an older (or newer) version?
shonfeder
pushed a commit
to shonfeder/tendermint-rs
that referenced
this issue
Jun 15, 2020
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 <[email protected]>
shonfeder
pushed a commit
to shonfeder/tendermint-rs
that referenced
this issue
Jun 15, 2020
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 <[email protected]>
shonfeder
pushed a commit
to shonfeder/tendermint-rs
that referenced
this issue
Jun 15, 2020
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 <[email protected]>
liamsi
pushed a commit
that referenced
this issue
Jun 15, 2020
* Add integration test pinned to tendermint-go v0.33 Closes #304 Also renames test-integration-ignored to test-integration-latest. With this change, CI runs two integration tests: 1. A `stable` test to protect against regressions, run against a pinned version of the tendermint/tendermint docker image. 2. A `latest` test to track whether we're maintaining parity with the latest development version of tendermint-go. Signed-off-by: Shon Feder <[email protected]> * Remove test for hardcoded ABCI version 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., - #249 - #238 - #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 <[email protected]> * Update changelog Signed-off-by: Shon Feder <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Note that integration tests on master are failing again. Now with:
What is odd about this is that this fails although there was no new release yet (this is from pending):
https://github.com/tendermint/tendermint/blob/master/CHANGELOG_PENDING.md#v0335
While it is cool to be warned early on about changes to come it would also be good if there was a mechanism to stick to a particular tendermint release (e.g. currently we are aiming for compatibility with 0.33).
related:
The text was updated successfully, but these errors were encountered: