Skip to content
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.

Sign up for GitHub

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

Jump to bottom

test: reduce number of happyPath steps #1179

Closed
wants to merge 4 commits into from
Closed

test: reduce number of happyPath steps #1179

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
389f6fd
test: reduce number of happyPath steps
MSalopek Aug 2, 2023
9044f33
test: reduce number of happyPath steps
MSalopek Aug 2, 2023
41797e9
test: update steps after opt-out
MSalopek Aug 2, 2023
5457e71
test: update steps and fix lint
MSalopek Aug 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions tests/e2e/steps.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,10 @@ var happyPathSteps = concatSteps(
stepsRedelegateForOptOut("consu"),
stepsDowntimeWithOptOut("consu"),
stepsRedelegate("consu"),
stepsDowntime("consu"),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to test for downtime twice, testing for downtime with soft opt-out also covers "normal" downtime.

Copy link
Contributor

@insumity insumity Aug 2, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although both use the downtimeSlashAction action, in stepsDowntimeWithOptOut there doesn't seem that slashing takes place since alice has < 5% of the total voting power. While in stepsDowntime, bob is indeed slashed.
Additionally stepsDowntime is also performing an unjailValidatorAction.
Therefore, wouldn't we weaken the happyPathSteps by removing the stepsDowntime?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will, but there's an additional issue that proposes changes to how this runs in the CI:

We can have multiple jobs running in parallel and make the execution time shorter

stepsRejectEquivocationProposal("consu", 2), // prop to tombstone bob is rejected
stepsDoubleSignOnProviderAndConsumer("consu"), // carol double signs on provider, bob double signs on consumer
stepsSubmitEquivocationProposal("consu", 2), // now prop to tombstone bob is submitted and accepted
stepsStartRelayer(),
stepsConsumerRemovalPropNotPassing("consu", 3), // submit removal prop but vote no on it - chain should stay
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked in shortHappyPathSteps

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When is the short happy path executed?
My understanding is that the normal happy path is executed during CI (e.g., automated tests before we merge), but not clear about the short happy_ path.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a misnomer. short uses cometmock but it is not running in CI yet

stepsStopChain("consu", 4), // stop chain
stepsStopChain("consu", 3), // stop chain
)

var shortHappyPathSteps = concatSteps(
Expand Down
5 changes: 3 additions & 2 deletions tests/e2e/steps_double_sign.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package main

// Steps that make carol double sign on the provider, and bob double sign on a single consumer
// Steps that make carol double sign on the provider, and bob double sign on a single consumer.
// These steps happen after opt-out downtime traces have executed.
func stepsDoubleSignOnProviderAndConsumer(consumerName string) []Step {
return []Step{
{
Expand All @@ -22,7 +23,7 @@ func stepsDoubleSignOnProviderAndConsumer(consumerName string) []Step {
ValPowers: &map[validatorID]uint{
validatorID("alice"): 509,
validatorID("bob"): 500,
validatorID("carol"): 495, // not tombstoned on consumerName yet
validatorID("carol"): 501, // not tombstoned on consumerName yet
},
},
},
Expand Down
4 changes: 2 additions & 2 deletions tests/e2e/steps_submit_equivocation_proposal.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ func stepsRejectEquivocationProposal(consumerName string, propNumber uint) []Ste
ValPowers: &map[validatorID]uint{
validatorID("alice"): 509,
validatorID("bob"): 500,
validatorID("carol"): 495,
validatorID("carol"): 501,
Copy link
Contributor

@insumity insumity Aug 2, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why were the voting powers changed here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some steps that changed the voting power were removed. Steps are executed sequentially, so if you remove one of them you have to account for the removal in other places.

},
ValBalances: &map[validatorID]uint{
validatorID("bob"): 9500000000,
Expand All @@ -35,7 +35,7 @@ func stepsRejectEquivocationProposal(consumerName string, propNumber uint) []Ste
ValPowers: &map[validatorID]uint{
validatorID("alice"): 509,
validatorID("bob"): 500,
validatorID("carol"): 495,
validatorID("carol"): 501,
},
},
},
Expand Down