0xPolygonMiden / miden-client

Client library that facilitates interaction with the Miden rollup
MIT License
25 stars 22 forks source link

feat: add CLI integration tests using assert_cmd #353

Closed mFragaBA closed 4 weeks ago

mFragaBA commented 1 month ago

closes #335

mFragaBA commented 1 month ago
* In cases where the client sync got stuck, the node was being spammed with requests.

Nice catch! I added a sleep in between syncs

* The `test_import_pending_notes` failed despite not being a `cli_test`.

we debugged this on a huddle and it was a bug when traversing the mmr indices on get_and_store_authenticated_block. Fixed it on 24fe59e with @igamigo suggestion.

* Is there a way to run the genesis account test without restarting the node each time?

We could change the genesis accounts so they're public accounts, but we'd need to use the node on next because #365 got merged to next but isn't on next yet.

* Is there a way to run the tests with a node that isn't in the miden-client directory? (e.g. the local docker node)

you could technically run them with a dockerized node by copying the genesis account files out and running:

mkdir -p ./miden-node/accounts
cp <path_to_genesis_account_files_extracted_from_node> ./miden-node/accounts/
cargo make integration-tests

that's kind of anoying though, and still wouldn't allow you to run integration tests against a remote node. Perhaps it'd be better to set the genesis test as ignored and have a Makefile command to run integration tests with the genesis test included and without. What do you think? Are there any other possibilities?

mFragaBA commented 1 month ago

@igamigo @tomyrd I ended up going with ignoring the test by default and enabling it on the integration-test-full target. integration-test-full is the one ran on the CI and we leave the integration-test to run without the ignored test