Skip to content

Extend fork_choice test format with on_payload_info#2965

Merged
hwwhww merged 3 commits intoethereum:devfrom
mkalinin:optimistic-sync-tests
Sep 12, 2022
Merged

Extend fork_choice test format with on_payload_info#2965
hwwhww merged 3 commits intoethereum:devfrom
mkalinin:optimistic-sync-tests

Conversation

@mkalinin
Copy link
Contributor

@mkalinin mkalinin commented Aug 3, 2022

Extends fork_choice tests format with payload_info, discussion in ethereum/consensus-spec-tests#28

@mkalinin mkalinin requested a review from hwwhww August 3, 2022 12:20
@hwwhww hwwhww mentioned this pull request Aug 23, 2022
4 tasks
Copy link
Contributor

@hwwhww hwwhww left a comment

Choose a reason for hiding this comment

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

Hey @mkalinin, I implemented a basic test in #2982

I am still on the fence between extending tests/formats/fork_choice/README.md or create a new tests/formats/sync/README.md that refers to the former. Thinking if the latter may be more clear in some ways. What do you think?

@mkalinin
Copy link
Contributor Author

Hey @mkalinin, I implemented a basic test in #2982

I am still on the fence between extending tests/formats/fork_choice/README.md or create a new tests/formats/sync/README.md that refers to the former. Thinking if the latter may be more clear in some ways. What do you think?

Unless there is any concern or obstacle, I would take a simple path forward which is extending the test by one more step, the step is optional and may not be supported by CL clients

@hwwhww hwwhww added the testing CI, actions, tests, testing infra label Sep 8, 2022
Copy link
Contributor

@hwwhww hwwhww left a comment

Choose a reason for hiding this comment

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

lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

testing CI, actions, tests, testing infra

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants