tests and tooling (#4)

* init openspec

* clean out src, move mod into lib, remove trash

* create tests

* pre-commit hook

* add tests to CI

* update website

* README, CONTRIBUTING and Makefile

* openspec

* archive task

* fix ci order

* fix integration test

* fix validation tests

src/cli/connection.rs

@@ -6,10 +6,10 @@ use std::process::{Command, Stdio};
 use std::thread;
 use std::time::Duration;
 
-use anyhow::{Context, Result, bail};
+use anyhow::{bail, Context, Result};
 
 use crate::cli::GlobalOpts;
-use crate::core::doctor::{DoctorReport, run as run_doctor_report};
+use crate::core::doctor::{run as run_doctor_report, DoctorReport};
 use crate::core::paths::{pid_path_for_session, socket_dir, socket_path_for_session};
 use crate::core::protocol::{Request, Response};
 
@@ -95,7 +95,8 @@ fn send_request_over_stream(mut stream: UnixStream, request: &Request) -> Result
 }
 
 fn ping_daemon(opts: &GlobalOpts) -> Result<()> {
-    let response = send_request_over_stream(connect_socket(&socket_path(opts))?, &Request::new("ping"))?;
+    let response =
+        send_request_over_stream(connect_socket(&socket_path(opts))?, &Request::new("ping"))?;
     if response.success {
         Ok(())
     } else {
@@ -212,7 +213,9 @@ pub fn daemon_status(opts: &GlobalOpts) -> Result<()> {
     let path = socket_path(opts);
     match ping_daemon(opts) {
         Ok(()) => println!("Daemon running ({})", path.display()),
-        Err(_) if path.exists() => println!("Daemon socket exists but is unhealthy ({})", path.display()),
+        Err(_) if path.exists() => {
+            println!("Daemon socket exists but is unhealthy ({})", path.display())
+        }
         Err(_) => println!("Daemon not running"),
     }
     Ok(())
@@ -226,7 +229,11 @@ fn print_doctor_report(report: &DoctorReport, json_output: bool) -> Result<()> {
 
     println!(
         "deskctl doctor: {}",
-        if report.healthy { "healthy" } else { "issues found" }
+        if report.healthy {
+            "healthy"
+        } else {
+            "issues found"
+        }
     );
     for check in &report.checks {
         let status = if check.ok { "OK" } else { "FAIL" };

src/cli/mod.rs

@@ -1,4 +1,4 @@
-mod connection;
+pub mod connection;
 
 use anyhow::Result;
 use clap::{Args, Parser, Subcommand};

src/core/doctor.rs

@@ -84,13 +84,16 @@ pub fn run(socket_path: &Path) -> DoctorReport {
         checks.push(match backend.capture_screenshot() {
             Ok(image) => check_ok(
                 "screenshot",
-                format!("Captured {}x{} desktop image", image.width(), image.height()),
+                format!(
+                    "Captured {}x{} desktop image",
+                    image.width(),
+                    image.height()
+                ),
             ),
             Err(error) => check_fail(
                 "screenshot",
                 error.to_string(),
-                "Verify the X11 session permits desktop capture on the active display."
-                    .to_string(),
+                "Verify the X11 session permits desktop capture on the active display.".to_string(),
             ),
         });
     } else {
@@ -117,7 +120,10 @@ fn check_socket_dir(socket_path: &Path) -> DoctorCheck {
     let Some(socket_dir) = socket_path.parent() else {
         return check_fail(
             "socket-dir",
-            format!("Socket path {} has no parent directory", socket_path.display()),
+            format!(
+                "Socket path {} has no parent directory",
+                socket_path.display()
+            ),
             "Use a socket path inside a writable directory.".to_string(),
         );
     };
@@ -203,37 +209,3 @@ fn check_fail(name: &str, details: String, fix: String) -> DoctorCheck {
         fix: Some(fix),
     }
 }
-
-#[cfg(all(test, target_os = "linux"))]
-mod tests {
-    use super::run;
-    use crate::test_support::{X11TestEnv, env_lock};
-
-    #[test]
-    fn doctor_reports_healthy_x11_environment_under_xvfb() {
-        let _guard = env_lock().lock().unwrap();
-        let Some(env) = X11TestEnv::new().unwrap() else {
-            eprintln!("Skipping Xvfb-dependent doctor test");
-            return;
-        };
-        env.create_window("deskctl doctor test", "DeskctlDoctor").unwrap();
-
-        let socket_path = std::env::temp_dir().join("deskctl-doctor-test.sock");
-        let report = run(&socket_path);
-
-        assert!(report.checks.iter().any(|check| check.name == "display" && check.ok));
-        assert!(report.checks.iter().any(|check| check.name == "backend" && check.ok));
-        assert!(
-            report
-                .checks
-                .iter()
-                .any(|check| check.name == "window-enumeration" && check.ok)
-        );
-        assert!(
-            report
-                .checks
-                .iter()
-                .any(|check| check.name == "screenshot" && check.ok)
-        );
-    }
-}

src/core/refs.rs

@@ -136,8 +136,12 @@ impl RefMap {
 
     /// Resolve a selector to the center coordinates of the window.
     pub fn resolve_to_center(&self, selector: &str) -> Option<(i32, i32)> {
-        self.resolve(selector)
-            .map(|entry| (entry.x + entry.width as i32 / 2, entry.y + entry.height as i32 / 2))
+        self.resolve(selector).map(|entry| {
+            (
+                entry.x + entry.width as i32 / 2,
+                entry.y + entry.height as i32 / 2,
+            )
+        })
     }
 
     pub fn entries(&self) -> impl Iterator<Item = (&String, &RefEntry)> {
@@ -182,7 +186,10 @@ mod tests {
 
         assert_eq!(refs.resolve("@w1").unwrap().window_id, window_id);
         assert_eq!(refs.resolve(&window_id).unwrap().backend_window_id, 42);
-        assert_eq!(refs.resolve(&format!("id={window_id}")).unwrap().title, "Editor");
+        assert_eq!(
+            refs.resolve(&format!("id={window_id}")).unwrap().title,
+            "Editor"
+        );
     }
 
     #[test]

src/daemon/handler.rs

@@ -394,14 +394,13 @@ fn capture_snapshot(
 ) -> Result<Snapshot> {
     let windows = refresh_windows(state)?;
     let screenshot_path = path.unwrap_or_else(temp_screenshot_path);
-    let screenshot = capture_and_save_screenshot(
-        state,
-        &screenshot_path,
-        annotate,
-        Some(&windows),
-    )?;
+    let screenshot =
+        capture_and_save_screenshot(state, &screenshot_path, annotate, Some(&windows))?;
 
-    Ok(Snapshot { screenshot, windows })
+    Ok(Snapshot {
+        screenshot,
+        windows,
+    })
 }
 
 fn capture_and_save_screenshot(
@@ -439,55 +438,3 @@ fn parse_coords(value: &str) -> Option<(i32, i32)> {
     let y = parts[1].trim().parse().ok()?;
     Some((x, y))
 }
-
-#[cfg(all(test, target_os = "linux"))]
-mod tests {
-    use std::sync::Arc;
-
-    use tokio::runtime::Builder;
-    use tokio::sync::Mutex;
-
-    use super::handle_request;
-    use crate::core::protocol::Request;
-    use crate::daemon::state::DaemonState;
-    use crate::test_support::{X11TestEnv, deskctl_tmp_screenshot_count, env_lock};
-
-    #[test]
-    fn list_windows_is_side_effect_free_under_xvfb() {
-        let _guard = env_lock().lock().unwrap();
-        let Some(env) = X11TestEnv::new().unwrap() else {
-            eprintln!("Skipping Xvfb-dependent list-windows test");
-            return;
-        };
-        env.create_window("deskctl list-windows test", "DeskctlList").unwrap();
-
-        let before = deskctl_tmp_screenshot_count();
-        let runtime = Builder::new_current_thread().enable_all().build().unwrap();
-        let state = Arc::new(Mutex::new(
-            DaemonState::new(
-                "test".to_string(),
-                std::env::temp_dir().join("deskctl-list-windows.sock"),
-            )
-            .unwrap(),
-        ));
-
-        let response = runtime.block_on(handle_request(&Request::new("list-windows"), &state));
-        assert!(response.success);
-
-        let data = response.data.unwrap();
-        let windows = data
-            .get("windows")
-            .and_then(|value| value.as_array())
-            .unwrap();
-        assert!(windows.iter().any(|window| {
-            window
-                .get("title")
-                .and_then(|value| value.as_str())
-                .map(|title| title == "deskctl list-windows test")
-                .unwrap_or(false)
-        }));
-
-        let after = deskctl_tmp_screenshot_count();
-        assert_eq!(before, after, "list-windows should not create screenshot artifacts");
-    }
-}

src/lib.rs

@@ -0,0 +1,11 @@
+pub mod backend;
+pub mod cli;
+pub mod core;
+pub mod daemon;
+
+pub fn run() -> anyhow::Result<()> {
+    if std::env::var("DESKCTL_DAEMON").is_ok() {
+        return daemon::run();
+    }
+    cli::run()
+}

src/main.rs

@@ -1,13 +1,3 @@
-mod backend;
-mod cli;
-mod core;
-mod daemon;
-#[cfg(test)]
-mod test_support;
-
 fn main() -> anyhow::Result<()> {
-    if std::env::var("DESKCTL_DAEMON").is_ok() {
-        return daemon::run();
-    }
-    cli::run()
+    deskctl::run()
 }

src/test_support.rs

@@ -1,150 +0,0 @@
-#![cfg(all(test, target_os = "linux"))]
-
-use std::path::Path;
-use std::process::{Child, Command, Stdio};
-use std::sync::{Mutex, OnceLock};
-use std::thread;
-use std::time::Duration;
-
-use anyhow::{Context, Result};
-use x11rb::connection::Connection;
-use x11rb::protocol::xproto::{
-    AtomEnum, ConnectionExt as XprotoConnectionExt, CreateWindowAux, EventMask, PropMode,
-    WindowClass,
-};
-
-pub fn env_lock() -> &'static Mutex<()> {
-    static LOCK: OnceLock<Mutex<()>> = OnceLock::new();
-    LOCK.get_or_init(|| Mutex::new(()))
-}
-
-pub struct X11TestEnv {
-    child: Child,
-    old_display: Option<String>,
-    old_session_type: Option<String>,
-}
-
-impl X11TestEnv {
-    pub fn new() -> Result<Option<Self>> {
-        if Command::new("Xvfb")
-            .arg("-help")
-            .stdout(Stdio::null())
-            .stderr(Stdio::null())
-            .status()
-            .is_err()
-        {
-            return Ok(None);
-        }
-
-        for display_num in 90..110 {
-            let display = format!(":{display_num}");
-            let lock_path = format!("/tmp/.X{display_num}-lock");
-            let unix_socket = format!("/tmp/.X11-unix/X{display_num}");
-            if Path::new(&lock_path).exists() || Path::new(&unix_socket).exists() {
-                continue;
-            }
-
-            let child = Command::new("Xvfb")
-                .arg(&display)
-                .arg("-screen")
-                .arg("0")
-                .arg("1024x768x24")
-                .arg("-nolisten")
-                .arg("tcp")
-                .stdout(Stdio::null())
-                .stderr(Stdio::null())
-                .spawn()
-                .with_context(|| format!("Failed to launch Xvfb on {display}"))?;
-
-            thread::sleep(Duration::from_millis(250));
-
-            let old_display = std::env::var("DISPLAY").ok();
-            let old_session_type = std::env::var("XDG_SESSION_TYPE").ok();
-            std::env::set_var("DISPLAY", &display);
-            std::env::set_var("XDG_SESSION_TYPE", "x11");
-
-            return Ok(Some(Self {
-                child,
-                old_display,
-                old_session_type,
-            }));
-        }
-
-        anyhow::bail!("Failed to find a free Xvfb display")
-    }
-
-    pub fn create_window(&self, title: &str, app_class: &str) -> Result<()> {
-        let (conn, screen_num) =
-            x11rb::connect(None).context("Failed to connect to test Xvfb display")?;
-        let screen = &conn.setup().roots[screen_num];
-        let window = conn.generate_id()?;
-
-        conn.create_window(
-            x11rb::COPY_DEPTH_FROM_PARENT,
-            window,
-            screen.root,
-            10,
-            10,
-            320,
-            180,
-            0,
-            WindowClass::INPUT_OUTPUT,
-            0,
-            &CreateWindowAux::new()
-                .background_pixel(screen.white_pixel)
-                .event_mask(EventMask::EXPOSURE),
-        )?;
-        conn.change_property8(
-            PropMode::REPLACE,
-            window,
-            AtomEnum::WM_NAME,
-            AtomEnum::STRING,
-            title.as_bytes(),
-        )?;
-        let class_bytes = format!("{app_class}\0{app_class}\0");
-        conn.change_property8(
-            PropMode::REPLACE,
-            window,
-            AtomEnum::WM_CLASS,
-            AtomEnum::STRING,
-            class_bytes.as_bytes(),
-        )?;
-        conn.map_window(window)?;
-        conn.flush()?;
-
-        thread::sleep(Duration::from_millis(150));
-        Ok(())
-    }
-}
-
-impl Drop for X11TestEnv {
-    fn drop(&mut self) {
-        let _ = self.child.kill();
-        let _ = self.child.wait();
-
-        match &self.old_display {
-            Some(value) => std::env::set_var("DISPLAY", value),
-            None => std::env::remove_var("DISPLAY"),
-        }
-
-        match &self.old_session_type {
-            Some(value) => std::env::set_var("XDG_SESSION_TYPE", value),
-            None => std::env::remove_var("XDG_SESSION_TYPE"),
-        }
-    }
-}
-
-pub fn deskctl_tmp_screenshot_count() -> usize {
-    std::fs::read_dir("/tmp")
-        .ok()
-        .into_iter()
-        .flat_map(|iter| iter.filter_map(Result::ok))
-        .filter(|entry| {
-            entry
-                .file_name()
-                .to_str()
-                .map(|name| name.starts_with("deskctl-") && name.ends_with(".png"))
-                .unwrap_or(false)
-        })
-        .count()
-}