fix: some broken tests (#2249)

Turns out we weren't actually running any of the tests in
`glaredb/tests`. This fixes some issues that were identified when
running those tests.

Additionally updates the unit test command to properly run all unit
tests.

.github/workflows/ci.yaml

@@ -76,9 +76,6 @@ jobs:
       - name: Unit Tests
         run: just unit-tests
 
-      - name: Doc Tests
-        run: just doc-tests
-
   python-binding-tests:
     name: Python Binding Tests
     runs-on: ubuntu-latest-8-cores

Cargo.toml

@@ -28,6 +28,7 @@ prost = "0.12"
 prost-types = "0.12"
 prost-build = "0.12"
 tonic = { version = "0.10", features = ["transport", "tls", "tls-roots"] }
+tempfile = "3.8.1"
 
 [workspace.dependencies.deltalake]
 git = "https://github.com/delta-io/delta-rs.git"

crates/datasources/Cargo.toml

@@ -43,7 +43,7 @@ serde_bytes = "0.11.12"
 serde_with = "3.1.0"
 serde_json = { workspace = true }
 snowflake_connector = { path = "../snowflake_connector" }
-tempfile = "3.8.1"
+tempfile = { workspace = true }
 ssh-key = { version = "0.6.3", features = ["ed25519", "alloc"] }
 thiserror.workspace = true
 tokio-util = { version = "*" }

crates/glaredb/Cargo.toml

@@ -41,3 +41,4 @@ atty = "0.2.14"
 predicates = "3.0.4"
 assert_cmd = "2.0.12"
 tokio-postgres = "0.7.8"
+tempfile = { workspace = true }

crates/glaredb/src/bin/main.rs

@@ -51,16 +51,24 @@ fn main() -> Result<()> {
         None => Commands::Local(cli.local_args),
     };
 
-    match &command {
-        Commands::Local(args) => {
-            if let Some(file) = &args.log_file {
-                logutil::init(
-                    cli.verbose,
-                    cli.log_mode.unwrap_or_default().into(),
-                    Some(file),
-                )
-            }
-        }
+    match (&command, cli.log_mode, cli.verbose) {
+        (
+            // User specified a log file, so we should use it.
+            Commands::Local(LocalArgs {
+                log_file: Some(log_file),
+                ..
+            }),
+            _,
+            _,
+        ) => logutil::init(
+            cli.verbose,
+            // Use JSON logging by default when writing to a file.
+            cli.log_mode.unwrap_or(LoggingMode::Json).into(),
+            Some(log_file),
+        ),
+        // Disable logging when running locally since it'll clobber the repl
+        // _unless_ the user specified a logging related option.
+        (Commands::Local { .. }, None, 0) => (),
         _ => logutil::init(cli.verbose, cli.log_mode.unwrap_or_default().into(), None),
     }
 

crates/glaredb/tests/log_file_test.rs

@@ -5,25 +5,27 @@ use std::{
     io::Read,
 };
 
+use tempfile::NamedTempFile;
+
 use crate::setup::{make_cli, DEFAULT_TIMEOUT};
 
 #[test]
 fn test_log_file() -> Result<(), Box<dyn std::error::Error>> {
     let mut cmd = make_cli();
-    let log_file_name = "test.log";
+    let tmp_file = NamedTempFile::new().unwrap();
+    let file_path = tmp_file.path();
+    let file_path = file_path.canonicalize().unwrap();
+    let file_path = file_path.as_os_str().to_str().unwrap();
+
     cmd.timeout(DEFAULT_TIMEOUT)
+        .arg("--log-file")
+        .arg(file_path)
         .arg("-q")
         .arg("select 1;")
-        .arg("--log-file")
-        .arg(log_file_name)
         .assert()
         .success();
 
-    let mut log_file = std::env::current_dir().expect("failed to retrieve current dir");
-    log_file.push(log_file_name);
-    assert!(log_file.exists());
-
-    let mut file = OpenOptions::new().read(true).open(&log_file)?;
+    let mut file = OpenOptions::new().read(true).open(&tmp_file)?;
     assert!(file.metadata()?.len() > 0);
 
     let mut content = String::new();
@@ -35,28 +37,28 @@ fn test_log_file() -> Result<(), Box<dyn std::error::Error>> {
 
     assert!(content.contains("Starting in-memory metastore"));
 
-    remove_file(log_file)?;
+    remove_file(tmp_file)?;
     Ok(())
 }
 
 #[test]
 fn test_log_file_verbosity_level() -> Result<(), Box<dyn std::error::Error>> {
     let mut cmd = make_cli();
-    let log_file_name = "test.log";
+    let tmp_file = NamedTempFile::new().unwrap();
+    let file_path = tmp_file.path();
+    let file_path = file_path.canonicalize().unwrap();
+    let file_path = file_path.as_os_str().to_str().unwrap();
+
     cmd.timeout(DEFAULT_TIMEOUT)
         .arg("-v")
+        .arg("--log-file")
+        .arg(file_path)
         .arg("-q")
         .arg("select 1;")
-        .arg("--log-file")
-        .arg(log_file_name)
         .assert()
         .success();
 
-    let mut log_file = std::env::current_dir().expect("failed to retrieve current dir");
-    log_file.push(log_file_name);
-    assert!(log_file.exists());
-
-    let mut file = OpenOptions::new().read(true).open(&log_file)?;
+    let mut file = OpenOptions::new().read(true).open(&tmp_file)?;
     assert!(file.metadata()?.len() > 0);
 
     let mut content = String::new();
@@ -65,23 +67,6 @@ fn test_log_file_verbosity_level() -> Result<(), Box<dyn std::error::Error>> {
     assert!(content.contains("DEBUG"));
     assert!(!content.contains("TRACE"));
 
-    remove_file(log_file)?;
+    remove_file(tmp_file)?;
     Ok(())
 }
-
-#[test]
-fn test_skipping_logs() {
-    let mut cmd = make_cli();
-    let log_file_name = "/usr/bin/debug.log";
-    cmd.timeout(DEFAULT_TIMEOUT)
-        .arg("-q")
-        .arg("select 1;")
-        .arg("--log-file")
-        .arg(log_file_name)
-        .assert()
-        .success();
-
-    let mut log_file = std::env::current_dir().expect("failed to retrieve current dir");
-    log_file.push(log_file_name);
-    assert!(!log_file.exists());
-}

crates/glaredb/tests/setup.rs

@@ -1,5 +1,10 @@
+use std::time::Duration;
+
 use assert_cmd::cmd::Command;
 
 pub fn make_cli() -> Command {
     Command::cargo_bin(env!("CARGO_PKG_NAME")).expect("Failed to find binary")
 }
+
+#[allow(dead_code)] // Used in the tests. IDK why clippy is complaining about it.
+pub const DEFAULT_TIMEOUT: Duration = Duration::from_secs(5);

justfile

@@ -48,7 +48,7 @@ test *args: protoc
 
 # Run unit tests.
 unit-tests *args: protoc
-  just test --lib --bins {{args}}
+  just test --workspace --exclude testing {{args}}
 
 # Run doc tests.
 doc-tests: protoc