[REPLACED] Set up memory output comparison for contracts run in starknet-devnet tests by fmoletta · Pull Request #163 · lambdaclass/cairo-vm-py

fmoletta · 2022-11-29T16:15:23Z

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 outputs
Also 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:

Add patched envs build script
Expose write_binary_memory in PyCairoRunner
Add script/target to run each test in both envs and compare memory outputs after each run
Filter failing tests
Add Makefile target compare-memory-devnet
Integrate into CI

Future plans:

Add trace comparison
Produce different named files then running contracts so that we can compare every output instead of the last sets
Run contracts outside of devnet tests
Target other projects that use starknet

This PR has been replaced by its improved version #174

codecov-commenter · 2022-11-29T20:54:25Z

Codecov Report

Merging #163 (c257bcd) into main (37d5b6d) will increase coverage by 1.22%.
The diff coverage is 0.00%.

❗ Current head c257bcd differs from pull request most recent head b17141f. Consider uploading reports for the commit b17141f to get more accurate results

@@            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

Impacted Files	Coverage Δ
src/cairo_runner.rs	`70.00% <0.00%> (-0.53%)`	⬇️
src/ids.rs	`89.13% <0.00%> (+7.59%)`	⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

…point

Oppen

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.

Oppen · 2022-12-06T14:25:12Z

+# Set up the virtual envs
+	scripts/memory_comparator/build_envs.sh
+# Clone the starknet-devnet from github
+	git clone git@github.com:Shard-Labs/starknet-devnet.git


Let's checkout a fixed tag instead of main. We need stability for comparisons.

Oppen · 2022-12-06T14:25:42Z

+# Set up the virtual envs
+	scripts/memory_comparator/build_envs.sh
+# Clone the starknet-devnet from github
+	git clone git@github.com:Shard-Labs/starknet-devnet.git


This belongs into a separate setup target.

Oppen · 2022-12-06T14:27:12Z

+# 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


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.

Oppen · 2022-12-06T14:28:01Z

+#!/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


Why are we copy-pasting this script?

Oppen · 2022-12-06T14:28:20Z

+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


This looks like a lot of duplication.

Oppen · 2022-12-06T14:30:43Z

Nevermind tho, I just realized it's a work in progress 🤦‍♂️

Federica added 18 commits November 29, 2022 11:50

Expose write_binary_memory method

368152e

Add patches and build script

28c9cff

Draft steps on Makefile

d6f45ff

Remove duplicate patch files

2f9e060

Add target to .PHONY

2a20de6

Fix paths in build_envs script

33d1b1e

Add starknet setup

876b8d2

Add more setup

5d6d678

Fix

f1373b6

Start run_tests_compare_memory script

0ced513

Add files to make clean

228f2d6

Move env patching to makefile

cf84748

Add memory comparison to Makefile

64aa871

Finish script

7592c35

Complete makefile target

c48c3ff

Add compare-devnet-memory workflow

0528a42

Fix syntax

fc15bd1

Fix workflow file

1cbb024

fmoletta changed the title ~~Set up memory output comparison for contracts run in starkness-devnet tests~~ Set up memory output comparison for contracts run in starknet-devnet tests Nov 29, 2022

Federica added 2 commits November 29, 2022 17:40

Clippy fix

058f437

Remove ilegal option

ca86762

Federica added 8 commits November 29, 2022 17:59

Add python setup to workflow

cc2e8a0

Fix stuff

4704322

Fix workflow

c33c942

Debugging

a15914c

Debugging

b8042bf

Debugging

e097100

Debug

f083415

Debug

1108e7d

Federica added 18 commits November 30, 2022 15:46

Fix bash syntax

d708d55

Remove debug step

24485e0

Add success message

4d4233a

Insert bug

1164c2f

Replace memory_comparator

9b941b5

Make new comparator executable

3b13b36

Fix indentation

9b2ee59

Filter out file which doesnt produce memory outputs in execute_entry_…

11c66ec

…point

Filter out file which doesnt produce memory outputs

80e7f91

Filter out file which doesnt produce memory outputs

831c2da

Exclude more files

e537487

Remove bug

a7988f5

Merge branch 'main' into memory-comparison-devnet

0662336

Update patch file

deb8d09

Fix patch

4850a57

Include more tests

c257bcd

Fix syntax

4b00ac4

Fix syntax

b17141f

fmoletta marked this pull request as ready for review December 1, 2022 22:03

fmoletta mentioned this pull request Dec 1, 2022

Set up memory + trace output comparison for contracts run in starknet-devnet tests #174

Open

11 tasks

Oppen suggested changes Dec 6, 2022

View reviewed changes

fmoletta changed the title ~~Set up memory output comparison for contracts run in starknet-devnet tests~~ [REPLACED] Set up memory output comparison for contracts run in starknet-devnet tests Dec 7, 2022

fmoletta marked this pull request as draft December 7, 2022 20:50

Jrigada closed this Dec 13, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[REPLACED] Set up memory output comparison for contracts run in starknet-devnet tests#163

[REPLACED] Set up memory output comparison for contracts run in starknet-devnet tests#163
fmoletta wants to merge 46 commits into
mainfrom
memory-comparison-devnet

fmoletta commented Nov 29, 2022 •

edited

Loading

Uh oh!

codecov-commenter commented Nov 29, 2022 •

edited

Loading

Uh oh!

Oppen left a comment

Uh oh!

Oppen Dec 6, 2022

Uh oh!

Oppen Dec 6, 2022

Uh oh!

Oppen Dec 6, 2022

Uh oh!

Oppen Dec 6, 2022

Uh oh!

Oppen Dec 6, 2022

Uh oh!

Oppen commented Dec 6, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

fmoletta commented Nov 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented Nov 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Oppen left a comment

Choose a reason for hiding this comment

Uh oh!

Oppen Dec 6, 2022

Choose a reason for hiding this comment

Uh oh!

Oppen Dec 6, 2022

Choose a reason for hiding this comment

Uh oh!

Oppen Dec 6, 2022

Choose a reason for hiding this comment

Uh oh!

Oppen Dec 6, 2022

Choose a reason for hiding this comment

Uh oh!

Oppen Dec 6, 2022

Choose a reason for hiding this comment

Uh oh!

Oppen commented Dec 6, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

fmoletta commented Nov 29, 2022 •

edited

Loading

codecov-commenter commented Nov 29, 2022 •

edited

Loading