Address review comments, fix test run

Created using spr 1.3.4

.github/buildomat/build-and-test.sh

@@ -66,6 +66,11 @@ ptime -m timeout 2h cargo nextest run --profile ci --locked --verbose
 banner doctest
 ptime -m timeout 1h cargo test --doc --locked --verbose --no-fail-fast
 
+# We expect the seed CRDB to be placed here, so we explicitly remove it so the
+# rmdir check below doesn't get triggered. Nextest doesn't have support for
+# teardown scripts so this is the best we've got.
+rm -rf "$TEST_TMPDIR/crdb-base"
+
 #
 # Make sure that we have left nothing around in $TEST_TMPDIR.  The easiest way
 # to check is to try to remove it with `rmdir`.

crdb-seed/src/main.rs

@@ -18,39 +18,47 @@ fn digest_unique_to_schema() -> String {
     hex::encode(digest.as_ref())
 }
 
-async fn ensure_seed_directory_exists(log: &Logger) -> Utf8PathBuf {
+enum SeedDirectoryStatus {
+    Created,
+    Existing,
+}
+
+async fn ensure_seed_directory_exists(
+    log: &Logger,
+) -> (Utf8PathBuf, SeedDirectoryStatus) {
     let base_seed_dir = Utf8PathBuf::from_path_buf(std::env::temp_dir())
         .expect("Not a UTF-8 path")
         .join("crdb-base");
     std::fs::create_dir_all(&base_seed_dir).unwrap();
     let desired_seed_dir = base_seed_dir.join(digest_unique_to_schema());
 
-    // If the directory didn't exist when we started, try to create it.
+    if desired_seed_dir.exists() {
+        return (desired_seed_dir, SeedDirectoryStatus::Existing);
+    }
+
+    // The directory didn't exist when we started, so try to create it.
     //
-    // Note that this may be executed concurrently by many tests, so
-    // we should consider it possible for another caller to create this
-    // seed directory before we finish setting it up ourselves.
-    if !desired_seed_dir.exists() {
-        let tmp_seed_dir =
-            camino_tempfile::Utf8TempDir::new_in(base_seed_dir).unwrap();
-        dev::test_setup_database_seed(log, tmp_seed_dir.path()).await;
+    // Nextest will execute it just once, but it is possible for a user to start
+    // up multiple nextest processes to be running at the same time. So we
+    // should consider it possible for another caller to create this seed
+    // directory before we finish setting it up ourselves.
+    let tmp_seed_dir =
+        camino_tempfile::Utf8TempDir::new_in(base_seed_dir).unwrap();
+    dev::test_setup_database_seed(log, tmp_seed_dir.path()).await;
 
-        // If we can successfully perform the rename, we made the seed directory
-        // faster than other tests.
-        //
-        // If we couldn't perform the rename, the directory might already exist.
-        // Check that this is the error we encountered -- otherwise, we're
-        // struggling.
-        if let Err(err) =
-            std::fs::rename(tmp_seed_dir.path(), &desired_seed_dir)
-        {
-            if !desired_seed_dir.exists() {
-                panic!("Cannot rename seed directory for CockroachDB: {err}");
-            }
+    // If we can successfully perform the rename, there was either no
+    // contention or we won a creation race.
+    //
+    // If we couldn't perform the rename, the directory might already exist.
+    // Check that this is the error we encountered -- otherwise, we're
+    // struggling.
+    if let Err(err) = std::fs::rename(tmp_seed_dir.path(), &desired_seed_dir) {
+        if !desired_seed_dir.exists() {
+            panic!("Cannot rename seed directory for CockroachDB: {err}");
         }
     }
 
-    desired_seed_dir
+    (desired_seed_dir, SeedDirectoryStatus::Created)
 }
 
 #[tokio::main]
@@ -61,11 +69,24 @@ async fn main() {
         "crdb_seeding",
         &ConfigLogging::StderrTerminal { level: ConfigLoggingLevel::Info },
     );
-    let dir = ensure_seed_directory_exists(&logctx.log).await;
+    let (dir, status) = ensure_seed_directory_exists(&logctx.log).await;
+    match status {
+        SeedDirectoryStatus::Created => {
+            slog::info!(logctx.log, "Created seed directory: `{dir}`");
+        }
+        SeedDirectoryStatus::Existing => {
+            slog::info!(logctx.log, "Using existing seed directory: `{dir}`");
+        }
+    }
     if let Ok(env_path) = std::env::var("NEXTEST_ENV") {
         let mut file = std::fs::File::create(&env_path)
             .expect("failed to open NEXTEST_ENV file");
-        write!(file, "CRDB_SEED_DIR={}", dir.as_str())
+        writeln!(file, "CRDB_SEED_DIR={dir}")
             .expect("failed to write to NEXTEST_ENV file");
+    } else {
+        slog::warn!(
+            logctx.log,
+            "NEXTEST_ENV not set (is this script running under nextest?)"
+        );
     }
 }

nexus/test-utils/src/db.rs

@@ -20,8 +20,6 @@ fn seed_dir() -> Utf8PathBuf {
     // The setup script should set this environment variable.
     let seed_dir = std::env::var("CRDB_SEED_DIR")
         .expect("CRDB_SEED_DIR missing -- are you running this test with `cargo nextest run`?");
-    // TODO: replace with temp dir written out by nextest
-    // Use "out/" for now.
     seed_dir.into()
 }