Skip to content
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

fix: Ensure we test all builds/tests (doctests, some build configs) #1895

Merged
merged 7 commits into from
Sep 3, 2024

Conversation

dhedey
Copy link
Contributor

@dhedey dhedey commented Sep 1, 2024

Summary

Further tweaks off the back of #1891 to make our testing more comprehensive / reproducible. Just lots of tech debt which I've finally addressed and fixed up. Details below.

Details

  • Fixes doctests
  • Ensures doctests run in CI and locally
  • Removed helpers from test.sh to make what it does more obvious / explicit
  • Updates ./update-cargo-lock-minimally.sh to fix all the Cargo lock files and lock templates, if you hit a problem caused by --locked flags in CI.
  • Fixes compilation of radix-engine-profiling and add that build to CI
  • Removes unneeded / outdated radix-engine dev dependencies in lots of test blueprint Cargo.tomls which caused errors when building them.
  • Fixes resim/tests/resim.sh which tried to output to a target directory which no longer existed (this fixes an error which got merged with arose from build: Use Cargo.lock for all crates #1891 when run on a new CI bot)

Testing

CI tests pass. Doctests re-enabled.

Update Recommendations

None. For once :D

Copy link

github-actions bot commented Sep 1, 2024

Phylum Report Link

Copy link

github-actions bot commented Sep 1, 2024

Docker tags
docker.io/radixdlt/private-scrypto-builder:1412be4d60

Copy link

github-actions bot commented Sep 1, 2024

Benchmark for 1412be4

Click to view benchmark
Test Base PR %
costing::bench_prepare_wasm 63.6±0.11ms 62.7±0.12ms -1.42%
costing::decode_encoded_i8_array_to_manifest_raw_value 19.6±0.01ms 19.3±0.01ms -1.53%
costing::decode_encoded_i8_array_to_manifest_value 41.4±0.05ms 41.6±0.07ms +0.48%
costing::decode_encoded_tuple_array_to_manifest_raw_value 63.1±0.06ms 63.0±0.15ms -0.16%
costing::decode_encoded_tuple_array_to_manifest_value 118.6±1.22ms 108.9±0.21ms -8.18%
costing::decode_encoded_u8_array_to_manifest_raw_value 26.3±0.09µs 26.0±0.03µs -1.14%
costing::decode_encoded_u8_array_to_manifest_value 41.3±0.07ms 41.5±0.09ms +0.48%
costing::decode_rpd_to_manifest_raw_value 12.3±0.02µs 12.5±0.03µs +1.63%
costing::decode_rpd_to_manifest_value 10.7±0.03µs 10.8±0.03µs +0.93%
costing::deserialize_wasm 1247.1±3.81µs 1249.0±3.99µs +0.15%
costing::execute_transaction_creating_big_vec_substates 706.3±12.73ms 707.1±15.35ms +0.11%
costing::execute_transaction_reading_big_vec_substates 589.5±1.13ms 584.2±1.92ms -0.90%
costing::instantiate_flash_loan 1031.8±1120.79µs 1022.1±987.37µs -0.94%
costing::instantiate_radiswap 951.2±584.24µs 1022.1±955.68µs +7.45%
costing::spin_loop 20.9±0.12ms 20.5±0.07ms -1.91%
costing::validate_sbor_payload 27.5±0.05µs 27.3±0.08µs -0.73%
costing::validate_sbor_payload_bytes 204.1±0.53ns 200.7±0.48ns -1.67%
costing::validate_secp256k1 76.6±0.32µs 76.8±0.23µs +0.26%
costing::validate_wasm 33.7±0.03ms 33.8±0.03ms +0.30%
decimal::add/0 8.4±0.00ns 8.4±0.00ns 0.00%
decimal::add/rust-native 9.9±0.01ns 9.9±0.01ns 0.00%
decimal::add/wasmi 225.7±0.29ns 220.5±0.50ns -2.30%
decimal::add/wasmi-call-native 2.1±0.01µs 2.1±0.00µs 0.00%
decimal::div/0 185.4±0.08ns 187.2±0.27ns +0.97%
decimal::from_string/0 153.5±0.21ns 154.4±0.15ns +0.59%
decimal::mul/0 148.7±0.35ns 149.0±0.13ns +0.20%
decimal::mul/rust-native 149.0±0.21ns 144.4±0.17ns -3.09%
decimal::mul/wasmi 11.9±0.02µs 11.7±0.10µs -1.68%
decimal::mul/wasmi-call-native 2.3±0.00µs 2.2±0.00µs -4.35%
decimal::pow/0 702.9±0.60ns 699.6±0.70ns -0.47%
decimal::pow/rust-native 667.3±0.96ns 672.4±1.25ns +0.76%
decimal::pow/wasmi 57.3±0.39µs 57.2±0.46µs -0.17%
decimal::pow/wasmi-call-native 2.5±0.00µs 2.4±0.01µs -4.00%
decimal::root/0 8.0±0.01µs 8.1±0.01µs +1.25%
decimal::sub/0 8.4±0.02ns 8.4±0.03ns 0.00%
decimal::to_string/0 452.6±0.52ns 440.9±0.70ns -2.59%
precise_decimal::add/0 8.9±0.03ns 9.0±0.05ns +1.12%
precise_decimal::add/rust-native 10.7±0.02ns 10.7±0.03ns 0.00%
precise_decimal::add/wasmi 288.4±0.52ns 280.8±1.11ns -2.64%
precise_decimal::add/wasmi-call-native 2.8±0.00µs 2.8±0.01µs 0.00%
precise_decimal::div/0 316.7±0.22ns 316.3±1.69ns -0.13%
precise_decimal::from_string/0 213.0±0.41ns 213.3±0.84ns +0.14%
precise_decimal::mul/0 368.6±1.40ns 364.5±1.52ns -1.11%
precise_decimal::mul/rust-native 321.2±0.55ns 318.1±0.21ns -0.97%
precise_decimal::mul/wasmi 34.2±0.14µs 33.7±0.13µs -1.46%
precise_decimal::mul/wasmi-call-native 3.2±0.00µs 3.2±0.01µs 0.00%
precise_decimal::pow/0 1953.6±5.71ns 1928.4±1.20ns -1.29%
precise_decimal::pow/rust-native 1516.3±0.61ns 1524.5±2.26ns +0.54%
precise_decimal::pow/wasmi 164.8±1.78µs 164.6±0.46µs -0.12%
precise_decimal::pow/wasmi-call-native 5.7±0.00µs 5.7±0.01µs 0.00%
precise_decimal::root/0 58.2±0.04µs 58.1±0.03µs -0.17%
precise_decimal::sub/0 9.0±0.02ns 9.2±0.08ns +2.22%
precise_decimal::to_string/0 721.8±1.15ns 695.7±1.77ns -3.62%
schema::validate_payload 365.5±0.59µs 365.3±0.42µs -0.05%
transaction::radiswap 5.3±0.03ms 5.2±0.02ms -1.89%
transaction::transfer 1942.3±4.32µs 1925.9±4.32µs -0.84%
transaction_processing::prepare 2.6±0.00ms 2.6±0.00ms 0.00%
transaction_processing::prepare_and_decompile 6.3±0.01ms 6.3±0.02ms 0.00%
transaction_processing::prepare_and_decompile_and_recompile 31.6±0.33ms 25.1±0.46ms -20.57%
transaction_validation::validate_manifest 43.0±0.08µs 42.9±0.05µs -0.23%
transaction_validation::verify_bls_2KB 1063.1±10.02µs 1003.8±16.03µs -5.58%
transaction_validation::verify_bls_32B 1000.8±3.92µs 1001.2±8.53µs +0.04%
transaction_validation::verify_ecdsa 74.5±0.06µs 74.6±0.08µs +0.13%
transaction_validation::verify_ed25519 56.9±0.37µs 55.2±0.05µs -2.99%

@dhedey dhedey changed the title fix: Doc tests re-enabled fix: Ensure we test all builds/tests (doctests, some build configs) Sep 3, 2024
@dhedey dhedey marked this pull request as ready for review September 3, 2024 09:05
@@ -419,18 +419,17 @@ checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd"

[[package]]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated our version of chrono because of this error when compiling plotters: plotters-rs/plotters#624

An example of why using a lock file is very helpful 👍

@dhedey dhedey changed the base branch from feature/cargo-lock to develop September 3, 2024 09:55
@iamyulong iamyulong merged commit 037044f into develop Sep 3, 2024
31 checks passed
@lrubasze
Copy link
Contributor

lrubasze commented Sep 4, 2024

Late to the party... But really impressive work! @dhedey 👏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants