Merge pull request #1304 from weiznich/bugfix/migration_argument

Fix global cli argument handling

diesel_cli/Cargo.toml

@@ -16,7 +16,7 @@ path = "src/main.rs"
 
 [dependencies]
 chrono = "0.4"
-clap = "2.20.3"
+clap = ">=2.27.0"
 diesel = { version = "0.16.0", default-features = false }
 dotenv = ">=0.8, <0.11"
 diesel_infer_schema = "0.16.0"

diesel_cli/src/main.rs

@@ -54,32 +54,32 @@ fn main() {
 
 fn run_migration_command(matches: &ArgMatches) {
     match matches.subcommand() {
-        ("run", Some(args)) => {
+        ("run", Some(_)) => {
             let database_url = database::database_url(matches);
-            let dir = migrations_dir(args);
+            let dir = migrations_dir(matches);
             call_with_conn!(
                 database_url,
                 migrations::run_pending_migrations_in_directory(&dir, &mut stdout())
             ).unwrap_or_else(handle_error);
         }
-        ("revert", Some(args)) => {
+        ("revert", Some(_)) => {
             let database_url = database::database_url(matches);
-            let dir = migrations_dir(args);
+            let dir = migrations_dir(matches);
             call_with_conn!(
                 database_url,
                 migrations::revert_latest_migration_in_directory(&dir)
             ).unwrap_or_else(handle_error);
         }
-        ("redo", Some(args)) => {
+        ("redo", Some(_)) => {
             let database_url = database::database_url(matches);
-            let dir = migrations_dir(args);
+            let dir = migrations_dir(matches);
             call_with_conn!(database_url, redo_latest_migration(&dir));
         }
-        ("list", Some(args)) => {
+        ("list", Some(_)) => {
             use std::ffi::OsStr;
 
             let database_url = database::database_url(matches);
-            let dir = migrations_dir(args);
+            let dir = migrations_dir(matches);
             let migrations =
                 call_with_conn!(database_url, migrations::mark_migrations_in_directory(&dir))
                     .unwrap_or_else(handle_error);
@@ -118,7 +118,7 @@ fn run_migration_command(matches: &ArgMatches) {
             let migration_name = args.value_of("MIGRATION_NAME").unwrap();
             let version = migration_version(args);
             let versioned_name = format!("{}_{}", version, migration_name);
-            let migration_dir = migrations_dir(args).join(versioned_name);
+            let migration_dir = migrations_dir(matches).join(versioned_name);
             fs::create_dir(&migration_dir).unwrap();
 
             let migration_dir_relative =
@@ -153,10 +153,20 @@ fn migration_version<'a>(matches: &'a ArgMatches) -> Box<Display + 'a> {
         .unwrap_or_else(|| Box::new(Utc::now().format(TIMESTAMP_FORMAT)))
 }
 
-fn migrations_dir(matches: &ArgMatches) -> PathBuf {
+fn migrations_dir_from_cli(matches: &ArgMatches) -> Option<PathBuf> {
     matches
         .value_of("MIGRATION_DIRECTORY")
         .map(PathBuf::from)
+        .or_else(|| {
+            matches
+                .subcommand()
+                .1
+                .and_then(|s| migrations_dir_from_cli(s))
+        })
+}
+
+fn migrations_dir(matches: &ArgMatches) -> PathBuf {
+    migrations_dir_from_cli(matches)
         .or_else(|| env::var("MIGRATION_DIRECTORY").map(PathBuf::from).ok())
         .unwrap_or_else(|| {
             migrations::find_migrations_directory().unwrap_or_else(handle_error)

diesel_cli/tests/migration_run.rs

@@ -173,3 +173,143 @@ fn any_pending_migrations_after_running_and_creating() {
 
     assert!(result.stdout().contains("true\n"));
 }
+
+#[test]
+fn migration_run_runs_pending_migrations_custom_database_url_1() {
+    let p = project("migration_run_custom_db_url_1")
+        .folder("migrations")
+        .build();
+    let db_url = p.database_url();
+    let db = database(&db_url);
+
+    // Make sure the project is setup
+    p.command("setup").run();
+
+    p.create_migration(
+        "12345_create_users_table",
+        "CREATE TABLE users ( id INTEGER )",
+        "DROP TABLE users",
+    );
+
+    assert!(!db.table_exists("users"));
+
+    let result = p.command_without_database_url("migration")
+        .arg("run")
+        .arg("--database-url")
+        .arg(db_url)
+        .run();
+
+    assert!(result.is_success(), "Result was unsuccessful {:?}", result);
+    assert!(
+        result.stdout().contains("Running migration 12345"),
+        "Unexpected stdout {}",
+        result.stdout()
+    );
+    assert!(db.table_exists("users"));
+}
+
+#[test]
+fn migration_run_runs_pending_migrations_custom_database_url_2() {
+    let p = project("migration_run_custom_db_url_2")
+        .folder("migrations")
+        .build();
+    let db_url = p.database_url();
+    let db = database(&db_url);
+
+    // Make sure the project is setup
+    p.command("setup").run();
+
+    p.create_migration(
+        "12345_create_users_table",
+        "CREATE TABLE users ( id INTEGER )",
+        "DROP TABLE users",
+    );
+
+    assert!(!db.table_exists("users"));
+
+    let result = p.command_without_database_url("migration")
+        .arg("--database-url")
+        .arg(db_url)
+        .arg("run")
+        .run();
+
+    assert!(result.is_success(), "Result was unsuccessful {:?}", result);
+    assert!(
+        result.stdout().contains("Running migration 12345"),
+        "Unexpected stdout {}",
+        result.stdout()
+    );
+    assert!(db.table_exists("users"));
+}
+
+
+#[test]
+fn migration_run_runs_pending_migrations_custom_migration_dir_1() {
+    let p = project("migration_run_custom_migration_dir_1")
+        .folder("custom_migrations")
+        .build();
+    let db_url = p.database_url();
+    let db = database(&db_url);
+
+    // Make sure the project is setup
+    p.command("setup").run();
+
+    p.create_migration_in_directory(
+        "custom_migrations",
+        "12345_create_users_table",
+        "CREATE TABLE users ( id INTEGER )",
+        "DROP TABLE users",
+    );
+
+    assert!(!db.table_exists("users"));
+
+    let result = p.command("migration")
+        .arg("run")
+        .arg("--migration-dir")
+        .arg(p.migration_dir_in_directory("custom_migrations"))
+        .run();
+
+    assert!(result.is_success(), "Result was unsuccessful {:?}", result);
+    assert!(
+        result.stdout().contains("Running migration 12345"),
+        "Unexpected stdout {}",
+        result.stdout()
+    );
+    assert!(db.table_exists("users"));
+}
+
+
+#[test]
+fn migration_run_runs_pending_migrations_custom_migration_dir_2() {
+    let p = project("migration_run_custom_migration_dir_2")
+        .folder("custom_migrations")
+        .build();
+    let db_url = p.database_url();
+    let db = database(&db_url);
+
+    // Make sure the project is setup
+    p.command("setup").run();
+
+    p.create_migration_in_directory(
+        "custom_migrations",
+        "12345_create_users_table",
+        "CREATE TABLE users ( id INTEGER )",
+        "DROP TABLE users",
+    );
+
+    assert!(!db.table_exists("users"));
+
+    let result = p.command("migration")
+        .arg("--migration-dir")
+        .arg(p.migration_dir_in_directory("custom_migrations"))
+        .arg("run")
+        .run();
+
+    assert!(result.is_success(), "Result was unsuccessful {:?}", result);
+    assert!(
+        result.stdout().contains("Running migration 12345"),
+        "Unexpected stdout {}",
+        result.stdout()
+    );
+    assert!(db.table_exists("users"));
+}

diesel_cli/tests/support/project_builder.rs

@@ -127,6 +127,11 @@ impl Project {
         fs::remove_dir_all(file).unwrap();
     }
 
+    pub fn migration_dir_in_directory(&self, directory: &str) -> String {
+        let migration_path = self.directory.path().join(directory);
+        migration_path.display().to_string()
+    }
+
     pub fn create_migration(&self, name: &str, up: &str, down: &str) {
         self.create_migration_in_directory("migrations", name, up, down);
     }