cleanup move_pr.sh to make it suitable for CI for v2 #13652

brmataptos · 2024-06-12T02:32:48Z

Description

Cleanup of the the move_pr.sh script to allow use in CI:

relocate v2 doc test outputs, use v2 to build for v2 integration tests
use nextest -profile smoke-test by default to tolerate rare Heisenbugs by replaying 3x
a few cleanups

Slightly more description:

Small cleanups to move_pr.sh, including actually paying attention to MOVE_PR_PROFILE env var.
Use nextest --profile smoke-test by default to allow re-runs in case of intermittent failures (Heisenbugs). Use env var MOVE_PR_NEXTEST_PROFILE to allow this to be overridden.
Use new MVC_DOCGEN_OUTPUT_DIR flag to move tests to be used with MOVE_COMPILER_V2=true to relocate all move-compiler-v2-generated docs to keep them distinct from v1-generated docs. Check in these v2 docs for comparison.
Clean up a few compiler-v2 verification tests and v1.matched that were left over from Honor #[verify_only] in move-compiler-v2. Add relevant tests from v1. Also add tests of #[deprecated]. #13732.
Add a comment to script tell people what to do if tests aren't running due to an old version of cargo-nextest.

Fix #13651.

Type of Change

Which Components or Systems Does This Change Impact?

How Has This Been Tested?

run repeatedly.

Key Areas to Review

Do we need to update .config/nextest.toml to force cargo-profile = "ci" for [profile.ci]?

Checklist

I have read and followed the CONTRIBUTING doc
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I identified and added all stakeholders and component owners affected by this change as reviewers
I tested both happy and unhappy path of the functionality
I have made corresponding changes to the documentation

trunk-io · 2024-06-12T02:32:52Z

⏱️ 2h 54m total CI duration on this PR

Job	Cumulative Duration	Recent Runs
rust-targeted-unit-tests	48m	🟩 🟩
rust-move-tests	25m	🟩 🟩
execution-performance / single-node-performance	24m	🟩
rust-move-unit-coverage	23m	🟩 🟩
forge-compat-test / forge	13m	🟩
rust-move-unit-coverage	11m	🟩
rust-lints	11m	🟩 🟩
run-tests-main-branch	9m	🟩 🟩
general-lints	4m	🟩 🟩
check-dynamic-deps	3m	🟩 🟩
semgrep/ci	50s	🟩 🟩
file_change_determinator	28s	🟩 🟩
file_change_determinator	23s	🟩 🟩
permission-check	9s	🟩 🟩
permission-check	9s	🟩 🟩
permission-check	6s	🟩 🟩
permission-check	4s	🟩 🟩

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

Job	Duration	vs 7d avg	Delta
rust-targeted-unit-tests	24m	16m
rust-lints	5m	7m

_{settings ⋅ feedback ⋅ docs ⋅ learn more about trunk.io}

codecov · 2024-06-12T02:45:33Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 59.0%. Comparing base (bd0dd6a) to head (8896d25).
Report is 4 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff            @@
##             main   #13652     +/-   ##
=========================================
- Coverage    59.0%    59.0%   -0.1%     
=========================================
  Files         820      820             
  Lines      197451   197455      +4     
=========================================
  Hits       116620   116620             
- Misses      80831    80835      +4

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

wrwg · 2024-06-12T16:20:13Z

third_party/move/scripts/move_pr.sh

@@ -143,31 +143,32 @@ if [ ! -z "$CHECK" ]; then
  )
 fi

+CARGO_OP_PARAMS="--locked --profile $MOVE_PR_PROFILE"


That does not make sense, the parameter is definitely --cargo-profile to select the cargo profile not the nexttest one. It is essential that we use the same profile for cargo build and nextest, which seems to be hampered by this PR. Maybe by accident there is still a nextest profile of the same name, but that is not what we want to select here.

See also here: https://nexte.st/docs/running/#options-and-arguments

Why should those change be necessary in the first place?

As discussed off-line, this was needed due to an outdated version of carg-nextest. I've updated this PR to add a comment to warn about this case and provide a solution, and also correctly read the MOVE_PR_PROFILE environment variable if it is defined.

vineethk

LGTM.

brmataptos · 2024-07-01T00:38:02Z

Ok, I've changed docgen to add a MVC_DOCGEN_OUTPUT_DIR env var (for testing only) to put the V2-generated .md files in a special dir tests/compiler-v2-doc for this test, allowing safe coexistence with V1. I've made it suitable for CI, I think.

brmataptos · 2024-07-01T00:38:12Z

PTAL

…file

… upgrade cargo-nextest

…brary version (#13318)" This reverts commit bbce0f1.

…ibrary version (#13318)" This reverts commit 7e265ac.

…test

github-actions · 2024-07-03T01:45:31Z

✅ Forge suite `compat` success on `1c2ee7082d6eff8c811ee25d6f5a7d00860a75d5` ==> `8896d255dd3e14a447a0813c7daa483638999866`

Compatibility test results for 1c2ee7082d6eff8c811ee25d6f5a7d00860a75d5 ==> 8896d255dd3e14a447a0813c7daa483638999866 (PR)
1. Check liveness of validators at old version: 1c2ee7082d6eff8c811ee25d6f5a7d00860a75d5
compatibility::simple-validator-upgrade::liveness-check : committed: 8978.46162112616 txn/s, latency: 3295.0479710626973 ms, (p50: 2600 ms, p90: 3600 ms, p99: 24100 ms), latency samples: 360780
2. Upgrading first Validator to new version: 8896d255dd3e14a447a0813c7daa483638999866
compatibility::simple-validator-upgrade::single-validator-upgrading : committed: 3354.0970912270595 txn/s, latency: 7134.997704367302 ms, (p50: 8400 ms, p90: 9700 ms, p99: 11400 ms), latency samples: 89300
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 3329.958444928367 txn/s, latency: 9287.986701578404 ms, (p50: 9300 ms, p90: 14700 ms, p99: 15100 ms), latency samples: 135580
3. Upgrading rest of first batch to new version: 8896d255dd3e14a447a0813c7daa483638999866
compatibility::simple-validator-upgrade::half-validator-upgrading : committed: 3279.3957704542186 txn/s, latency: 7249.378053020426 ms, (p50: 8300 ms, p90: 11200 ms, p99: 12700 ms), latency samples: 92040
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 3354.640677747104 txn/s, latency: 9209.489639236732 ms, (p50: 9400 ms, p90: 14400 ms, p99: 14900 ms), latency samples: 134160
4. upgrading second batch to new version: 8896d255dd3e14a447a0813c7daa483638999866
compatibility::simple-validator-upgrade::rest-validator-upgrading : committed: 6074.1745196278625 txn/s, latency: 4575.996514877527 ms, (p50: 4900 ms, p90: 5700 ms, p99: 6300 ms), latency samples: 121660
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 6539.759336810048 txn/s, latency: 5054.702432834515 ms, (p50: 5000 ms, p90: 7200 ms, p99: 9100 ms), latency samples: 228540
5. check swarm health
Compatibility test for 1c2ee7082d6eff8c811ee25d6f5a7d00860a75d5 ==> 8896d255dd3e14a447a0813c7daa483638999866 passed
Test Ok

github-actions · 2024-07-03T01:46:15Z

✅ Forge suite `realistic_env_max_load` success on `8896d255dd3e14a447a0813c7daa483638999866`

two traffics test: inner traffic : committed: 8604.785392109477 txn/s, latency: 4554.756670898742 ms, (p50: 4500 ms, p90: 5700 ms, p99: 9900 ms), latency samples: 3717640
two traffics test : committed: 100.11054654405235 txn/s, latency: 1979.6466666666668 ms, (p50: 2000 ms, p90: 2300 ms, p99: 2700 ms), latency samples: 1800
Latency breakdown for phase 0: ["QsBatchToPos: max: 0.219, avg: 0.211", "QsPosToProposal: max: 0.107, avg: 0.103", "ConsensusProposalToOrdered: max: 0.289, avg: 0.287", "ConsensusOrderedToCommit: max: 0.376, avg: 0.368", "ConsensusProposalToCommit: max: 0.664, avg: 0.655"]
Max round gap was 1 [limit 4] at version 1890627. Max no progress secs was 5.033032 [limit 15] at version 1890627.
Test Ok

github-actions · 2024-07-03T01:48:04Z

✅ Forge suite `framework_upgrade` success on `1c2ee7082d6eff8c811ee25d6f5a7d00860a75d5` ==> `8896d255dd3e14a447a0813c7daa483638999866`

Compatibility test results for 1c2ee7082d6eff8c811ee25d6f5a7d00860a75d5 ==> 8896d255dd3e14a447a0813c7daa483638999866 (PR)
Upgrade the nodes to version: 8896d255dd3e14a447a0813c7daa483638999866
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1144.2124725689887 txn/s, submitted: 1146.0258045381945 txn/s, failed submission: 1.8133319692060041 txn/s, expired: 1.8133319692060041 txn/s, latency: 2781.6946810618065 ms, (p50: 2100 ms, p90: 5400 ms, p99: 9700 ms), latency samples: 100960
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1047.2358591180412 txn/s, submitted: 1050.0869724517656 txn/s, failed submission: 2.85111333372451 txn/s, expired: 2.85111333372451 txn/s, latency: 2795.9034659685863 ms, (p50: 2100 ms, p90: 4900 ms, p99: 9200 ms), latency samples: 95500
5. check swarm health
Compatibility test for 1c2ee7082d6eff8c811ee25d6f5a7d00860a75d5 ==> 8896d255dd3e14a447a0813c7daa483638999866 passed
Upgrade the remaining nodes to version: 8896d255dd3e14a447a0813c7daa483638999866
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1071.661317195762 txn/s, submitted: 1074.1673279290003 txn/s, failed submission: 2.506010733238389 txn/s, expired: 2.506010733238389 txn/s, latency: 3075.202168367347 ms, (p50: 2300 ms, p90: 5400 ms, p99: 10200 ms), latency samples: 94080
Test Ok

brmataptos requested review from wrwg and rahxephon89 June 12, 2024 02:32

wrwg reviewed Jun 12, 2024

View reviewed changes

brmataptos requested review from vineethk and fEst1ck June 13, 2024 16:57

brmataptos changed the title ~~cargo nextest --cargo-profile ci seems to be broken, just use --profile~~ some cleanup of move_pr.sh to actually read MOVE_PR_PROFILE from the environment Jun 13, 2024

brmataptos requested a review from wrwg June 18, 2024 02:10

vineethk approved these changes Jun 24, 2024

View reviewed changes

brmataptos force-pushed the brm-issue-13651 branch from 34e7e4a to b692358 Compare June 24, 2024 17:05

brmataptos requested review from gregnazario, banool, areshand, davidiw and movekevin as code owners June 24, 2024 17:05

brmataptos requested a review from vineethk July 1, 2024 00:36

brmataptos force-pushed the brm-issue-13651 branch from b692358 to c9ed68a Compare July 1, 2024 00:36

brmataptos changed the title ~~some cleanup of move_pr.sh to actually read MOVE_PR_PROFILE from the environment~~ cleanup move_pr.sh to make it suitable for CI for v2 Jul 1, 2024

wrwg approved these changes Jul 1, 2024

View reviewed changes

brmataptos added 8 commits July 2, 2024 08:59

cargo nextest --cargo-profile ci seems to be broken, just use --pro…

4c3717b

…file

fix script to use nextest --cargo-profile again since it works if you…

d417b9f

… upgrade cargo-nextest

add a comment to help future people with problems

8e36b7f

Revert "[Feature] [Package System] Add option to override standard li…

eea9f88

…brary version (#13318)" This reverts commit bbce0f1.

move move_pr.sh v2 docs for aptos-framework to avoid conflicts with v1

3e9808f

update test outputs

ca6a891

update more v2 doc outputs under aptos-move

d831899

fix v1.matched for verification tests, fix some stray files

23ddedb

brmataptos added 4 commits July 2, 2024 08:59

Reapply "[Feature] [Package System] Add option to override standard l…

0e123f9

…ibrary version (#13318)" This reverts commit 7e265ac.

Use nextest -profile ci by default to use replays to handle heisenbugs

f61fb59

update test outputs, add -t to -a

1b0151f

remove unneeded changes to move_pr.sh script, leaving more sturdy v2 …

c5fc1a8

…test

brmataptos force-pushed the brm-issue-13651 branch from c9ed68a to c5fc1a8 Compare July 2, 2024 16:00

Merge branch 'main' into brm-issue-13651

fe8e9ea

brmataptos enabled auto-merge (squash) July 2, 2024 16:32

vineethk approved these changes Jul 2, 2024

View reviewed changes

This comment has been minimized.

Sign in to view

Merge branch 'main' into brm-issue-13651

8896d25

This comment has been minimized.

Sign in to view

brmataptos merged commit 00f5930 into main Jul 3, 2024
46 of 49 checks passed

brmataptos deleted the brm-issue-13651 branch July 3, 2024 03:27

brmataptos mentioned this pull request Jul 3, 2024

add missing tests/compiler-v2/doc/ files for move_pr.sh #13918

Draft

21 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cleanup move_pr.sh to make it suitable for CI for v2 #13652

cleanup move_pr.sh to make it suitable for CI for v2 #13652

brmataptos commented Jun 12, 2024 •

edited

Loading

trunk-io bot commented Jun 12, 2024 •

edited

Loading

codecov bot commented Jun 12, 2024 •

edited

Loading

wrwg Jun 12, 2024

brmataptos Jun 18, 2024

vineethk left a comment

brmataptos commented Jul 1, 2024

brmataptos commented Jul 1, 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.

github-actions bot commented Jul 3, 2024

github-actions bot commented Jul 3, 2024

github-actions bot commented Jul 3, 2024

cleanup move_pr.sh to make it suitable for CI for v2 #13652

cleanup move_pr.sh to make it suitable for CI for v2 #13652

Conversation

brmataptos commented Jun 12, 2024 • edited Loading

Description

Type of Change

Which Components or Systems Does This Change Impact?

How Has This Been Tested?

Key Areas to Review

Checklist

trunk-io bot commented Jun 12, 2024 • edited Loading

codecov bot commented Jun 12, 2024 • edited Loading

Codecov Report

wrwg Jun 12, 2024

Choose a reason for hiding this comment

brmataptos Jun 18, 2024

Choose a reason for hiding this comment

vineethk left a comment

Choose a reason for hiding this comment

brmataptos commented Jul 1, 2024

brmataptos commented Jul 1, 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.

github-actions bot commented Jul 3, 2024

✅ Forge suite compat success on 1c2ee7082d6eff8c811ee25d6f5a7d00860a75d5 ==> 8896d255dd3e14a447a0813c7daa483638999866

github-actions bot commented Jul 3, 2024

✅ Forge suite realistic_env_max_load success on 8896d255dd3e14a447a0813c7daa483638999866

github-actions bot commented Jul 3, 2024

✅ Forge suite framework_upgrade success on 1c2ee7082d6eff8c811ee25d6f5a7d00860a75d5 ==> 8896d255dd3e14a447a0813c7daa483638999866

brmataptos commented Jun 12, 2024 •

edited

Loading

trunk-io bot commented Jun 12, 2024 •

edited

Loading

codecov bot commented Jun 12, 2024 •

edited

Loading

✅ Forge suite `compat` success on `1c2ee7082d6eff8c811ee25d6f5a7d00860a75d5` ==> `8896d255dd3e14a447a0813c7daa483638999866`

✅ Forge suite `realistic_env_max_load` success on `8896d255dd3e14a447a0813c7daa483638999866`

✅ Forge suite `framework_upgrade` success on `1c2ee7082d6eff8c811ee25d6f5a7d00860a75d5` ==> `8896d255dd3e14a447a0813c7daa483638999866`