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.

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

[4.0 -> main] Increase robustness of snapshot diff python test #873

Merged
merged 6 commits into from
Mar 23, 2023

Conversation

vladtr
Copy link
Contributor

@vladtr vladtr commented Mar 22, 2023

Bringing in #856 to main

This PR mitigates rare and annoying issue of the test failing when there is a new block happening in between of scheduling and "now" create_snapshot calls. The fix is to execute create_snapshot first and schedule programmable snapshot at exact same block num. Same as with irreversible one.

Resolves #853

@vladtr vladtr requested review from heifner and linh2931 March 22, 2023 22:01
@@ -346,7 +346,7 @@ def getEosAccount(self, name, exitOnError=False, returnType=ReturnType.json):
return self.processCleosCmd(cmd, cmdDesc, silentErrors=False, exitOnError=exitOnError, exitMsg=msg, returnType=returnType)

def getTable(self, contract, scope, table, exitOnError=False):
cmdDesc = "get table"
cmdDesc = "get table --time-limit 999"
Copy link
Contributor

@ClaytonCalabrese ClaytonCalabrese Mar 22, 2023

Choose a reason for hiding this comment

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

This will probably break nodeos 2.0 compatibility for performance test. This time limit is handled in configureVersion(self) as self.cleosLimit but was lost due to the splitting of Node.py into queries.

https://github.com/AntelopeIO/leap/pull/874/files#diff-4d1412150dda54fbc6280cc9053914a45d01e43085e35b0a5a68e9c8758ed42bR350 restores this in a way that won't break 2.0 compatibility.

Copy link
Contributor Author

@vladtr vladtr Mar 22, 2023

Choose a reason for hiding this comment

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

I see, that was brought from un-merged to main PR with Kevin's commit, should I just remove this change from this PR? It has nothing to do with said python test

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed it from this pr, so it properly has only one file with my changes

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks

@vladtr vladtr merged commit 5d1ab0c into main Mar 23, 2023
@vladtr vladtr deleted the increase-snapshot-test-robustness-main branch March 23, 2023 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Test failure: nodeos_snapshot_diff_test failed
4 participants