dojoengine / dojo

Dojo is a toolchain for building provable games and autonomous worlds with Cairo
https://dojoengine.org
Apache License 2.0
407 stars 165 forks source link

Migrate testing to use `starknet-foundry` #693

Open tarrencev opened 1 year ago

tarrencev commented 1 year ago

Currently we are using the cairo test runner. This task is to migrate tests to https://github.com/foundry-rs/starknet-foundry

shramee commented 1 year ago

snforge for sozo test

Line of thought, WIP

  1. Adding forge package,
    • max_recursion_depth is missing from our custom blockifier.
    • Sync dojoengine/blockifier to starkware-libs:9238b4d786bb5de2fac68d22257b0479eb8649e4 (Forge dep)
  2. Forge requires compiled targets, perhaps sozo build before forge::run()?
  3. Potential cairo-lang-* version conflicts.
  4. Forge requires corelib
  5. Perhaps pass db directly to forge?
  6. What's predeployed_contracts in forge::run?
  7. Build forge equivalents for forge::run

Guys, lemme know if this makes sense.

tarrencev commented 1 year ago

@shramee perhaps we should adopt a pattern similar to https://docs.swmansion.com/scarb/docs/testing#using-third-party-test-runners

that way we can avoid the dependency resolution challenges with so many different deps wanting different versions of cairo

shramee commented 1 year ago

Yeah, I was thinking sozo build might be required for snforge command, which builds all artifacts before tests. Which might require stepping into Rust to change some stuff?

Edited: I'll check if we can add Dojo plug-in to forge, but chances are slim with compiled binary.

shramee commented 1 year ago

Hi @tarrencev, Just to confirm, why are we switching to foundry? Like what do we expect to be able to do. And whether we should use 0.2.0 or wait for newer version with Cairo 2.1.0 which has signed ints.

shramee commented 1 year ago

WIP

Have sozo test call forge::run to run tests via forge to allow access cheats and other good stuff that comes with Starknet Foundry for better easier tests.

Highlights

Probably some issue with core_lib but looks really good to me, could also be DojoPlugin missing from test_collector in Foundry.

Dev Notes

forge::run args from snforge

package_path: /Users/shramee/www/starknet/starknet_forge_template
lib_path: /Users/shramee/www/starknet/starknet_forge_template/src/lib.cairo
dependencies: [LinkedLibrary { name: "starknet_forge_template", path: "/Users/shramee/www/starknet/starknet_forge_template/src" }]
runner_config: RunnerConfig { test_name_filter: None, exact_match: false, exit_first: false }
corelib: /var/folders/hg/qktjzkwj6w78g7bryxs00ynw0000gn/T/.tmpV76qHb
contracts: HashMap<String, StarknetContractArtifacts>
predeployed_contracts: /var/folders/hg/qktjzkwj6w78g7bryxs00ynw0000gn/T/.tmp0avphg

forge::run args from sozo test

package_path: /Users/shramee/www/starknet/dojo/crates/dojo-core
lib_path: /Users/shramee/www/starknet/dojo/crates/dojo-core/src/lib.cairo
dependencies: []
runner_config: RunnerConfig { test_name_filter: None, exact_match: false, exit_first: false }
corelib: /Users/shramee/Library/Caches/com.swmansion.scarb/registry/std/v2.0.1/core/src
contracts: HashMap<String, StarknetContractArtifacts>
predeployed_contracts: /var/folders/hg/qktjzkwj6w78g7bryxs00ynw0000gn/T/.tmpKco0if
shramee commented 1 year ago

The library was missing PanicDestruct, Screenshot 2023-08-02 at 21 45 44

Upgraded cairo-lang* to 2.0.2 and scarb to 0.5.2 But that wasn't enough. Foundry is using a specific revision of cairo >2.0.2. So added their core-lib to dojo.

Final piece of the puzzle involves copying across sn-foundry test-collector with a patch to replace it in snforge and adding DojoPlugin. But this causes Cairo version conflicts which is hard to resolve with scarb depending on v2.0.2.

What would we like to do?

shramee commented 1 year ago

Okay, I thought patching everything to use 2.0.2 could do it but snfoundry is actually using a cairo-lang-sierra-type-size crate which isn't in v2.0.2. We could try the other way round and patch everything to the newer sn-foundry's cairo version. cairo-lang-sierra-type-size Seems to have types for them signed ints everyones raving about 🤤

tarrencev commented 1 year ago

Ah interesting, thanks for digging in! I'm close to updating the repo to Cairo 2.1.0. Perhaps we should wait for that?

shramee commented 1 year ago

Yeah, sounds good! 👍

ponderingdemocritus commented 7 months ago

can we close this @tarrencev ?

glihm commented 3 months ago

After discussion with SWM team, starknet foundry is not yet ready to be integrated as a library due to the current work on new proc macros.