[testsuite] improve smoke tests for upgrading framework (0LNetworkCom…

…munity#198)

framework/src/upgrade_fixtures/make_upgrade_test_fixtures.md

@@ -1,5 +1,17 @@
+## Upgrade Fixtures
+All fixtures needed to test upgrades are generated on the first run of any test
+that requires it.
 
-## Generate these fixtures
+A test will call:
+`upgrade_fixtures::testsuite_warmup_fixtures();`
+
+It will only build if there are no such fixtures in the expected path
+`framework/src/upgrade_fixtures/fixtures`.
+
+Sometimes if the test aborts the fixtures may be in an incomplete state. So you
+can delete the fixtures or generate manually, see below.
+
+## Generate these fixtures manually
 Making these fixtures is prone to error, so we have some helpers.
 
 TODO: fixtures should be done pre-test in a build.rs
@@ -9,11 +21,14 @@ TODO: fixtures should be done pre-test in a build.rs
 RUST_MIN_STACK=104857600 cargo t -- make_the_upgrade_fixtures --include-ignored
 ```
 
-## Fixtures
+## How the test fixtures work
 
 We want to test that we can modify code in the 0x1 address and upgrade successfully.
 
-To do this we will create a framework upgrade script for MoveStdlib. We will add a module and function that would not otherwise not be there; "all your base"
+To do this we will create a framework upgrade script for MoveStdlib. We will add
+a module and function that would not otherwise not be there;
+"all_your_base.move". That way after the upgrade, when we call that function we
+get a result, instead of an abort (in the case of an upgrade not being successful).
 
 ```
 
@@ -45,4 +60,5 @@ make upgrade_fixtures
 If everything went well you'll have a `script.mv` and a `script_sha3` file here. The script_sha3 should be a different hash that you've had before if there are any changes to the MoveStdlib.
 
 #### 3. Run the test
-Any test that call the newly deployed view function which should return hex enconded "us", which is "0x7573".
+Any test that call the newly deployed view function which should return hex
+enconded "us", which is "0x7573".

framework/src/upgrade_fixtures/mod.rs

@@ -10,22 +10,24 @@ use std::sync::Once;
 
 static INIT: Once = Once::new();
 
-/// helper to create fixtures before the testsuite runs.
-pub fn testsuite_warmup_fixtures() {
+/// testsuite helper to create fixtures before the testsuite runs.
+pub fn testsuite_maybe_warmup_fixtures() {
     INIT.call_once(|| {
-        // don't regenerate
+        // TODO: decide how to force rebuild, or never rebuild. Maybe envvar? or
+        // using build.rs
+
         let fixture_path = fixtures_path();
         let p = fixture_path.join("upgrade-single-lib");
         if p.exists() {
+            // don't regenerate
             return;
         }
         // initialization code here
         upgrade_fixtures().expect("could no warmup upgrade fixtures");
     });
 }
 
-// TODO: This could be generated dynamically at the start of the test suites. using `Once`. Though if the tools aren't compiled it will take approximately forever to do so. Hence fixtures, though not ideal.
-
+/// where we store upgrade fixtures
 pub fn fixtures_path() -> PathBuf {
     let this_crate = PathBuf::from_str(env!("CARGO_MANIFEST_DIR")).unwrap();
     this_crate
@@ -34,6 +36,8 @@ pub fn fixtures_path() -> PathBuf {
         .join("fixtures")
 }
 
+/// add the all_your_base.move file which serves as the canary to test if the
+/// upgrade went through
 pub fn insert_test_file(core_module_name: &str, remove: bool) -> anyhow::Result<()> {
     //1. Copy the allyourbase code to the module.
     let this_crate = PathBuf::from_str(env!("CARGO_MANIFEST_DIR")).unwrap();
@@ -67,7 +71,12 @@ pub fn insert_test_file(core_module_name: &str, remove: bool) -> anyhow::Result<
     Ok(())
 }
 
-pub fn generate_fixtures(output_path: PathBuf, modules: Vec<String>) -> anyhow::Result<()> {
+// like is sounds
+pub fn generate_fixtures(
+    output_path: PathBuf,
+    modules: Vec<String>,
+    force_incompatible_upgrade: bool,
+) -> anyhow::Result<()> {
     println!("generating files, this will take some time, go do some laundry");
     let destination_module = modules.last().unwrap().clone();
     insert_test_file(&destination_module, false).context("could not insert test file")?;
@@ -82,7 +91,7 @@ pub fn generate_fixtures(output_path: PathBuf, modules: Vec<String>) -> anyhow::
         &output_path,
         &libra_framework_sources,
         &Some(modules),
-        false,
+        force_incompatible_upgrade,
     )?;
     // ok, cleanup
     insert_test_file(&destination_module, true)?;
@@ -106,7 +115,7 @@ pub fn upgrade_fixtures() -> anyhow::Result<()> {
     let p = fixture_path.join("upgrade-single-lib");
     std::fs::create_dir_all(&p)?;
     let modules = vec!["move-stdlib".to_string()];
-    generate_fixtures(p, modules)?;
+    generate_fixtures(p, modules, false)?;
 
     // for multi step upgrades
     // places the all_your_base in the libra_framework dir
@@ -118,6 +127,18 @@ pub fn upgrade_fixtures() -> anyhow::Result<()> {
         "libra-framework".to_string(),
     ];
 
-    generate_fixtures(p, modules)?;
+    generate_fixtures(p, modules, false)?;
+
+    // generate fixtures with arbitrary release
+    let p = fixture_path.join("upgrade-multi-lib-force");
+    std::fs::create_dir_all(&p)?;
+    let modules = vec![
+        "move-stdlib".to_string(),
+        "vendor-stdlib".to_string(),
+        "libra-framework".to_string(),
+    ];
+
+    generate_fixtures(p, modules, true)?;
+
     Ok(())
 }

tools/txs/tests/upgrade_multiple.rs → tools/txs/tests/support/mod.rs

@@ -1,7 +1,5 @@
-//! test framework upgrades with multiple steps
-
 use diem_types::chain_id::NamedChain;
-use libra_framework::upgrade_fixtures;
+use libra_framework::{release::ReleaseTarget, upgrade_fixtures};
 use libra_query::query_view;
 use libra_smoke_tests::{configure_validator, libra_smoke::LibraSmoke};
 use libra_txs::{
@@ -10,24 +8,29 @@ use libra_txs::{
 };
 use libra_types::legacy_types::app_cfg::TxCost;
 
-/// Testing that we can upgrade the chain framework using txs tools.
-/// Note: We have another upgrade meta test in ./smoke-tests
-/// We assume a built transaction script for upgrade in tests/fixtures/test_upgrade.
-/// 1. a validator can submit a proposal with txs
-/// 2. the validator can vote for the proposal
-/// 3. check that the proposal is resolvable
-/// 4. resolve a propsosal by sending the upgrade payload.
-/// 5. Check that the new function all_your_base can be called
-#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
-async fn smoke_upgrade_multiple_steps() {
-    upgrade_fixtures::testsuite_warmup_fixtures();
+/// If there are multiple modules being upgraded only one of the modules (the
+/// first) needs to be included in the proposal.
+/// The transaction script which upgrades the first module, also sets the
+/// transaction hash for the subsequent module needed to be upgraded.
+/// these hashes are produced offline during the framework upgrade builder
+/// workflow.
+pub async fn upgrade_multiple_impl(
+    dir_path: &str,
+    modules: Vec<&str>,
+    prior_release: ReleaseTarget,
+) {
+    upgrade_fixtures::testsuite_maybe_warmup_fixtures();
 
     let d = diem_temppath::TempPath::new();
 
-    let mut s = LibraSmoke::new(Some(1))
+    let mut s = LibraSmoke::new_with_target(Some(1), prior_release)
         .await
         .expect("could not start libra smoke");
 
+    // let mut s = LibraSmoke::new(Some(1))
+    //     .await
+    //     .expect("could not start libra smoke");
+
     let (_, _app_cfg) =
         configure_validator::init_val_config_files(&mut s.swarm, 0, d.path().to_owned())
             .await
@@ -39,10 +42,18 @@ async fn smoke_upgrade_multiple_steps() {
         query_view::get_view(&s.client(), "0x1::all_your_base::are_belong_to", None, None).await;
     assert!(query_res.is_err(), "expected all_your_base to fail");
 
-    ///// NOTE THERE ARE MULTIPLE STEPS, we are getting the artifacts for the first step.
+    ///// NOTE THERE ARE MULTIPLE STEPS, we are getting the artifacts for the
+    // first step. This is what sets the governance in motion
+    // we do not need to submit proposals for each subsequent step.
+    // that's because the resolution of the the first step, already
+    // includes the hash of the second step, which gets stored in
+    // advance of the user resolving the step 2 with its transaction.
+
+    //////////// PROPOSAL ////////////
+    // Set up governance proposal, just with first module
     let script_dir = upgrade_fixtures::fixtures_path()
-        .join("upgrade-multi-lib")
-        .join("1-move-stdlib");
+        .join(dir_path)
+        .join(modules[0]); // take first module usually "1-move-stdlib"
     assert!(script_dir.exists(), "can't find upgrade fixtures");
 
     let mut cli = TxsCli {
@@ -64,6 +75,8 @@ async fn smoke_upgrade_multiple_steps() {
         .await
         .expect("cli could not send upgrade proposal");
 
+    //////////// VOTING ////////////
+
     // ALICE VOTES
     cli.subcommand = Some(Governance(Vote {
         proposal_id: 0,
@@ -128,46 +141,29 @@ async fn smoke_upgrade_multiple_steps() {
         "expected this script hash, did you change the fixtures?"
     );
 
-    ///////// SHOW TIME, RESOLVE FIRST STEP 1/3////////
-    // Now try to resolve upgrade
-    cli.subcommand = Some(Governance(Resolve {
-        proposal_id: 0,
-        proposal_script_dir: script_dir,
-    }));
-    cli.run().await.expect("cannot resolve proposal at step 1");
-    //////////////////////////////
+    //////////// RESOLVE ////////////
 
-    ///////// SHOW TIME, RESOLVE SECOND STEP 2/3 ////////
+    for name in modules {
+        ///////// SHOW TIME, RESOLVE EACH STEP ////////
 
-    let script_dir = upgrade_fixtures::fixtures_path()
-        .join("upgrade-multi-lib")
-        .join("2-vendor-stdlib");
-    cli.subcommand = Some(Governance(Resolve {
-        proposal_id: 0,
-        proposal_script_dir: script_dir,
-    }));
-    cli.run().await.expect("cannot resolve proposal at step 2");
-    //////////////////////////////
+        let script_dir = upgrade_fixtures::fixtures_path().join(dir_path).join(name);
 
-    ///////// SHOW TIME, RESOLVE THIRD STEP 3/3 ////////
-    // THIS IS THE STEP THAT CONTAINS THE CHANGED MODULE all_your_base
-    // Now try to resolve upgrade
-    let script_dir = upgrade_fixtures::fixtures_path()
-        .join("upgrade-multi-lib")
-        .join("3-libra-framework");
-    cli.subcommand = Some(Governance(Resolve {
-        proposal_id: 0,
-        proposal_script_dir: script_dir,
-    }));
-    cli.run().await.expect("cannot resolve proposal at step 3");
-    //////////////////////////////
+        cli.subcommand = Some(Governance(Resolve {
+            proposal_id: 0,
+            proposal_script_dir: script_dir,
+        }));
+        cli.run()
+            .await
+            .unwrap_or_else(|_| panic!("cannot resolve proposal at step {name}"));
+    }
 
+    //////////// VERIFY SUCCESS ////////////
     let query_res =
         query_view::get_view(&s.client(), "0x1::all_your_base::are_belong_to", None, None)
             .await
             .expect("no all_your_base module found");
     assert!(&query_res.as_array().unwrap()[0]
         .as_str()
         .unwrap()
-        .contains("7573"));
+        .contains("7573")); // bytes for "us"
 }