Skip to content

Conversation

@potuz
Copy link
Contributor

@potuz potuz commented Sep 1, 2022

Implement ethereum/consensus-specs#2982

The semantics of the valid tests in forkchoice and sync is different this is why we fail currently, if that is changed the tests pass.

Passing spectests as of 09/11.

@potuz potuz force-pushed the optimistic_sync_spectests branch from 75317a9 to d793eed Compare September 2, 2022 11:01
@potuz potuz force-pushed the optimistic_sync_spectests branch from d793eed to d32e676 Compare September 11, 2022 15:25
@potuz potuz marked this pull request as ready for review September 13, 2022 01:01
@potuz potuz requested a review from a team as a code owner September 13, 2022 01:01
@potuz potuz changed the title [WIP] Implement optimistic sync spectests Implement optimistic sync spectests Sep 13, 2022
Comment on lines +36 to +38
if err != nil {
return nil, ""
}
Copy link
Collaborator

@terencechain terencechain Sep 13, 2022

Choose a reason for hiding this comment

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

Can you explain this change? or even better add a comment

Copy link
Collaborator

Choose a reason for hiding this comment

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

Addressed offline, this is due to folder changes

Comment on lines +36 to +38
testFolders, _ := utils.TestFolders(t, config, version.String(fork), basePath)
if testFolders == nil {
return
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if t.Fatal is better than return here to eliminate false positives

@prylabs-bulldozer prylabs-bulldozer bot merged commit a27feb4 into develop Sep 15, 2022
@delete-merged-branch delete-merged-branch bot deleted the optimistic_sync_spectests branch September 15, 2022 05:42
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.

2 participants