pull error rendering out of error type, snapshot tests, only return an

error don't print, use named params in fomrat, remove mut from finish()
cache files, capitalization show be lower

crates/sui/src/client_commands.rs

@@ -929,14 +929,7 @@ impl SuiClientCommands {
                     upgrade_package,
                 ) = upgrade_result?;
 
-                check_compatibility(
-                    &client,
-                    package_id,
-                    &compiled_modules,
-                    upgrade_package,
-                    protocol_config,
-                )
-                .await?;
+                check_compatibility(&client, package_id, upgrade_package, protocol_config).await?;
 
                 let tx_kind = client
                     .transaction_builder()

crates/sui/src/unit_tests/fixtures/upgrade_errors/struct_missing_v1/sources/UpgradeErrors.move → crates/sui/src/unit_tests/fixtures/upgrade_errors/declarations_missing_v1/sources/UpgradeErrors.move

@@ -9,5 +9,14 @@ module upgrades::upgrades {
     public struct StructToBeRemoved {
         b: u64
     }
+
+    public enum EnumToBeRemoved {
+        A,
+        B
+    }
+
+    public fun fun_to_be_removed(): u64 {
+        0
+    }
 }
 

crates/sui/src/unit_tests/fixtures/upgrade_errors/struct_missing_v2/sources/UpgradeErrors.move → crates/sui/src/unit_tests/fixtures/upgrade_errors/declarations_missing_v2/sources/UpgradeErrors.move

@@ -5,7 +5,9 @@
 
 #[allow(unused_field)]
 module upgrades::upgrades {
-    // struct missing
+    // removed declarations
     // public struct StructToBeRemoved {}
+    // public enum EnumToBeRemoved {}
+    // public fun fun_to_be_removed() {}
 }
 

crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__declarations_missing.snap

@@ -0,0 +1,30 @@
+---
+source: crates/sui/src/unit_tests/upgrade_compatibility_tests.rs
+expression: normalize_path(err.to_string())
+---
+Upgrade compatibility check failed:
+error: struct is missing
+  ┌─ crates/sui/src/unit_tests/fixtures/upgrade_errors/declarations_missing_v2/sources/UpgradeErrors.move:7:18
+  │
+7 │ module upgrades::upgrades {
+  │                  ^^^^^^^^ Module 'upgrades' expected struct 'StructToBeRemoved', but found none
+  │
+  = The struct is missing in the new module, add the previously defined: 'StructToBeRemoved'
+
+error: enum is missing
+  ┌─ crates/sui/src/unit_tests/fixtures/upgrade_errors/declarations_missing_v2/sources/UpgradeErrors.move:7:18
+  │
+7 │ module upgrades::upgrades {
+  │                  ^^^^^^^^ Module 'upgrades' expected enum 'EnumToBeRemoved', but found none
+  │
+  = The enum is missing in the new module, add the previously defined: 'EnumToBeRemoved'
+
+error: public function is missing
+  ┌─ crates/sui/src/unit_tests/fixtures/upgrade_errors/declarations_missing_v2/sources/UpgradeErrors.move:7:18
+  │
+7 │ module upgrades::upgrades {
+  │                  ^^^^^^^^ Module 'upgrades' expected public function 'fun_to_be_removed', but found none
+  │
+  = The public function is missing in the new module, add the previously defined: 'fun_to_be_removed'
+
+

crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__struct_missing.snap

@@ -2,5 +2,27 @@
 source: crates/sui/src/unit_tests/upgrade_compatibility_tests.rs
 expression: err.to_string()
 ---
-Upgrade compatibility check failed with the following errors:
-- StructMissing { name: Identifier("StructToBeRemoved"), old_struct: Struct { abilities: [], type_parameters: [], fields: [Field { name: Identifier("b"), type_: U64 }] } }
+Upgrade compatibility check failed:
+error: struct is missing
+  ┌─ /Users/jordanjennings/code/sui/crates/sui/src/unit_tests/fixtures/upgrade_errors/declarations_missing_v2/sources/UpgradeErrors.move:7:18
+  │
+7 │ module upgrades::upgrades {
+  │                  ^^^^^^^^ Module 'upgrades' expected struct 'StructToBeRemoved', but found none
+  │
+  = The struct is missing in the new module, add the previously defined: 'StructToBeRemoved'
+
+error: enum is missing
+  ┌─ /Users/jordanjennings/code/sui/crates/sui/src/unit_tests/fixtures/upgrade_errors/declarations_missing_v2/sources/UpgradeErrors.move:7:18
+  │
+7 │ module upgrades::upgrades {
+  │                  ^^^^^^^^ Module 'upgrades' expected enum 'EnumToBeRemoved', but found none
+  │
+  = The enum is missing in the new module, add the previously defined: 'EnumToBeRemoved'
+
+error: public function is missing
+  ┌─ /Users/jordanjennings/code/sui/crates/sui/src/unit_tests/fixtures/upgrade_errors/declarations_missing_v2/sources/UpgradeErrors.move:7:18
+  │
+7 │ module upgrades::upgrades {
+  │                  ^^^^^^^^ Module 'upgrades' expected public function 'fun_to_be_removed', but found none
+  │
+  = The public function is missing in the new module, add the previously defined: 'fun_to_be_removed'

crates/sui/src/unit_tests/upgrade_compatibility_tests.rs

@@ -6,48 +6,56 @@ use insta::assert_snapshot;
 use move_binary_format::CompiledModule;
 use std::path::PathBuf;
 use sui_move_build::BuildConfig;
+use sui_move_build::CompiledPackage;
 
 #[test]
+#[should_panic]
 fn test_all_fail() {
-    let (pkg_v1, pkg_v2) = get_packages("all");
+    let (mods_v1, pkg_v2) = get_packages("all");
 
-    let result = compare_packages(pkg_v1, pkg_v2);
-    assert!(result.is_err());
-    let err = result.unwrap_err();
-
-    assert_snapshot!(err.to_string());
+    // panics: Not all errors are implemented yet
+    compare_packages(mods_v1, pkg_v2, false).unwrap();
 }
 
 #[test]
-fn test_struct_missing() {
-    let (pkg_v1, pkg_v2) = get_packages("struct_missing");
-    let result = compare_packages(pkg_v1, pkg_v2);
+fn test_declarations_missing() {
+    let (pkg_v1, pkg_v2) = get_packages("declarations_missing");
+    let result = compare_packages(pkg_v1, pkg_v2, false);
+
+    // find lines that have   ┌─ /Users/jordanjennings/code/sui/crates/sui/src/unit_tests/fixtures/upgrade_errors/declarations_missing_v2/sources/UpgradeErrors.move:7:18
+    // remove the path before /sui/
+    let result = result.map_err(|err| {
+        let err = err.to_string();
+        let err = err.replace("/Users/jordanjennings/code/sui/", "");
+        err
+    });
+
 
     assert!(result.is_err());
     let err = result.unwrap_err();
-    assert_snapshot!(err.to_string());
+    assert_snapshot!(normalize_path(err.to_string()));
 }
 
 #[test]
 fn test_friend_link_ok() {
     let (pkg_v1, pkg_v2) = get_packages("friend_linking");
     // upgrade compatibility ignores friend linking
-    assert!(compare_packages(pkg_v1, pkg_v2).is_ok());
+    assert!(compare_packages(pkg_v1, pkg_v2, false).is_ok());
 }
 
 #[test]
 fn test_entry_linking_ok() {
     let (pkg_v1, pkg_v2) = get_packages("entry_linking");
     // upgrade compatibility ignores entry linking
-    assert!(compare_packages(pkg_v1, pkg_v2).is_ok());
+    assert!(compare_packages(pkg_v1, pkg_v2, false).is_ok());
 }
 
-fn get_packages(name: &str) -> (Vec<CompiledModule>, Vec<CompiledModule>) {
+fn get_packages(name: &str) -> (Vec<CompiledModule>, CompiledPackage) {
     let mut path: PathBuf = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
     path.push("src/unit_tests/fixtures/upgrade_errors/");
     path.push(format!("{}_v1", name));
 
-    let pkg_v1 = BuildConfig::new_for_testing()
+    let mods_v1 = BuildConfig::new_for_testing()
         .build(&path)
         .unwrap()
         .into_modules();
@@ -56,10 +64,15 @@ fn get_packages(name: &str) -> (Vec<CompiledModule>, Vec<CompiledModule>) {
     path.push("src/unit_tests/fixtures/upgrade_errors/");
     path.push(format!("{}_v2", name));
 
-    let pkg_v2 = BuildConfig::new_for_testing()
-        .build(&path)
-        .unwrap()
-        .into_modules();
+    let pkg_v2 = BuildConfig::new_for_testing().build(&path).unwrap();
 
-    (pkg_v1, pkg_v2)
+    (mods_v1, pkg_v2)
 }
+
+/// Snapshots will differ on each machine, normalize to prevent test failures
+fn normalize_path(err_string: String) -> String {
+    // match the part of the path that is before /sui/ could be any home directory
+    let re = regex::Regex::new(r"(^ ┌─ /[-a-zA-Z0-9_/ .+()]+/sui/)").unwrap();
+    let path = re.replace(&err_string, " ┌─ /sui/");
+    path.to_string()
+}