-
Notifications
You must be signed in to change notification settings - Fork 146
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
testing: e2e tests for soft opt out #857
Conversation
@mpoke lets chat tomorrow about whether this PR validates that the default value of |
I don't think it does. IIUC, both the consumer and provider in the tests are running the same version of ICS, the one where the provider sets the |
@mpoke |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved!
I like that it's in happy path so we have an A/B test confirming that both downtime jailing scenarios work as expected.
// soft opt-out threshold to 0.05 in the consumer genesis to ensure that the | ||
// consumer binary doesn't panic. Sdk requires that all params are set to valid | ||
// values from the genesis file. | ||
genesisChanges: ".app_state.ccvconsumer.params.soft_opt_out_threshold = \"0.05\"", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No biggie, but couldn't this be 0?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0 is a valid value for that param, but setting it to 0 would make the test fail (just confirmed) and wouldn't validate the feature. It'd essentially mean 0% of validator set is allowed to opt out
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved, with one minor comment
Description
Adds e2e tests validating #765
Linked issues
Closes #849
Type of change
If you've checked more than one of the first three boxes, consider splitting this PR into multiple PRs!
Feature
: Changes and/or adds code behavior, irrelevant to bug fixesFix
: Changes and/or adds code behavior, specifically to fix a bugRefactor
: Changes existing code style, naming, structure, etc.Testing
: Adds testingDocs
: Adds documentationRegression tests
Existing happy path steps are still there
New behavior tests
Two new steps functions were added, stepsRedelegateForOptOut and stepsDowntimeWithOptOut.
Versioning Implications
If the above box is checked, which version should be bumped?
MAJOR
: Consensus breaking changes to both the provider and consumers(s), including updates/breaking changes to IBC communication between provider and consumer(s)MINOR
: Consensus breaking changes which affect either only the provider or only the consumer(s)PATCH
: Non consensus breaking changesTargeting
Please select one of the following: