-
Notifications
You must be signed in to change notification settings - Fork 26
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(batcher): block builder transaction failed test #2171
test(batcher): block builder transaction failed test #2171
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2171 +/- ##
==========================================
- Coverage 40.10% 5.30% -34.81%
==========================================
Files 26 145 +119
Lines 1895 16985 +15090
Branches 1895 16985 +15090
==========================================
+ Hits 760 901 +141
- Misses 1100 16007 +14907
- Partials 35 77 +42 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
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 1 files reviewed, 1 unresolved discussion (waiting on @alonh5)
crates/starknet_batcher/src/block_builder_test.rs
line 207 at r1 (raw file):
} fn transaction_failed_test_expectations(
I can add this test as a case to the previous test, with a bit of refactor.
wdyt?
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 1 files reviewed, 1 unresolved discussion (waiting on @alonh5)
crates/starknet_batcher/src/block_builder_test.rs
line 207 at r1 (raw file):
Previously, Yael-Starkware (YaelD) wrote…
I can add this test as a case to the previous test, with a bit of refactor.
wdyt?
(this comment was supposed to be written over the test function and not here ).
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 r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Yael-Starkware)
crates/starknet_batcher/src/block_builder_test.rs
line 207 at r1 (raw file):
Previously, Yael-Starkware (YaelD) wrote…
(this comment was supposed to be written over the test function and not here ).
If it's not to big of a refactor and you think it makes sense, then yeah.
1374f3d
to
cb7e26e
Compare
d3224ef
to
fa0b49a
Compare
cb7e26e
to
11bdf96
Compare
fa0b49a
to
495aefc
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 1 files reviewed, 1 unresolved discussion (waiting on @alonh5)
crates/starknet_batcher/src/block_builder_test.rs
line 207 at r1 (raw file):
Previously, alonh5 (Alon Haramati) wrote…
If it's not to big of a refactor and you think it makes sense, then yeah.
ack
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 r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Yael-Starkware)
Merge activity
|
5eae844
to
201bb08
Compare
201bb08
to
8aa18b8
Compare
No description provided.