-
Notifications
You must be signed in to change notification settings - Fork 220
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
chore: replace proxy mining with sha mining in tests #3455
chore: replace proxy mining with sha mining in tests #3455
Conversation
9e7700a
to
6d0b26e
Compare
4b1dca9
to
c796644
Compare
bump up block amounts rerorg speed improvement
c796644
to
06036d1
Compare
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.
I'm not keen on this. It makes the code more complicated instead of simpler.
There are a number of tests that are now marked as flaky, an indication that this is a regression
@@ -96,7 +96,7 @@ Feature: Mempool | |||
Then SENDER has TX1 in NOT_STORED state | |||
Then SENDER has TX2 in MINED state | |||
|
|||
@critical | |||
@critical @flaky |
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.
How is this regressing now?
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.
Its a new test, created in PR #3439
It failed on CI this morning and I have not been able to get it to fail locally.
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.
This is looking good. I like the addition of waiting on the base node state. One comment and we will tackle Flakey tests in the next sprint.
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.
Ok, looking at this again I see that the state check generally replaces a wait X seconds. I think this can go in.
@@ -197,6 +199,8 @@ Feature: Reorgs | |||
And I connect node NODE_B1 to node NODE_B3 and wait 1 seconds | |||
And I connect node NODE_B2 to node NODE_B4 and wait 1 seconds | |||
And I connect node SEED_B1 to node SEED_B2 and wait <SYNC_TIME> seconds |
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.
Perhaps we can remove this wait <SYNC_TIME> seconds
and then keep the below
Description
Replaced merge mining proxy mining in most tests with sha3 miner.
Added base_node state to grpc for use in the tests
Motivation and Context
The monero merge mining proxy makes a network call to the outside which can be unreliable. The sha3 miner only uses local information which is faster as well.
We use sleeps that in the cucumber code to ensure that base_nodes have time to sync, we can just if they are in listening state to check if we can go to the next step
How Has This Been Tested?
Manual cucumber tests