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

Split memory_state to avoid giant test module #1258

Merged
merged 1 commit into from
Nov 9, 2020
Merged

Conversation

yaahc
Copy link
Contributor

@yaahc yaahc commented Nov 6, 2020

Motivation

Prior to this PR memory_state defined and implemented functionality for three different types, Chain, NonFinalizedState, and QueuedBlocks. Each of these components will need a fair number of unit tests, and I realized that as its currently organized it would be difficult to organize the tests or at a glance figure out which tests are testing which components.

Solution

This PR changes the organization of memory_state such that each component it exports is defined in its own module. In follow up PRs each module will get its own test module, which will focus exclusively on unit tests for the item defined there-in.

The code in this pull request has:

  • Documentation Comments
  • Unit Tests and Property Tests

Review

Related Issues

Follow Up Work

This PR doesn't implement any of the items mentioned in the above tracking issue, but it should make it easier to review the code that later does introduce said tests.

@yaahc yaahc changed the base branch from main to height-consistency November 6, 2020 01:43
@yaahc yaahc marked this pull request as draft November 6, 2020 01:43
@yaahc yaahc added the S-blocked Status: Blocked on other tasks label Nov 6, 2020
@yaahc yaahc self-assigned this Nov 6, 2020
@yaahc yaahc requested a review from a team November 6, 2020 01:43
@yaahc
Copy link
Contributor Author

yaahc commented Nov 6, 2020

blocked by #1257, which it is based upon

Base automatically changed from height-consistency to main November 6, 2020 04:00
@teor2345 teor2345 removed the S-blocked Status: Blocked on other tasks label Nov 6, 2020
@teor2345
Copy link
Contributor

teor2345 commented Nov 6, 2020

I merged #1257, so I think this PR is unblocked.

@teor2345
Copy link
Contributor

teor2345 commented Nov 6, 2020

(I'll review after the conflicts have been resolved, because that makes git diff --color-moved work better.)

Copy link
Contributor

@dconnolly dconnolly left a comment

Choose a reason for hiding this comment

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

it lorge

@yaahc yaahc merged commit 818fede into main Nov 9, 2020
@yaahc yaahc deleted the memory-state-split branch November 9, 2020 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants