-
Notifications
You must be signed in to change notification settings - Fork 49
Move test utils to testing module #1422
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
base: main
Are you sure you want to change the base?
Conversation
Benchmark results Main vs HEAD.Base
Head
Base
Head
Base
Head
Base
Head
Base
Head
Base
Head
|
Benchmarking resultsBenchmark for program
|
Command | Mean [s] | Min [s] | Max [s] | Relative |
---|---|---|---|---|
Cairo-vm (Rust, Cairo 1) |
10.799 ± 0.047 | 10.709 | 10.875 | 4.39 ± 0.05 |
cairo-native (embedded AOT) |
2.461 ± 0.024 | 2.421 | 2.486 | 1.00 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
2.528 ± 0.020 | 2.502 | 2.570 | 1.03 ± 0.01 |
Benchmark for program dict_snapshot
Open benchmarks
Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
---|---|---|---|---|
Cairo-vm (Rust, Cairo 1) |
522.8 ± 6.5 | 514.1 | 535.0 | 1.00 |
cairo-native (embedded AOT) |
2154.6 ± 51.1 | 2112.5 | 2279.5 | 4.12 ± 0.11 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
2269.2 ± 21.0 | 2232.6 | 2306.2 | 4.34 ± 0.07 |
Benchmark for program factorial_2M
Open benchmarks
Command | Mean [s] | Min [s] | Max [s] | Relative |
---|---|---|---|---|
Cairo-vm (Rust, Cairo 1) |
4.748 ± 0.016 | 4.728 | 4.780 | 1.85 ± 0.02 |
cairo-native (embedded AOT) |
2.568 ± 0.021 | 2.541 | 2.615 | 1.00 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
2.602 ± 0.023 | 2.575 | 2.648 | 1.01 ± 0.01 |
Benchmark for program fib_2M
Open benchmarks
Command | Mean [s] | Min [s] | Max [s] | Relative |
---|---|---|---|---|
Cairo-vm (Rust, Cairo 1) |
4.683 ± 0.013 | 4.662 | 4.700 | 2.23 ± 0.02 |
cairo-native (embedded AOT) |
2.100 ± 0.021 | 2.068 | 2.134 | 1.00 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
2.145 ± 0.010 | 2.127 | 2.163 | 1.02 ± 0.01 |
Benchmark for program linear_search
Open benchmarks
Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
---|---|---|---|---|
Cairo-vm (Rust, Cairo 1) |
565.8 ± 5.5 | 557.0 | 573.2 | 1.00 |
cairo-native (embedded AOT) |
2185.4 ± 12.1 | 2158.5 | 2196.8 | 3.86 ± 0.04 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
2349.3 ± 28.8 | 2321.3 | 2419.3 | 4.15 ± 0.07 |
Benchmark for program logistic_map
Open benchmarks
Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
---|---|---|---|---|
Cairo-vm (Rust, Cairo 1) |
379.2 ± 1.5 | 376.9 | 381.7 | 1.00 |
cairo-native (embedded AOT) |
2256.3 ± 15.5 | 2230.0 | 2275.5 | 5.95 ± 0.05 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
2434.4 ± 19.7 | 2404.6 | 2468.4 | 6.42 ± 0.06 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1422 +/- ##
==========================================
- Coverage 81.30% 81.30% -0.01%
==========================================
Files 105 105
Lines 25741 25738 -3
==========================================
- Hits 20929 20926 -3
Misses 4812 4812 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
src/utils/testing.rs
Outdated
// ============================== | ||
// == TESTS: get_integer_layout | ||
// ============================== | ||
/// Ensures that the host's `u8` is compatible with its compiled counterpart. | ||
#[test] | ||
fn test_alignment_compatibility_u8() { | ||
assert_eq!(get_integer_layout(8).align(), 1); | ||
} | ||
|
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.
These tests should be kept in the utils.rs file, inside of a test
module. This file is reserved for testing utilities only.
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.
Mmm, I don't see why they should be kept there. Aren't unit tests usually kept in the same file?
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.
Exactly, these unit tests are for functions defined in utils.rs, 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.
Oh, you are right. I'll change it
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.
Done in: e6aac24
…class/cairo_native into move-test-utils-to-testing-module
✅ Code is now correctly formatted. |
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.
Now that these functions are public, maybe they could be reused in our integration tests.
Move test utils to testing module
Closes #1409
Move testing utilities to
utils/testing.rs
.Introduces Breaking Changes?
No.
starknet-blocks.yml
workflow to use these PRs.These PRs should be merged after this one right away, in that order.
Checklist