Skip to content

Commit

Permalink
Improve error message for migration extract and add tests (#1338)
Browse files Browse the repository at this point in the history
  • Loading branch information
aljazerzen authored Jul 4, 2024
1 parent a5338e3 commit 71895ea
Show file tree
Hide file tree
Showing 4 changed files with 118 additions and 3 deletions.
10 changes: 9 additions & 1 deletion src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,15 @@ fn main() {
if let Some(e) = err.downcast_ref::<edgedb_errors::Error>() {
print::edgedb_error(e, false);
} else {
print::error(err);
let mut error_chain = err.chain();
if let Some(first) = error_chain.next() {
print::error(first);
} else {
print::error(" <empty error message>");
}
for e in error_chain {
eprintln!(" Caused by: {e}");
}
}
for item in err.chain() {
if let Some(e) = item.downcast_ref::<hint::HintedError>() {
Expand Down
33 changes: 32 additions & 1 deletion src/migrations/extract.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use anyhow::Context as _;
use fs_err as fs;
use std::iter::Once;
use std::path::Path;

use crate::commands::{ExitCode, Options};
use crate::connect::Connection;
Expand Down Expand Up @@ -126,14 +128,33 @@ pub async fn extract(
}
}

// make sure that migrations dir exists
let to_migrations_dir = src_ctx.schema_dir.join("migrations");
if !to_migrations_dir.is_dir() {
if src_ctx.schema_dir.is_dir() {
print::warn(format!(
"Creating directory {}",
to_migrations_dir.display()
));
fs::create_dir(to_migrations_dir)?;
} else {
anyhow::bail!(
"Cannot write migrations because path {} is not a directory",
src_ctx.schema_dir.display()
);
}
}

// copy migration files
let mut updated = false;
for from in migration::read_names(&temp_ctx).await? {
let to = src_ctx
.schema_dir
.join("migrations")
.join(from.file_name().expect(""));
print::success_msg("Writing", to.display());
fs::copy(from, to)?;
fs::copy(from, &to)
.with_context(|| format!("Cannot write {}", relative_to_current_dir(&to).display()))?;
updated = true;
}
for path in to_delete {
Expand All @@ -149,3 +170,13 @@ pub async fn extract(
}
Ok(())
}

fn relative_to_current_dir(path: &Path) -> &Path {
let curr_dir = std::env::current_dir().ok();

if let Some(stripped) = curr_dir.and_then(|wd| path.strip_prefix(&wd).ok()) {
stripped
} else {
path
}
}
73 changes: 72 additions & 1 deletion tests/func/migrations.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::SERVER;
use crate::{rm_migration_files, SERVER};
use predicates::boolean::PredicateBooleanExt;
use predicates::str::{contains, ends_with};
use std::fs;
Expand Down Expand Up @@ -720,6 +720,77 @@ fn modified1() {
);
}

#[test]
fn extract01() {
SERVER
.admin_cmd()
.arg("branch")
.arg("create")
.arg("extract01")
.arg("--empty")
.assert()
.success();
SERVER
.admin_cmd()
.arg("--branch=extract01")
.arg("query")
.arg(
r#"
start migration to { module default { type X; } };
populate migration;
commit migration;
"#,
)
.assert()
.success();

// no migrations dir, needs to create
fs::remove_dir_all("tests/migrations/db1/extract01/migrations").ok();
SERVER
.admin_cmd()
.arg("--branch=extract01")
.arg("migration")
.arg("extract")
.arg("--schema-dir=tests/migrations/db1/extract01")
.assert()
.success()
.stderr(contains("Creating directory").and(contains("Writing")));

// base case
rm_migration_files("tests/migrations/db1/extract01", &[1]);
SERVER
.admin_cmd()
.arg("--branch=extract01")
.arg("migration")
.arg("extract")
.arg("--schema-dir=tests/migrations/db1/extract01")
.assert()
.success()
.stderr(contains("Writing"));

// test error printing
rm_migration_files("tests/migrations/db1/extract01", &[1]);
let mut perm = fs::metadata("tests/migrations/db1/extract01/migrations")
.unwrap()
.permissions();
perm.set_readonly(true);
fs::set_permissions("tests/migrations/db1/extract01/migrations", perm).unwrap();
SERVER
.admin_cmd()
.arg("--branch=extract01")
.arg("migration")
.arg("extract")
.arg("--schema-dir=tests/migrations/db1/extract01")
.assert()
.failure()
.stderr(
contains("Writing")
.and(contains("edgedb error: Cannot write"))
.and(contains("\n Caused by: failed to copy file from"))
.and(contains("\n Caused by: Permission denied")),
);
}

#[test]
fn error() {
SERVER
Expand Down
5 changes: 5 additions & 0 deletions tests/migrations/db1/extract01/default.esdl
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module default {
type Type1 {
property field1 -> str;
};
};

0 comments on commit 71895ea

Please sign in to comment.