-
Notifications
You must be signed in to change notification settings - Fork 164
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 adding programs using non squashed Felt252Dict #1797
Conversation
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1797 +/- ##
==========================================
+ Coverage 94.79% 94.89% +0.09%
==========================================
Files 102 102
Lines 40150 40151 +1
==========================================
+ Hits 38059 38100 +41
+ Misses 2091 2051 -40 ☔ View full report in Codecov by Sentry. |
Benchmark Results for unmodified programs 🚀
|
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.
Nice @FrancoGiachetta
I leaved you some comments and we can merge it
Another point. I think it is a good idea to move the non squashed programs to another folder like
cairo_programs/cairo-1-programs/dict_without_squash
or something similar
cairo1-run/src/main.rs
Outdated
@@ -320,13 +320,6 @@ mod tests { | |||
None | |||
)] | |||
#[case("array_integer_tuple.cairo", "[1] 1", "[1 1 1]", None, None)] | |||
#[case( |
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.
why are we removing this test?
|
||
// TODO: remove this temporary fix once fixed in cairo | ||
#[inline(never)] | ||
fn identity<T>(t: T) -> T { t } |
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.
add endline
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.
I dont get why we change this file,
The idea was to add two new test, not removing the existing ones
"{66675: [8 9 10 11] 66676: [1 2 3]}", | ||
None | ||
)] | ||
fn test_run_progarm_non_proof( |
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.
Add a comment here explaining why we test these programs in a different way than the others
@pefontana changes were done. Sorry about the files, seems I've had some trouble renaming the files. |
cairo_programs/cairo-1-programs/dict_non_squashed/dict_with_struct_non_squash.cairo
Outdated
Show resolved
Hide resolved
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.
LGTM, except for a small typo
cairo1-run/memory
Outdated
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.
What is this file?
Add non squashed Felt252Dict tests
Description
PR #1721 modified felt tests so that the work with
SquashedFelt252Dict
. This PR adds the same tests with the difference that they useFelt252Dict
.Checklist