From 871377533e805fd67bd95378d029ef89e3492789 Mon Sep 17 00:00:00 2001 From: Oliver Eyton-Williams Date: Tue, 10 Nov 2026 13:12:45 +0100 Subject: [PATCH 1/4] refactor: reword curriculum's server.yaml This clarifies a few points I found confusing --- databases/curriculum/config/server.yaml | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/databases/curriculum/config/server.yaml b/databases/curriculum/config/server.yaml index e79a982..c3977f6 100644 --- a/databases/curriculum/config/server.yaml +++ b/databases/curriculum/config/server.yaml @@ -1,5 +1,5 @@ # Dolt sql-server config. Mounted read-only; changes apply on restart. -# Info level: debug logs routine SQL and results, which include private answer keys. +# Info level: debug logs routine queries and results, which include private answer keys. log_level: info log_format: json behavior: @@ -14,14 +14,15 @@ listener: max_connections: 50 read_timeout_millis: 300000 write_timeout_millis: 300000 -# Explicit paths, all on the one volume, so a stopped-volume copy is a complete backup. +# Explicit paths, all on the one volume, so a copy of the volume is a complete backup. data_dir: /var/lib/dolt/databases cfg_dir: /var/lib/dolt/config privilege_file: /var/lib/dolt/config/privileges.db branch_control_file: /var/lib/dolt/config/branch_control.db system_variables: - # Points at a directory that does not exist: no LOAD DATA / SELECT INTO OUTFILE. - secure_file_priv: /var/lib/dolt/disabled-file-import + # Points at a directory that does not exist, stopping LOAD DATA / SELECT INTO OUTFILE queries. + # We need to pass a directory because dolt does not pass NULL to MySQL. + secure_file_priv: /directory/that/does/not/exist local_infile: false dolt_allow_commit_conflicts: 0 dolt_force_transaction_commit: 0 From 8378e337e0678abd70578c93401503f2eb012be5 Mon Sep 17 00:00:00 2001 From: Oliver Eyton-Williams Date: Tue, 10 Nov 2026 13:16:00 +0100 Subject: [PATCH 2/4] refactor(cdb): remove redundant test sqlx already implicitly requires unique versions since migration will fail if there are duplicates. Since other tests include migrations we still have coverage. --- databases/curriculum/src/lib.rs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/databases/curriculum/src/lib.rs b/databases/curriculum/src/lib.rs index c99a307..8973c3f 100644 --- a/databases/curriculum/src/lib.rs +++ b/databases/curriculum/src/lib.rs @@ -414,14 +414,6 @@ mod tests { Ok(count > 0) } - #[test] - fn migration_versions_are_unique() { - let mut versions = up_versions(&migrator()); - let count = versions.len(); - versions.dedup(); - assert_eq!(versions.len(), count); - } - #[tokio::test] async fn each_migration_is_one_dolt_commit_on_dev() -> Result<(), Error> { let Some(db) = TestDb::new().await? else { From f64d8a0c2b7b8a1d181d4ad61f605aa790c6772b Mon Sep 17 00:00:00 2001 From: Oliver Eyton-Williams Date: Tue, 10 Nov 2026 13:31:59 +0100 Subject: [PATCH 3/4] refactor(cdb): provide default CDB_MIGRATOR_URL (panic if missing) Silently ignoring tests can be confusing: ideally tests should either pass or fail. Now devs will get a failure and a hint about how to fix it. --- databases/curriculum/.env.example | 1 + databases/curriculum/src/lib.rs | 39 ++++++++----------------------- 2 files changed, 11 insertions(+), 29 deletions(-) diff --git a/databases/curriculum/.env.example b/databases/curriculum/.env.example index 394863e..f205153 100644 --- a/databases/curriculum/.env.example +++ b/databases/curriculum/.env.example @@ -3,3 +3,4 @@ CDB_PORT=13306 # Root SQL user: `openssl rand -hex 32`. Applies on an empty volume only. CDB_ROOT_PASSWORD= PLATFORM_NETWORK=platform +CDB_MIGRATOR_URL=mysql://root:$CDB_ROOT_PASSWORD@127.0.0.1:13306/curriculum diff --git a/databases/curriculum/src/lib.rs b/databases/curriculum/src/lib.rs index 8973c3f..486fbf4 100644 --- a/databases/curriculum/src/lib.rs +++ b/databases/curriculum/src/lib.rs @@ -271,8 +271,7 @@ pub(crate) mod testing { .ok() .filter(|v| !v.is_empty()) else { - eprintln!("CDB_MIGRATOR_URL not set: skipping Dolt test"); - return Ok(None); + panic!("CDB_MIGRATOR_URL must be set for testing. Hint: copy the value from .env.example into .env"); }; let options = MySqlConnectOptions::from_str(&url)?; let name = format!("cdb_test_{}", uuid::Uuid::new_v4().simple()); @@ -416,9 +415,7 @@ mod tests { #[tokio::test] async fn each_migration_is_one_dolt_commit_on_dev() -> Result<(), Error> { - let Some(db) = TestDb::new().await? else { - return Ok(()); - }; + let db = TestDb::new().await?.unwrap(); let expected = up_versions(&migrator()); assert_eq!(db.setup().await?, expected); @@ -455,9 +452,7 @@ mod tests { #[tokio::test] async fn rerun_is_a_no_op() -> Result<(), Error> { - let Some(db) = TestDb::new().await? else { - return Ok(()); - }; + let db = TestDb::new().await?.unwrap(); db.setup().await?; let before = db.hash_of(DEV).await?; @@ -471,9 +466,7 @@ mod tests { /// Two runs at once: the named lock serializes them. #[tokio::test] async fn concurrent_runs_apply_each_migration_once() -> Result<(), Error> { - let Some(db) = TestDb::new().await? else { - return Ok(()); - }; + let db = TestDb::new().await?.unwrap(); let migrator = migrator(); let (a, b) = tokio::join!(db.migrate_with(&migrator), db.migrate_with(&migrator)); @@ -488,9 +481,7 @@ mod tests { #[tokio::test] async fn older_bundle_runs_against_newer_migrations() -> Result<(), Error> { - let Some(db) = TestDb::new().await? else { - return Ok(()); - }; + let db = TestDb::new().await?.unwrap(); db.setup().await?; apply_newer_migration(&db).await?; @@ -501,9 +492,7 @@ mod tests { #[tokio::test] async fn strict_migrator_rejects_newer_migrations() -> Result<(), Error> { - let Some(db) = TestDb::new().await? else { - return Ok(()); - }; + let db = TestDb::new().await?.unwrap(); db.setup().await?; apply_newer_migration(&db).await?; let mut migrator = migrator(); @@ -523,9 +512,7 @@ mod tests { #[tokio::test] async fn rejects_changed_migration() -> Result<(), Error> { - let Some(db) = TestDb::new().await? else { - return Ok(()); - }; + let db = TestDb::new().await?.unwrap(); db.setup().await?; db.commit_on_dev("UPDATE _sqlx_migrations SET checksum = X'00' WHERE version = 1") .await?; @@ -545,9 +532,7 @@ mod tests { /// The failed migration stays on the scratch branch, half applied; the next run starts over. #[tokio::test] async fn failed_migration_leaves_dev_untouched() -> Result<(), Error> { - let Some(db) = TestDb::new().await? else { - return Ok(()); - }; + let db = TestDb::new().await?.unwrap(); db.setup().await?; let before = db.hash_of(DEV).await?; @@ -570,9 +555,7 @@ mod tests { /// eDd commits content to `dev` while migrations apply: the run ends in a merge commit with both. #[tokio::test] async fn merges_when_dev_moved() -> Result<(), Error> { - let Some(db) = TestDb::new().await? else { - return Ok(()); - }; + let db = TestDb::new().await?.unwrap(); let migrator = migrator(); let mut conn = db.connect().await?; @@ -608,9 +591,7 @@ mod tests { #[tokio::test] async fn never_touches_other_branches() -> Result<(), Error> { - let Some(db) = TestDb::new().await? else { - return Ok(()); - }; + let db = TestDb::new().await?.unwrap(); let mut conn = db.connect().await?; sqlx::query("CALL DOLT_BRANCH('prd', ?)") .bind(DEV) From 709c1f7058c6df27ef0c31ba64130871cf1887e0 Mon Sep 17 00:00:00 2001 From: Oliver Eyton-Williams Date: Tue, 10 Nov 2026 13:43:03 +0100 Subject: [PATCH 4/4] refactor: format --- databases/curriculum/src/lib.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/databases/curriculum/src/lib.rs b/databases/curriculum/src/lib.rs index 486fbf4..a6ce8e7 100644 --- a/databases/curriculum/src/lib.rs +++ b/databases/curriculum/src/lib.rs @@ -271,7 +271,9 @@ pub(crate) mod testing { .ok() .filter(|v| !v.is_empty()) else { - panic!("CDB_MIGRATOR_URL must be set for testing. Hint: copy the value from .env.example into .env"); + panic!( + "CDB_MIGRATOR_URL must be set for testing. Hint: copy the value from .env.example into .env" + ); }; let options = MySqlConnectOptions::from_str(&url)?; let name = format!("cdb_test_{}", uuid::Uuid::new_v4().simple());