ChainSafe / forest

🌲 Rust Filecoin Node Implementation
https://forest.chainsafe.io
Apache License 2.0
637 stars 157 forks source link

Forest integration and unit tests #2945

Open aatifsyed opened 1 year ago

aatifsyed commented 1 year ago

Issue summary Our tests right now are a bit messy, they're split across

Some tests call out to the web, or are otherwise undesirable to run locally - these tend to be put in shell scripts.

~When I do run cargo test, it takes 5 minutes, which is too long for a nice dev-test cycle.~

Additionally, whenever I'm debugging an integration test, I have to write a bunch of boilerplate to actually get logs out of forest via the Assert.

I'd like the following

Other information and links

LesnyRumcajs commented 1 year ago

Are you not able to run the tests locally?

aatifsyed commented 1 year ago

Not our integration tests - they fetch gigabytes from the web, and take ages, hence wanting segregated tests with a single test runner

LesnyRumcajs commented 1 year ago

Could you please elaborate on how those would those segregated tests look like? Unless you're on cellular, they shouldn't take long, even locally. image

aatifsyed commented 1 year ago

Updated the description :)

LesnyRumcajs commented 1 year ago

How will this avoid pulling snapshots to your local machine? If you want to check if you haven't messed up migrations, you need all those snapshots. The thing that we could do is to cache them somewhere.

As for the Rust test duration - don't use cargo test. Use cargo nextest run. You will notice that except for a few tests (which were behind a slow-tests flag once, but it got removed for some reason), they finish in 10s. That's not 5 minutes.

...
... more tests
...
        PASS [   2.547s] forest_message_pool msgpool::tests::test_msg_chains
        PASS [   4.756s] forest_key_management keystore::test::test_read_write_encrypted_keystore
        PASS [   4.776s] forest_key_management keystore::test::test_generate_key
        PASS [   6.039s] forest_libp2p_bitswap::request_manager tests::request_manager_e2e_test
        PASS [   7.838s] forest_utils io::writer_checksum::test::file_writer_fs_buf_writer
        PASS [  19.297s] forest-daemon::import_snapshot_tests importing_bad_snapshot_should_fail
        PASS [  31.672s] forest-cli::config_tests test_download_location_of_proof_parameter_files_env
        PASS [  32.643s] forest-cli::config_tests test_download_location_of_proof_parameter_files_default
------------
     Summary [  32.677s] 243 tests run: 243 passed (1 leaky), 0 skipped

As for whether we use shell, Ruby or Rust for integration tests - that's a detail. It's more important to have the tests in the first place. I'd gladly eliminate shell, though; asserting anything is a nightmare there.

aatifsyed commented 1 year ago

Maybe I should simply get a better PC

        PASS [  10.530s] forest_key_management keystore::test::test_generate_key
        PASS [  10.707s] forest_key_management keystore::test::test_read_write_encrypted_keystore
        PASS [  36.846s] forest_utils io::writer_checksum::test::file_writer_fs_buf_writer
        LEAK [  53.346s] forest_libp2p_bitswap::go_compat tests::bitswap_go_compat_test
        SLOW [> 60.000s] forest-cli::config test_download_location_of_proof_parameter_files_default
        SLOW [> 60.000s] forest-cli::config test_download_location_of_proof_parameter_files_env
        SLOW [> 60.000s] forest-daemon::import_snapshot_tests importing_bad_snapshot_should_fail
        PASS [  77.934s] forest-daemon::import_snapshot_tests importing_bad_snapshot_should_fail
        PASS [ 117.726s] forest-cli::config test_download_location_of_proof_parameter_files_env
        PASS [ 118.712s] forest-cli::config test_download_location_of_proof_parameter_files_default
------------
     Summary [ 118.753s] 227 tests run: 227 passed (3 slow, 1 leaky), 0 skipped

In all seriousness, this is 16 thread 12th gen i5, which shouldn't really struggle :/

LesnyRumcajs commented 1 year ago

Getting a better PC is never a bad idea!