-
Notifications
You must be signed in to change notification settings - Fork 89
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(starknet_client): add tests for a 0.13.2 block #2181
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2181 +/- ##
==========================================
+ Coverage 65.97% 66.41% +0.44%
==========================================
Files 136 139 +3
Lines 18128 18379 +251
Branches 18128 18379 +251
==========================================
+ Hits 11960 12207 +247
Misses 4876 4876
- Partials 1292 1296 +4 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @dan-starkware and @ShahakShama)
crates/starknet_client/resources/reader/block_post_0_13_2.json
line 1 at r1 (raw file):
{
IDK how to review this file, so I'm relying on your test making use of it and that's it...
Code quote:
{
crates/starknet_client/src/reader/objects/block_test.rs
line 159 at r1 (raw file):
); // TODO(shahak): Find a block with at least 2 transactions, and uncomment the code below.
What is the blocker for this? Why not do it now?
Code quote:
// TODO(shahak): Find a block with at least 2 transactions, and uncomment the code below.
dc9e2fe
to
e73f058
Compare
The base branch was changed.
ae49485
to
c1aa4e5
Compare
c1aa4e5
to
5f46255
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 2 files reviewed, all discussions resolved (waiting on @dan-starkware and @matan-starkware)
crates/starknet_client/src/reader/objects/block_test.rs
line 159 at r1 (raw file):
Previously, matan-starkware wrote…
What is the blocker for this? Why not do it now?
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 2 of 2 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @dan-starkware and @ShahakShama)
crates/starknet_client/src/reader/objects/block_test.rs
line 175 at r3 (raw file):
} ) );
It looks like you don't care about any of the fields, so why not remove this clutter?
(for the entire test)
Suggestion:
assert_matches!(
err,
ReaderClientError::TransactionReceiptsError(
_ )
);
5f46255
to
b66a914
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @dan-starkware and @matan-starkware)
crates/starknet_client/src/reader/objects/block_test.rs
line 175 at r3 (raw file):
Previously, matan-starkware wrote…
It looks like you don't care about any of the fields, so why not remove this clutter?
(for the entire test)
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @dan-starkware and @ShahakShama)
crates/starknet_client/src/reader/objects/block_test.rs
line 175 at r3 (raw file):
Previously, ShahakShama wrote…
Done.
I left comments on the other places "(for the entire test)"
crates/starknet_client/src/reader/objects/block_test.rs
line 126 at r4 (raw file):
num_of_receipts: _, } )
Suggestion:
ReaderClientError::TransactionReceiptsError(
_
)
crates/starknet_client/src/reader/objects/block_test.rs
line 141 at r4 (raw file):
receipt_tx_index: _, } )
Suggestion:
ReaderClientError::TransactionReceiptsError(
_
)
crates/starknet_client/src/reader/objects/block_test.rs
line 156 at r4 (raw file):
receipt_tx_hash: _, } )
Suggestion:
ReaderClientError::TransactionReceiptsError(
_
)
b66a914
to
9ff8a6a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @dan-starkware and @matan-starkware)
crates/starknet_client/src/reader/objects/block_test.rs
line 126 at r4 (raw file):
num_of_receipts: _, } )
Done.
crates/starknet_client/src/reader/objects/block_test.rs
line 141 at r4 (raw file):
receipt_tx_index: _, } )
Done.
crates/starknet_client/src/reader/objects/block_test.rs
line 156 at r4 (raw file):
receipt_tx_hash: _, } )
Done.
This change is