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

[E2E] Improve platform handling, fix faucet tests #12161

Merged
merged 1 commit into from
Feb 22, 2024

Conversation

banool
Copy link
Contributor

@banool banool commented Feb 22, 2024

Description

Improve platform handling in the faucet and CLI e2e tests so you don't have to worry about the docker platform env vars.

Beyond that, I fixed the faucet CI in two ways:

  • The tests for confirming that the faucet works with the existing devnet / testnet were broken due to code changing, but I suppose the tests were disabled so the bad change landed. Broken run.
    • The breakage came from 72f04e0 so the tests have been broken for a hot minute. Only the tests were broken.
  • The tests for confirming that upstream changes don't break compatibility with the existing faucet were broken due to syntax error in the CI that I think I messed up ages ago.

Test Plan

I changed pull_request_target to pull_request temporarily to confirm that this worked: https://github.com/aptos-labs/aptos-core/actions/runs/8001108730/job/21851689189?pr=12161.

The faucet tests are legit and are meant to be reliable signal that something breaks faucet compatibility, so I will make them land blocking after this change lands and looks good for a bit.

This compiles now:

cargo test -p aptos-faucet-core --features integration-tests

Copy link

trunk-io bot commented Feb 22, 2024

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

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

Job Duration vs 7d avg Delta
rust-images / rust-all 20m 13m +48%
windows-build 12m 20m -38%

settingsfeedbackdocs ⋅ learn more about trunk.io

@banool banool added CICD:build-images when this label is present github actions will start build+push rust images from the PR. CICD:run-e2e-tests when this label is present github actions will run all land-blocking e2e tests from the PR CICD:experimental-forge CICD:experimental-build CICD:non-required-tests If this label is present, non-required tests will be run on the PR. and removed CICD:experimental-forge labels Feb 22, 2024

This comment has been minimized.

This comment has been minimized.

@banool banool force-pushed the banool/fix-cli-e2e-tests branch from de013b9 to 1050d74 Compare February 22, 2024 07:42
@banool banool changed the title [E2E] Improve platform handling [E2E] Improve platform handling, fix faucet and faucet tests Feb 22, 2024
@banool banool force-pushed the banool/fix-cli-e2e-tests branch from 1050d74 to 9f487ad Compare February 22, 2024 07:49
@banool banool force-pushed the banool/fix-cli-e2e-tests branch from 9f487ad to e072140 Compare February 22, 2024 07:57
@banool banool marked this pull request as ready for review February 22, 2024 07:57
@banool banool requested review from gregnazario and a team as code owners February 22, 2024 07:57
@banool banool changed the title [E2E] Improve platform handling, fix faucet and faucet tests [E2E] Improve platform handling, fix faucet tests Feb 22, 2024
@banool banool enabled auto-merge (squash) February 22, 2024 08:00

This comment has been minimized.

This comment has been minimized.

Copy link
Contributor

✅ Forge suite compat success on aptos-node-v1.9.5 ==> e072140278b43efa66e9bfd7bed78522c1819fec

Compatibility test results for aptos-node-v1.9.5 ==> e072140278b43efa66e9bfd7bed78522c1819fec (PR)
1. Check liveness of validators at old version: aptos-node-v1.9.5
compatibility::simple-validator-upgrade::liveness-check : committed: 6450 txn/s, latency: 5132 ms, (p50: 4800 ms, p90: 8100 ms, p99: 16000 ms), latency samples: 225760
2. Upgrading first Validator to new version: e072140278b43efa66e9bfd7bed78522c1819fec
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 1875 txn/s, latency: 15891 ms, (p50: 17300 ms, p90: 22000 ms, p99: 22300 ms), latency samples: 97500
3. Upgrading rest of first batch to new version: e072140278b43efa66e9bfd7bed78522c1819fec
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 149 txn/s, submitted: 339 txn/s, expired: 190 txn/s, latency: 38192 ms, (p50: 38700 ms, p90: 96800 ms, p99: 101400 ms), latency samples: 21129
4. upgrading second batch to new version: e072140278b43efa66e9bfd7bed78522c1819fec
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 2609 txn/s, latency: 11042 ms, (p50: 11500 ms, p90: 18100 ms, p99: 19000 ms), latency samples: 127860
5. check swarm health
Compatibility test for aptos-node-v1.9.5 ==> e072140278b43efa66e9bfd7bed78522c1819fec passed
Test Ok

Copy link
Contributor

✅ Forge suite realistic_env_max_load success on e072140278b43efa66e9bfd7bed78522c1819fec

two traffics test: inner traffic : committed: 7833 txn/s, latency: 5008 ms, (p50: 4800 ms, p90: 6000 ms, p99: 10200 ms), latency samples: 3384240
two traffics test : committed: 100 txn/s, latency: 2067 ms, (p50: 2000 ms, p90: 2300 ms, p99: 5400 ms), latency samples: 1800
Latency breakdown for phase 0: ["QsBatchToPos: max: 0.232, avg: 0.204", "QsPosToProposal: max: 0.434, avg: 0.400", "ConsensusProposalToOrdered: max: 0.578, avg: 0.540", "ConsensusOrderedToCommit: max: 0.368, avg: 0.348", "ConsensusProposalToCommit: max: 0.907, avg: 0.888"]
Max round gap was 1 [limit 4] at version 1631767. Max no progress secs was 4.210727 [limit 15] at version 1631767.
Test Ok

Copy link

codecov bot commented Feb 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (d69d3c1) 70.2% compared to head (de013b9) 70.0%.
Report is 1 commits behind head on main.

❗ Current head de013b9 differs from pull request most recent head e072140. Consider uploading reports for the commit e072140 to get more accurate results

Additional details and impacted files
@@             Coverage Diff             @@
##             main   #12161       +/-   ##
===========================================
- Coverage    70.2%    70.0%     -0.3%     
===========================================
  Files         850     2237     +1387     
  Lines      189123   421307   +232184     
===========================================
+ Hits       132950   295058   +162108     
- Misses      56173   126249    +70076     

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

@banool banool merged commit a164676 into main Feb 22, 2024
59 of 81 checks passed
@banool banool deleted the banool/fix-cli-e2e-tests branch February 22, 2024 18:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CICD:build-images when this label is present github actions will start build+push rust images from the PR. CICD:experimental-build 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants