From 3d5edb42d5860f204059d060b9575f98318a853f Mon Sep 17 00:00:00 2001 From: Georg Semmler Date: Tue, 14 Nov 2017 23:10:39 +0100 Subject: [PATCH 1/4] Bump clap for fix for global cli argument handling --- diesel_cli/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/diesel_cli/Cargo.toml b/diesel_cli/Cargo.toml index 214ca2804a1c..0722bde284dd 100644 --- a/diesel_cli/Cargo.toml +++ b/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" From 0134f1527d8a62de4f89b0e604092dd262d135d2 Mon Sep 17 00:00:00 2001 From: Georg Semmler Date: Tue, 14 Nov 2017 23:11:06 +0100 Subject: [PATCH 2/4] Fix #1301 Here we could not use the global argument thing because --migration_dir is no global argument, instead we have to look in each subcommand for the corresponding argument --- diesel_cli/src/main.rs | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/diesel_cli/src/main.rs b/diesel_cli/src/main.rs index 73e3a11445b7..9cb91be68289 100644 --- a/diesel_cli/src/main.rs +++ b/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,21 @@ fn migration_version<'a>(matches: &'a ArgMatches) -> Box { .unwrap_or_else(|| Box::new(Utc::now().format(TIMESTAMP_FORMAT))) } -fn migrations_dir(matches: &ArgMatches) -> PathBuf { +fn migrations_dir_from_cli(matches: &ArgMatches) -> Option { 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) From 91794b733323d7276393514c1fdb24350c226fbf Mon Sep 17 00:00:00 2001 From: Georg Semmler Date: Tue, 14 Nov 2017 23:12:37 +0100 Subject: [PATCH 3/4] Add some tests * 2 Testcases for --migration_dir * Readd the tests from #1201 for --database_url --- diesel_cli/tests/migration_run.rs | 138 ++++++++++++++++++++ diesel_cli/tests/support/project_builder.rs | 5 + 2 files changed, 143 insertions(+) diff --git a/diesel_cli/tests/migration_run.rs b/diesel_cli/tests/migration_run.rs index 53ca343bfea2..08cacf01931b 100644 --- a/diesel_cli/tests/migration_run.rs +++ b/diesel_cli/tests/migration_run.rs @@ -173,3 +173,141 @@ 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")); +} diff --git a/diesel_cli/tests/support/project_builder.rs b/diesel_cli/tests/support/project_builder.rs index a9bfe9038141..1dd974c99ade 100644 --- a/diesel_cli/tests/support/project_builder.rs +++ b/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); } From 90e2bbc283a042df3d62c5bc8f5078354393f4cd Mon Sep 17 00:00:00 2001 From: Georg Semmler Date: Wed, 15 Nov 2017 10:56:32 +0100 Subject: [PATCH 4/4] Run rustfmt --- diesel_cli/src/main.rs | 1 - diesel_cli/tests/migration_run.rs | 6 ++++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/diesel_cli/src/main.rs b/diesel_cli/src/main.rs index 9cb91be68289..83c3f865a3ac 100644 --- a/diesel_cli/src/main.rs +++ b/diesel_cli/src/main.rs @@ -163,7 +163,6 @@ fn migrations_dir_from_cli(matches: &ArgMatches) -> Option { .1 .and_then(|s| migrations_dir_from_cli(s)) }) - } fn migrations_dir(matches: &ArgMatches) -> PathBuf { diff --git a/diesel_cli/tests/migration_run.rs b/diesel_cli/tests/migration_run.rs index 08cacf01931b..f1151dffd815 100644 --- a/diesel_cli/tests/migration_run.rs +++ b/diesel_cli/tests/migration_run.rs @@ -254,7 +254,8 @@ fn migration_run_runs_pending_migrations_custom_migration_dir_1() { // Make sure the project is setup p.command("setup").run(); - p.create_migration_in_directory("custom_migrations", + p.create_migration_in_directory( + "custom_migrations", "12345_create_users_table", "CREATE TABLE users ( id INTEGER )", "DROP TABLE users", @@ -289,7 +290,8 @@ fn migration_run_runs_pending_migrations_custom_migration_dir_2() { // Make sure the project is setup p.command("setup").run(); - p.create_migration_in_directory("custom_migrations", + p.create_migration_in_directory( + "custom_migrations", "12345_create_users_table", "CREATE TABLE users ( id INTEGER )", "DROP TABLE users",