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

How to write tests for the consensus layer #2700

Merged
merged 47 commits into from
Nov 22, 2021
Merged

How to write tests for the consensus layer #2700

merged 47 commits into from
Nov 22, 2021

Conversation

qbzzt
Copy link
Contributor

@qbzzt qbzzt commented Oct 30, 2021

No description provided.

Copy link
Contributor

@protolambda protolambda left a comment

Choose a reason for hiding this comment

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

Hi Ori, nice to find you here in the consensus specs repo, thanks for the help documenting testing!

Some nitpicks in review, but generally this is very helpful, thank you 👍

tests/README.md Outdated Show resolved Hide resolved
tests/README.md Outdated Show resolved Hide resolved
tests/README.md Show resolved Hide resolved
tests/README.md Show resolved Hide resolved
tests/README.md Outdated Show resolved Hide resolved
tests/README.md Outdated Show resolved Hide resolved
tests/README.md Outdated Show resolved Hide resolved
@qbzzt
Copy link
Contributor Author

qbzzt commented Nov 1, 2021

Hi Ori, nice to find you here in the consensus specs repo, thanks for the help documenting testing!

Some nitpicks in review, but generally this is very helpful, thank you 👍

Nice to find you here too. Yes, I've been writing helping out EF with documentation and testing for a while now, long before I've even heard of Optimism.

I'm glad it helps. It's always difficult to know how much detail to put in a tutorial.

@qbzzt qbzzt marked this pull request as draft November 1, 2021 02:21
@qbzzt qbzzt requested a review from protolambda November 6, 2021 01:57
@qbzzt qbzzt marked this pull request as ready for review November 20, 2021 13:05
Copy link
Contributor

@djrtwo djrtwo left a comment

Choose a reason for hiding this comment

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

nice! just a handful of review comments


This type of test receives two parameters:

* `specs`: The protocol specifications
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* `specs`: The protocol specifications
* `spec`: The protocol specification (constants, functions, object definitions, etc)

tests/README.md Outdated Show resolved Hide resolved
tests/README.md Outdated Show resolved Hide resolved
tests/README.md Outdated Show resolved Hide resolved
tests/README.md Outdated Show resolved Hide resolved
tests/README.md Outdated Show resolved Hide resolved
tests/README.md Outdated

For every slot a validator is randomly selected as the proposer. Currently the proposer proposes the hash
for the current head of the beacon chain (the previous block). When shards are added, the proposer will also
propose a hash for the head of the assigned shard.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
propose a hash for the head of the assigned shard.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would just cut out a lot of the sharding stuff. There are actually separate proposrs that build the shard chains and it's probably not worth getting in the nuance here.

tests/README.md Outdated Show resolved Hide resolved
qbzzt and others added 7 commits November 22, 2021 11:27
Co-authored-by: Danny Ryan <dannyjryan@gmail.com>
Co-authored-by: Danny Ryan <dannyjryan@gmail.com>
Co-authored-by: Danny Ryan <dannyjryan@gmail.com>
Co-authored-by: Danny Ryan <dannyjryan@gmail.com>
Co-authored-by: Danny Ryan <dannyjryan@gmail.com>
Co-authored-by: Danny Ryan <dannyjryan@gmail.com>
@qbzzt qbzzt requested a review from djrtwo November 22, 2021 17:38
@djrtwo djrtwo merged commit 9999331 into ethereum:dev Nov 22, 2021
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