-
Notifications
You must be signed in to change notification settings - Fork 6
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
[REPLACED] Set up memory output comparison for contracts run in starknet-devnet tests #163
Conversation
Codecov Report
@@ Coverage Diff @@
## main #163 +/- ##
==========================================
+ Coverage 81.23% 82.45% +1.22%
==========================================
Files 13 13
Lines 714 724 +10
==========================================
+ Hits 580 597 +17
+ Misses 134 127 -7
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
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.
There's a lot of unnecessary duplication of code. There's also duplicate work being performed that should have proper handling via dependency management in the Makefile.
# Set up the virtual envs | ||
scripts/memory_comparator/build_envs.sh | ||
# Clone the starknet-devnet from github | ||
git clone [email protected]:Shard-Labs/starknet-devnet.git |
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.
Let's checkout a fixed tag instead of main
. We need stability for comparisons.
# Set up the virtual envs | ||
scripts/memory_comparator/build_envs.sh | ||
# Clone the starknet-devnet from github | ||
git clone [email protected]:Shard-Labs/starknet-devnet.git |
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.
This belongs into a separate setup
target.
# cairo-lang | ||
. scripts/memory_comparator/cairo-lang/bin/activate && \ | ||
pip install starknet-devnet && \ | ||
cd starknet-devnet; scripts/install_dev_tools.sh | ||
# Create the folder where we will store the memory outputs | ||
cd starknet-devnet; mkdir memory_files | ||
# Compile test files | ||
. scripts/memory_comparator/cairo-lang/bin/activate && \ | ||
cd starknet-devnet; scripts/compile_contracts.sh | ||
# Patch both envs | ||
patch --directory scripts/memory_comparator/cairo-rs-py/lib/python3.9/site-packages/ --strip 2 < scripts/memory_comparator/output-memory-cairo-rs-py.patch | ||
patch --directory scripts/memory_comparator/cairo-lang/lib/python3.9/site-packages/ --strip 2 < scripts/memory_comparator/output-memory-cairo-lang.patch |
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.
All the cairo-lang
commands can run from a different target that builds the outputs, so it only runs once if we need to repeatedly run this test.
#!/bin/sh | ||
|
||
set -e | ||
|
||
# This is not reaaaaally a robust way to find it, but you need to be actively | ||
# trying to break it for this to fail :) | ||
SCRIPT_DIR="scripts/memory_comparator" | ||
|
||
python3.9 -m venv --upgrade-deps ${SCRIPT_DIR}/cairo-lang ${SCRIPT_DIR}/cairo-rs-py | ||
${SCRIPT_DIR}/cairo-lang/bin/pip install cairo-lang==0.10.1 | ||
${SCRIPT_DIR}/cairo-rs-py/bin/pip install maturin==0.14.1 cairo-lang==0.10.1 | ||
${SCRIPT_DIR}/cairo-rs-py/bin/maturin build --manifest-path Cargo.toml --release --strip --interpreter 3.9 --no-default-features --features extension | ||
${SCRIPT_DIR}/cairo-rs-py/bin/pip install target/wheels/cairo_rs_py-*.whl | ||
|
||
${SCRIPT_DIR}/cairo-rs-py/bin/cairo-run --version | ||
${SCRIPT_DIR}/cairo-rs-py/bin/starknet --version |
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 copy-pasting this script?
compare_memory_devnet_ci: | ||
# Set up the virtual envs | ||
scripts/memory_comparator/build_envs.sh | ||
# Set up the starknet-devnet in each env | ||
# cairo-rs-py | ||
. scripts/memory_comparator/cairo-rs-py/bin/activate && \ | ||
pip install starknet-devnet && \ | ||
cd starknet-devnet; scripts/install_dev_tools.sh | ||
# cairo-lang | ||
. scripts/memory_comparator/cairo-lang/bin/activate && \ | ||
pip install starknet-devnet && \ | ||
cd starknet-devnet; scripts/install_dev_tools.sh | ||
# Create the folder where we will store the memory outputs | ||
cd starknet-devnet; mkdir memory_files | ||
# Compile test files | ||
. scripts/memory_comparator/cairo-lang/bin/activate && \ | ||
cd starknet-devnet; scripts/compile_contracts.sh | ||
# Patch both envs | ||
patch --directory scripts/memory_comparator/cairo-rs-py/lib/python3.9/site-packages/ --strip 2 < scripts/memory_comparator/output-memory-cairo-rs-py.patch | ||
patch --directory scripts/memory_comparator/cairo-lang/lib/python3.9/site-packages/ --strip 2 < scripts/memory_comparator/output-memory-cairo-lang.patch | ||
# Run each test one by one in each env and run the memory comparator | ||
. ./scripts/memory_comparator/run_tests_compare_memory.sh |
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.
This looks like a lot of duplication.
Nevermind tho, I just realized it's a work in progress 🤦♂️ |
Add a Makefile target
compare_memory_devnet
that runs the relevant starknet-devnet tests with cairo-rs-py runner and the original runner and compares the relocated memory outputsAlso add a workflow that runs this target
NOTE: Current CI time for this new workflow is 1 hour, some of it can be reduced by using cache
Steps:
write_binary_memory
in PyCairoRunnerFuture plans:
This PR has been replaced by its improved version #174