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

Test(rpc): Test support for different api modules #1288

Merged
merged 9 commits into from
Feb 12, 2023

Conversation

ensi321
Copy link
Contributor

@ensi321 ensi321 commented Feb 11, 2023

Closes #1224

@codecov-commenter
Copy link

codecov-commenter commented Feb 11, 2023

Codecov Report

Merging #1288 (490712a) into main (c9629d0) will increase coverage by 0.30%.
The diff coverage is 100.00%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##             main    #1288      +/-   ##
==========================================
+ Coverage   76.90%   77.20%   +0.30%     
==========================================
  Files         340      348       +8     
  Lines       37878    38598     +720     
==========================================
+ Hits        29131    29801     +670     
- Misses       8747     8797      +50     
Flag Coverage Δ
unit-tests 77.20% <100.00%> (+0.30%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
crates/rpc/rpc-types/src/eth/index.rs 69.64% <100.00%> (+69.64%) ⬆️
crates/staged-sync/src/utils/chainspec.rs 23.52% <0.00%> (-26.48%) ⬇️
crates/storage/db/src/abstraction/database.rs 80.00% <0.00%> (-20.00%) ⬇️
crates/staged-sync/src/config.rs 57.60% <0.00%> (-11.23%) ⬇️
crates/storage/db/src/tables/models/blocks.rs 76.54% <0.00%> (-11.12%) ⬇️
...rates/storage/db/src/implementation/mdbx/cursor.rs 74.64% <0.00%> (-7.18%) ⬇️
crates/net/network/src/session/handle.rs 28.57% <0.00%> (-7.15%) ⬇️
crates/transaction-pool/src/test_utils/mock.rs 62.05% <0.00%> (-6.74%) ⬇️
...ates/net/downloaders/src/test_utils/file_client.rs 85.02% <0.00%> (-3.09%) ⬇️
crates/storage/provider/src/utils.rs 94.00% <0.00%> (-2.73%) ⬇️
... and 108 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@mattsse mattsse added A-rpc Related to the RPC implementation C-test A change that impacts how or what we test labels Feb 11, 2023
@ensi321 ensi321 marked this pull request as ready for review February 11, 2023 18:07
@ensi321 ensi321 requested a review from gakonst as a code owner February 11, 2023 18:07
Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

this is great! q regarding the commented calls

It is useful to have either

  • test the endpoint is working
  • or returns an unimplemented error so the node does not panic

Comment on lines 76 to 81
// EthApiClient::send_transaction(client).await.err().unwrap()));
// EthApiClient::send_raw_transaction(client).await.err().unwrap()));
// EthApiClient::sign(client).await.err().unwrap()));
// EthApiClient::sign_transaction(client).await.err().unwrap()));
// EthApiClient::sign_typed_data(client).await.err().unwrap()));
// EthApiClient::get_proof(client).await.err().unwrap()));
Copy link
Collaborator

Choose a reason for hiding this comment

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

why are they currently commented?

Copy link
Member

Choose a reason for hiding this comment

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

@mattsse maybe you were looking at an older version of the PR?

Copy link
Member

@gakonst gakonst left a comment

Choose a reason for hiding this comment

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

LGTM, and as we implement more calls we should turn these tests on.

@mattsse latest version of the PR seesm to have no uncommented code, so good by me

@gakonst gakonst merged commit dc391f9 into paradigmxyz:main Feb 12, 2023
grantkee pushed a commit to layer-g/reth that referenced this pull request Feb 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rpc Related to the RPC implementation C-test A change that impacts how or what we test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add more RPC module tests
4 participants