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

[CI/CD] Integrate cargo-machete to remove unused deps. #12724

Merged
merged 1 commit into from Apr 2, 2024
Merged

Conversation

JoshLind
Copy link
Contributor

@JoshLind JoshLind commented Mar 28, 2024

Description

This PR integrates cargo-machete into our CI/CD pipeline to detect unused dependencies. The detection step is added to the linter workflow and will fail when unused dependencies are added.

To ensure this passes, I've gone through and removed all unused dependencies as reported by cargo-machete. Exceptions have been added in some cases to account for false positives (thankfully, these are not super common).

Note: it does look like this PR has reduced compilation times on various CI/CD jobs and tests, but we'll need to see enough runs to quantify/trust it.

Testing Plan

Manual verification and existing test infrastructure.

Note:

  1. Some non-required CI/CD jobs are failing, but these appear to be failing already on main, so it doesn't look like anything new in CI/CD has been broken.
  2. I slightly suspect some things (not tested in CI/CD) may be broken by this PR, but if/when these occur, I can work with authors to fix them and integrate them into CI/CD to prevent future breakages.

Copy link

trunk-io bot commented Mar 28, 2024

⏱️ 57h 59m total CI duration on this PR
Job Cumulative Duration Recent Runs
windows-build 6h 35m 🟩🟩🟩🟩🟩 (+14 more)
rust-unit-tests 5h 21m 🟥🟩🟩 (+12 more)
rust-unit-coverage 4h 27m 🟩
execution-performance / single-node-performance 4h 8m 🟩🟩🟩🟩🟩 (+6 more)
rust-smoke-coverage 3h 59m 🟩
rust-smoke-tests 3h 47m 🟩🟩🟩 (+4 more)
rust-move-unit-coverage 3h 26m 🟩🟩🟩🟩 (+9 more)
rust-network-perf-smoke-test 2h 19m 🟩🟩🟩🟩 (+4 more)
rust-images / rust-all 1h 52m 🟩🟩🟩🟩 (+4 more)
rust-lints 1h 50m 🟥🟥🟩🟥🟩 (+12 more)
forge-e2e-test / forge 1h 33m 🟩🟩🟩🟩 (+2 more)
forge-compat-test / forge 1h 29m 🟩🟩🟩🟩 (+2 more)
run-gas-calibration 1h 26m 🟩🟩🟩🟩 (+4 more)
run-tests-local-testnet 1h 24m 🟥🟥🟥🟥 (+4 more)
run-tests-devnet 1h 21m 🟩🟩🟩🟩🟩 (+6 more)
run-tests-testnet 1h 20m 🟩🟩🟩🟩🟩 (+6 more)
run-tests-main-branch 1h 13m 🟩🟩🟩🟩🟩 (+12 more)
rust-cryptohasher-domain-separation-check 1h 9m 🟥🟥🟥🟥 (+4 more)
check 1h 8m 🟩🟩🟩🟩🟩 (+13 more)
cli-e2e-tests / run-cli-tests 1h 1m 🟩🟥🟥🟥 (+2 more)
run-tests-devnet 55m 🟩🟩🟩🟩🟩 (+6 more)
run-tests-mainnet 55m 🟩🟩🟩🟩🟩 (+6 more)
run-tests-testnet 55m 🟩🟩🟩🟩🟩 (+6 more)
rust-network-perf-unit-test 54m 🟩🟩🟩🟩🟩 (+4 more)
faucet-tests-main / run-tests-main 54m 🟩🟩🟩🟩🟩 (+2 more)
check-dynamic-deps 39m 🟩🟩🟩🟩🟩 (+14 more)
general-lints 36m 🟩🟩🟩🟩🟩 (+12 more)
run-python-examples 24m 🟥🟥🟥🟥🟥 (+6 more)
indexer-grpc-e2e-tests / test-indexer-grpc-docker-compose 17m 🟩🟩🟩🟩🟩 (+2 more)
run-examples 12m 🟩🟩🟩🟩🟩 (+6 more)
semgrep/ci 7m 🟩🟩🟩🟩🟩 (+14 more)
node-api-compatibility-tests / node-api-compatibility-tests 6m 🟩🟩🟩🟩🟩 (+2 more)
file_change_determinator 4m 🟩🟩🟩🟩🟩 (+12 more)
file_change_determinator 3m 🟩🟩🟩🟩🟩 (+12 more)
file_change_determinator 3m 🟩🟩🟩🟩🟩 (+11 more)
execution-performance / file_change_determinator 2m 🟩🟩🟩🟩🟩 (+6 more)
file_change_determinator 2m 🟩🟩🟩🟩🟩 (+4 more)
permission-check 1m 🟩🟩🟩🟩🟩 (+13 more)
rust-move-tests 56s 🟩🟩🟩🟩🟩 (+12 more)
permission-check 53s 🟩🟩🟩🟩🟩 (+13 more)
permission-check 48s 🟩🟩🟩🟩🟩 (+11 more)
permission-check 45s 🟩🟩🟩🟩🟩 (+11 more)
permission-check 24s 🟩🟩🟩🟩🟩 (+4 more)
determine-docker-build-metadata 18s 🟩🟩🟩🟩🟩 (+4 more)
upload-to-codecov 17s 🟩

🚨 2 jobs on the last run were significantly faster/slower than expected

Job Duration vs 7d avg Delta
cli-e2e-tests / run-cli-tests 9m 6m +36%
rust-move-tests 3s 7m -99%

settingsfeedbackdocs ⋅ learn more about trunk.io

Copy link

codecov bot commented Mar 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 62.6%. Comparing base (7eda6b5) to head (7c7421e).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##             main   #12724     +/-   ##
=========================================
- Coverage    62.6%    62.6%   -0.1%     
=========================================
  Files         819      819             
  Lines      183454   183542     +88     
=========================================
- Hits       115015   115009      -6     
- Misses      68439    68533     +94     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@JoshLind JoshLind force-pushed the reduce_deps branch 6 times, most recently from 9667b1a to b980f05 Compare March 29, 2024 00:55
@JoshLind JoshLind added CICD:run-e2e-tests when this label is present github actions will run all land-blocking e2e tests from the PR CICD:non-required-tests If this label is present, non-required tests will be run on the PR. CICD:run-windows-tests Runs Windows related CI/CD tests. labels Mar 29, 2024

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

@JoshLind JoshLind force-pushed the reduce_deps branch 2 times, most recently from 2c61c8a to e01c419 Compare March 29, 2024 21:21

This comment has been minimized.

This comment has been minimized.

Copy link
Contributor

@msmouse msmouse left a comment

Choose a reason for hiding this comment

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

Thanks boss!

@JoshLind JoshLind merged commit 36189f5 into main Apr 2, 2024
52 of 56 checks passed
@JoshLind JoshLind deleted the reduce_deps branch April 2, 2024 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CICD:non-required-tests If this label is present, non-required tests will be run on the PR. CICD:run-e2e-tests when this label is present github actions will run all land-blocking e2e tests from the PR CICD:run-windows-tests Runs Windows related CI/CD tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants