Refactor movey-login logic, refactor movey tests

Author: ea-open-source

language/move-command-line-common/src/lib.rs

@@ -8,7 +8,7 @@ pub mod address;
 pub mod character_sets;
 pub mod env;
 pub mod files;
-pub mod movey;
+pub mod movey_constants;
 pub mod parser;
 pub mod testing;
 pub mod types;

language/move-command-line-common/src/movey.rs renamed to language/move-command-line-common/src/movey_constants.rs

@@ -7,6 +7,7 @@ pub mod disassemble;
 pub mod docgen;
 pub mod errmap;
 pub mod info;
+pub mod movey_login;
 pub mod new;
 pub mod prove;
 pub mod test;

language/tools/move-cli/src/base/mod.rs

@@ -3,100 +3,85 @@
 // SPDX-License-Identifier: Apache-2.0
 
 use anyhow::{bail, Result};
-use move_command_line_common::{env::MOVE_HOME, movey::MOVEY_URL};
+use clap::Parser;
+use move_command_line_common::{env::MOVE_HOME, movey_constants::MOVEY_URL};
 use std::{fs, fs::File, io, path::PathBuf};
 use toml_edit::easy::{map::Map, Value};
 
-pub const MOVEY_API_KEY_PATH: &str = "/movey_api_key.toml";
-
-pub struct TestMode {
-    pub test_path: String,
-}
-
-pub fn handle_movey_login_commands(test_path: Option<String>) -> Result<()> {
-    println!(
-        "Please paste the API Token found on {}/settings/tokens below",
-        MOVEY_URL
-    );
-    let mut line = String::new();
-    loop {
-        match io::stdin().read_line(&mut line) {
-            Ok(_) => {
-                if let Some('\n') = line.chars().next_back() {
-                    line.pop();
+pub const MOVEY_CREDENTIAL_PATH: &str = "/movey_credential.toml";
+
+#[derive(Parser)]
+#[clap(name = "movey-login")]
+pub struct MoveyLogin;
+
+impl MoveyLogin {
+    pub fn execute(self) -> Result<()> {
+        println!(
+            "Please paste the API Token found on {}/settings/tokens below",
+            MOVEY_URL
+        );
+        let mut line = String::new();
+        loop {
+            match io::stdin().read_line(&mut line) {
+                Ok(_) => {
+                    line = line.trim().to_string();
+                    if !line.is_empty() {
+                        break;
+                    }
+                    println!("Invalid API Token. Try again!");
                 }
-                if let Some('\r') = line.chars().next_back() {
-                    line.pop();
+                Err(err) => {
+                    bail!("Error reading file: {}", err);
                 }
-                if !line.is_empty() {
-                    break;
-                }
-                println!("Invalid API Token. Try again!");
-            }
-            Err(err) => {
-                bail!("Error reading file: {}", err);
             }
         }
+        Self::save_credential(line, MOVE_HOME.clone())?;
+        println!("Token for Movey saved.");
+        Ok(())
     }
-    let mut test_mode: Option<TestMode> = None;
-    if let Some(path) = test_path {
-        test_mode = Some(TestMode { test_path: path });
-    }
-    save_credential(line, test_mode)?;
-    println!("Token for Movey saved.");
-    Ok(())
-}
 
-pub fn save_credential(token: String, test_mode: Option<TestMode>) -> Result<()> {
-    let mut move_home;
-    if let Some(test_mode) = test_mode {
-        move_home = std::env::var("TEST_MOVE_HOME").unwrap();
-        if !test_mode.test_path.is_empty() {
-            move_home.push_str(&test_mode.test_path);
+    pub fn save_credential(token: String, move_home: String) -> Result<()> {
+        fs::create_dir_all(&move_home)?;
+        let credential_path = move_home + MOVEY_CREDENTIAL_PATH;
+        let credential_file = PathBuf::from(&credential_path);
+        if !credential_file.exists() {
+            let credential_file = File::create(&credential_path)?;
+            set_permissions(&credential_file, 0o600)?;
         }
-    } else {
-        move_home = MOVE_HOME.clone();
-    }
-    fs::create_dir_all(&move_home)?;
-    let credential_path = move_home + MOVEY_API_KEY_PATH;
-    let credential_file = PathBuf::from(&credential_path);
-    if !credential_file.exists() {
-        File::create(&credential_path)?;
-    }
 
-    let old_contents: String;
-    match fs::read_to_string(&credential_path) {
-        Ok(contents) => {
-            old_contents = contents;
+        let old_contents: String;
+        match fs::read_to_string(&credential_path) {
+            Ok(contents) => {
+                old_contents = contents;
+            }
+            Err(error) => bail!("Error reading input: {}", error),
         }
-        Err(error) => bail!("Error reading input: {}", error),
-    }
-    let mut toml: Value = old_contents
-        .parse()
-        .map_err(|e| anyhow::Error::from(e).context("could not parse input as TOML"))?;
-
-    if let Some(registry) = toml.as_table_mut().unwrap().get_mut("registry") {
-        if let Some(toml_token) = registry.as_table_mut().unwrap().get_mut("token") {
-            *toml_token = Value::String(token);
+        let mut toml: Value = old_contents
+            .parse()
+            .map_err(|e| anyhow::Error::from(e).context("could not parse input as TOML"))?;
+
+        // only update token key, keep the rest of the file intact
+        if let Some(registry) = toml.as_table_mut().unwrap().get_mut("registry") {
+            if let Some(toml_token) = registry.as_table_mut().unwrap().get_mut("token") {
+                *toml_token = Value::String(token);
+            } else {
+                registry
+                    .as_table_mut()
+                    .unwrap()
+                    .insert(String::from("token"), Value::String(token));
+            }
         } else {
-            registry
-                .as_table_mut()
+            let mut value = Map::new();
+            value.insert(String::from("token"), Value::String(token));
+            toml.as_table_mut()
                 .unwrap()
-                .insert(String::from("token"), Value::String(token));
+                .insert(String::from("registry"), Value::Table(value));
         }
-    } else {
-        let mut value = Map::new();
-        value.insert(String::from("token"), Value::String(token));
-        toml.as_table_mut()
-            .unwrap()
-            .insert(String::from("registry"), Value::Table(value));
-    }
 
-    let new_contents = toml.to_string();
-    fs::write(credential_file, new_contents).expect("Unable to write file");
-    let file = File::open(&credential_path)?;
-    set_permissions(&file, 0o600)?;
-    Ok(())
+        let new_contents = toml.to_string();
+        fs::write(credential_file, new_contents).expect("Unable to write file");
+        Ok(())
+    }
 }
 
 #[cfg(unix)]
@@ -123,13 +108,12 @@ mod tests {
     fn setup_move_home(test_path: &str) -> (String, String) {
         let cwd = env::current_dir().unwrap();
         let mut move_home: String = String::from(cwd.to_string_lossy());
-        env::set_var("TEST_MOVE_HOME", &move_home);
         if !test_path.is_empty() {
             move_home.push_str(&test_path);
         } else {
             move_home.push_str("/test");
         }
-        let credential_path = move_home.clone() + MOVEY_API_KEY_PATH;
+        let credential_path = move_home.clone() + MOVEY_CREDENTIAL_PATH;
         (move_home, credential_path)
     }
 
@@ -142,11 +126,7 @@ mod tests {
         let (move_home, credential_path) =
             setup_move_home("/save_credential_works_if_no_credential_file_exists");
         let _ = fs::remove_dir_all(&move_home);
-
-        let test_mode = Some(TestMode {
-            test_path: String::from("/save_credential_works_if_no_credential_file_exists"),
-        });
-        save_credential(String::from("test_token"), test_mode).unwrap();
+        MoveyLogin::save_credential(String::from("test_token"), move_home.clone()).unwrap();
 
         let contents = fs::read_to_string(&credential_path).expect("Unable to read file");
         let mut toml: Value = contents.parse().unwrap();
@@ -170,10 +150,7 @@ mod tests {
         let mut toml: Value = contents.parse().unwrap();
         assert!(toml.as_table_mut().unwrap().get_mut("registry").is_none());
 
-        let test_mode = Some(TestMode {
-            test_path: String::from("/save_credential_works_if_empty_credential_file_exists"),
-        });
-        save_credential(String::from("test_token"), test_mode).unwrap();
+        MoveyLogin::save_credential(String::from("test_token"), move_home.clone()).unwrap();
 
         let contents = fs::read_to_string(&credential_path).expect("Unable to read file");
         let mut toml: Value = contents.parse().unwrap();
@@ -204,10 +181,7 @@ mod tests {
         assert!(token.to_string().contains("old_test_token"));
         assert!(!token.to_string().contains("new_world"));
 
-        let test_mode = Some(TestMode {
-            test_path: String::from("/save_credential_works_if_token_field_exists"),
-        });
-        save_credential(String::from("new_world"), test_mode).unwrap();
+        MoveyLogin::save_credential(String::from("new_world"), move_home.clone()).unwrap();
 
         let contents = fs::read_to_string(&credential_path).expect("Unable to read file");
         let mut toml: Value = contents.parse().unwrap();
@@ -239,10 +213,7 @@ mod tests {
         let token = registry.as_table_mut().unwrap().get_mut("token").unwrap();
         assert!(!token.to_string().contains("test_token"));
 
-        let test_mode = Some(TestMode {
-            test_path: String::from("/save_credential_works_if_empty_token_field_exists"),
-        });
-        save_credential(String::from("test_token"), test_mode).unwrap();
+        MoveyLogin::save_credential(String::from("test_token"), move_home.clone()).unwrap();
 
         let contents = fs::read_to_string(&credential_path).expect("Unable to read file");
         let mut toml: Value = contents.parse().unwrap();

language/tools/move-cli/src/movey_login/cli.rs renamed to language/tools/move-cli/src/base/movey_login.rs

@@ -4,13 +4,12 @@
 
 use base::{
     build::Build, coverage::Coverage, disassemble::Disassemble, docgen::Docgen, errmap::Errmap,
-    info::Info, new::New, prove::Prove, test::Test,
+    info::Info, movey_login::MoveyLogin, new::New, prove::Prove, test::Test,
 };
 use move_package::BuildConfig;
 
 pub mod base;
 pub mod experimental;
-pub mod movey_login;
 pub mod sandbox;
 
 /// Default directory where saved Move resources live
@@ -93,10 +92,7 @@ pub enum Command {
         cmd: experimental::cli::ExperimentalCommand,
     },
     #[clap(name = "movey-login")]
-    MoveyLogin {
-        #[clap(long = "test-path")]
-        test_path: Option<String>,
-    },
+    MoveyLogin(MoveyLogin),
 }
 
 pub fn run_cli(
@@ -127,9 +123,7 @@ pub fn run_cli(
             &storage_dir,
         ),
         Command::Experimental { storage_dir, cmd } => cmd.handle_command(&move_args, &storage_dir),
-        Command::MoveyLogin { test_path } => {
-            movey_login::cli::handle_movey_login_commands(test_path)
-        }
+        Command::MoveyLogin(c) => c.execute(),
     }
 }
 

language/tools/move-cli/src/lib.rs

@@ -2,8 +2,8 @@
 // Copyright (c) The Move Contributors
 // SPDX-License-Identifier: Apache-2.0
 
-use move_cli::{movey_login::cli::MOVEY_API_KEY_PATH, sandbox::commands::test};
-use move_command_line_common::movey::MOVEY_URL;
+use move_cli::{base::movey_login::MOVEY_CREDENTIAL_PATH, sandbox::commands::test};
+use move_command_line_common::movey_constants::MOVEY_URL;
 #[cfg(unix)]
 use std::fs::File;
 use std::{env, fs, io::Write};
@@ -68,13 +68,9 @@ fn save_credential_works() {
     assert!(fs::read_to_string(&credential_path).is_err());
 
     match std::process::Command::new(cli_exe)
+        .env("MOVE_HOME", &move_home)
         .current_dir(".")
-        .args([
-            "movey-login",
-            "--test",
-            "--test-path",
-            "/save_credential_works",
-        ])
+        .args(["movey-login"])
         .stdin(Stdio::piped())
         .stdout(Stdio::piped())
         .spawn()
@@ -123,13 +119,9 @@ fn save_credential_fails_if_undeletable_credential_file_exists() {
     file.set_permissions(perms).unwrap();
 
     match std::process::Command::new(cli_exe)
+        .env("MOVE_HOME", &move_home)
         .current_dir(".")
-        .args([
-            "movey-login",
-            "--test",
-            "--test-path",
-            "/save_credential_fails_if_undeletable_credential_file_exists",
-        ])
+        .args(["movey-login"])
         .stdin(Stdio::piped())
         .stdout(Stdio::piped())
         .stderr(Stdio::piped())
@@ -171,11 +163,10 @@ fn save_credential_fails_if_undeletable_credential_file_exists() {
 fn setup_move_home(test_path: &str) -> (String, String) {
     let cwd = env::current_dir().unwrap();
     let mut move_home: String = String::from(cwd.to_string_lossy());
-    env::set_var("TEST_MOVE_HOME", &move_home);
     move_home.push_str(&test_path);
     let _ = fs::remove_dir_all(&move_home);
     fs::create_dir_all(&move_home).unwrap();
-    let credential_path = move_home.clone() + MOVEY_API_KEY_PATH;
+    let credential_path = move_home.clone() + MOVEY_CREDENTIAL_PATH;
     (move_home, credential_path)
 }