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.

Sign up for GitHub

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

Jump to bottom

internal/ethapi: add more testcase for block/header rpc #27325

Merged
merged 11 commits into from
May 29, 2023
Merged

internal/ethapi: add more testcase for block/header rpc #27325

merged 11 commits into from
May 29, 2023

Conversation

jsvisa
Copy link
Contributor

@jsvisa jsvisa commented May 23, 2023

No description provided.

s1na
s1na reviewed May 23, 2023
View reviewed changes
internal/ethapi/api_test.go Outdated Show resolved Hide resolved
s1na
s1na reviewed May 23, 2023
View reviewed changes
internal/ethapi/api_test.go Outdated Show resolved Hide resolved
@s1na
Copy link
Contributor

s1na commented May 23, 2023

It's great you're adding more tests! while you're at it can you also cover the ByHash variants for header and block?

@jsvisa jsvisa force-pushed the internal-ethapi-more-testcase branch from 50212eb to 5b5240a Compare May 23, 2023 15:11
@jsvisa
Copy link
Contributor Author

jsvisa commented May 23, 2023

Done, please take another look

It's great you're adding more tests! while you're at it can you also cover the ByHash variants for header and block?

s1na
s1na approved these changes May 23, 2023
View reviewed changes
Copy link
Contributor

@s1na s1na left a comment

Choose a reason for hiding this comment

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

Amazing, I can't believe we have 25 cases for getBlock variants now :)

@jsvisa
Copy link
Contributor Author

jsvisa commented May 23, 2023

Some of them are just for testing different boundaries and do not include the Safe, finalized block

After we merge this one, then I'm confident that I can complete the #27305.

@s1na
Copy link
Contributor

s1na commented May 24, 2023

Weird, one of the tests seems to be failing on windows: https://ci.appveyor.com/project/ethereum/go-ethereum/builds/47119845/job/owkbk2mxf3aygp5r?fullLog=true#L839

@jsvisa
Copy link
Contributor Author

jsvisa commented May 24, 2023

Size is not the same

want (string) (len=4) "size": (string) (len=5) "0x23e",
have (string) (len=4) "size": (string) (len=5) "0x21a",

diff is 36

maybe related to 32bit?

I'm going to find a Windows PC to test it manually.

@jsvisa
Copy link
Contributor Author

jsvisa commented May 24, 2023 •
edited
Loading

Seems its the reason for headerSize in 64 and 32 bits, check #27345 test case

Failed to run in win32
image

@jsvisa jsvisa mentioned this pull request May 25, 2023
Merged
jsvisa and others added 11 commits May 25, 2023 21:20
@jsvisa
internal/ethapi: test rpc transaction marshal
d931f24 
Signed-off-by: jsvisa <delweng@gmail.com>
@jsvisa
internal/ethapi: parallel test rpcmarshalblock
6db2dbb 
Signed-off-by: jsvisa <delweng@gmail.com>
@jsvisa
internal/ethapi: rpc pending block
0c4a4cc 
Signed-off-by: jsvisa <delweng@gmail.com>
@jsvisa
internal/ethapi: test getBlockHeader
e7eebd5 
Signed-off-by: jsvisa <delweng@gmail.com>
@jsvisa
internal/ethapi: add withdrawal testcase
2b76882 
Signed-off-by: jsvisa <delweng@gmail.com>
@jsvisa @s1na
Update internal/ethapi/api_test.go
220ab3c 
Co-authored-by: Sina Mahmoodi <1591639+s1na@users.noreply.github.com>
Signed-off-by: jsvisa <delweng@gmail.com>
@jsvisa
internal/ethapi: -2 -> pendingnumber
ce2fa14 
Signed-off-by: jsvisa <delweng@gmail.com>
@jsvisa
internal/ethapi: typo
8a12e59 
Signed-off-by: jsvisa <delweng@gmail.com>
@jsvisa
internal/ethapi: adjust pending/latest comment
d14ed8e 
Signed-off-by: jsvisa <delweng@gmail.com>
@jsvisa
internal/ethapi: add byHash test cases
fa61d6c 
Signed-off-by: jsvisa <delweng@gmail.com>
@jsvisa
internal/ethapi: rm field size in getHeaderByXXX
05f6eb4 
Signed-off-by: jsvisa <delweng@gmail.com>
@jsvisa jsvisa force-pushed the internal-ethapi-more-testcase branch from 5b5240a to 05f6eb4 Compare May 25, 2023 13:24
@holiman holiman merged commit c57b343 into ethereum:master May 29, 2023
@holiman holiman added this to the 1.12.1 milestone May 29, 2023
@holiman
Copy link
Contributor

holiman commented May 29, 2023

Thanks!

@jsvisa jsvisa deleted the internal-ethapi-more-testcase branch May 29, 2023 13:46
devopsbo3 pushed a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
@jsvisa @s1na @devopsbo3
internal/ethapi: add more testcases for block/header rpc (ethereum#27325
978944e 
)

Signed-off-by: jsvisa <delweng@gmail.com>
Co-authored-by: Sina Mahmoodi <1591639+s1na@users.noreply.github.com>
devopsbo3 added a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
@devopsbo3
Revert "internal/ethapi: add more testcases for block/header rpc (eth…
63c108e 
…ereum#27325)"

This reverts commit 978944e.
devopsbo3 added a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
@devopsbo3
Revert "internal/ethapi: add more testcases for block/header rpc (eth…
43119e7 
…ereum#27325)"

This reverts commit 978944e.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@s1na s1na s1na approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.12.1
Development

Successfully merging this pull request may close these issues.
3 participants
@jsvisa @s1na @holiman