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

tests(sequencer): add snapshot tests for app hash #1057

Merged
merged 8 commits into from
May 10, 2024

Conversation

noot
Copy link
Collaborator

@noot noot commented May 8, 2024

Summary

add snapshot tests for app hash to catch breaking changes made to the app logic.

Background

if changes are breaking, we should know about it. these tests should catch (most) breaking changes to the application state itself.

Changes

  • add snapshot tests for init_chain, finalize_block, and all actions except for IBC actions as they require extensive setup.

Testing

no code was changed, just tests added

Related Issues

closes #1051

@noot noot requested review from a team, joroshiba and SuperFluffy as code owners May 8, 2024 21:09
@github-actions github-actions bot added the sequencer pertaining to the astria-sequencer crate label May 8, 2024
Copy link
Contributor

@Fraser999 Fraser999 left a comment

Choose a reason for hiding this comment

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

LGTM.

thoughts on adding the snapshot directly to the unit tests that already exist there?

Seems like a good idea to me, but maybe best to wait for more opinions?

@noot noot enabled auto-merge May 9, 2024 20:44
@noot
Copy link
Collaborator Author

noot commented May 9, 2024

@Fraser999 ended up adding a snapshot test which tests every action!

@noot noot added this pull request to the merge queue May 10, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 10, 2024
@noot noot added this pull request to the merge queue May 10, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 10, 2024
@noot noot enabled auto-merge May 10, 2024 19:46
@noot noot added this pull request to the merge queue May 10, 2024
Merged via the queue into main with commit 48a05a9 May 10, 2024
36 checks passed
@noot noot deleted the noot/breaking-changes-test branch May 10, 2024 20:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sequencer pertaining to the astria-sequencer crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sequencer: breaking changes unit tests
3 participants