Madhava/refactor2

.github/workflows/ci.yml

@@ -30,3 +30,24 @@ jobs:
 
       - name: Lint (cargo fmt + clippy)
         run: ./lint.sh
+
+  test:
+    name: test
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v4
+        with:
+          submodules: recursive
+
+      - name: Set up Rust
+        uses: dtolnay/rust-toolchain@stable
+
+      - name: Cache Rust build
+        uses: Swatinem/rust-cache@v2
+        with:
+          workspaces: rust
+          key: test
+
+      - name: Test
+        run: ./test.sh

.github/workflows/coverage.yml

@@ -1,9 +1,7 @@
 name: Coverage
 
 on:
-  push:
-    branches: [main]
-  pull_request:
+  workflow_dispatch:
 
 permissions:
   contents: read

AGENTS.md

@@ -16,6 +16,6 @@ production limit measures production code, not test scaffolding. Test files
 should still be split when they mix unrelated behavior or become hard to scan.
 
 When a production file grows past 500 lines, split it before adding more
-behavior. Temporary exceptions must be listed in
-`rust/bioscript-core/tests/source_size.rs` with their current line count, and
-that count should not increase.
+behavior. Temporary exceptions must be listed in this file under
+`Current Refactor Backlog`; the source-size guard reads that list and fails when
+it drifts from the code.

lint.sh

@@ -25,3 +25,5 @@ filter_vendored() {
 }
 
 cargo clippy "${PKG_ARGS[@]}" --all-targets --color=never -- -D warnings 2> >(filter_vendored >&2)
+
+cargo test -p bioscript-core --test source_size -- --nocapture

rust/bioscript-cli/src/commands.rs

@@ -180,3 +180,167 @@ where
 
     Ok(())
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use bioscript_schema::ValidationReport;
+    use std::time::{SystemTime, UNIX_EPOCH};
+
+    fn temp_dir(label: &str) -> PathBuf {
+        let nanos = SystemTime::now()
+            .duration_since(UNIX_EPOCH)
+            .expect("clock drift")
+            .as_nanos();
+        let dir = std::env::temp_dir().join(format!(
+            "bioscript-cli-commands-{label}-{}-{nanos}",
+            std::process::id()
+        ));
+        fs::create_dir_all(&dir).unwrap();
+        dir
+    }
+
+    fn empty_report() -> ValidationReport {
+        ValidationReport {
+            files_scanned: 1,
+            reports: Vec::new(),
+        }
+    }
+
+    #[test]
+    fn command_parsers_report_prepare_and_inspect_argument_errors() {
+        for (args, expected) in [
+            (vec!["--root"], "--root requires a directory"),
+            (vec!["--input-file"], "--input-file requires a path"),
+            (vec!["--reference-file"], "--reference-file requires a path"),
+            (vec!["--input-format"], "--input-format requires a value"),
+            (vec!["--input-format", "bad"], "invalid --input-format"),
+            (vec!["--cache-dir"], "--cache-dir requires a path"),
+            (vec!["--unexpected"], "unexpected argument"),
+        ] {
+            let err = run_prepare(args.into_iter().map(str::to_owned).collect()).unwrap_err();
+            assert!(err.contains(expected), "{err}");
+        }
+
+        for (args, expected) in [
+            (Vec::<&str>::new(), "usage: bioscript inspect"),
+            (vec!["--input-index"], "--input-index requires a path"),
+            (vec!["--reference-file"], "--reference-file requires a path"),
+            (
+                vec!["--reference-index"],
+                "--reference-index requires a path",
+            ),
+            (vec!["input.txt", "extra"], "unexpected argument"),
+        ] {
+            let err = run_inspect(args.into_iter().map(str::to_owned).collect()).unwrap_err();
+            assert!(err.contains(expected), "{err}");
+        }
+    }
+
+    #[test]
+    fn validation_command_covers_report_success_and_error_paths() {
+        let dir = temp_dir("validation");
+        let input = dir.join("input.yaml");
+        fs::write(&input, "schema: bioscript:variant:1.0\n").unwrap();
+        let report = dir.join("reports/report.txt");
+
+        run_validation_command(
+            vec![
+                input.display().to_string(),
+                "--report".to_owned(),
+                report.display().to_string(),
+            ],
+            "usage",
+            |_| Ok(empty_report()),
+        )
+        .unwrap();
+        assert!(
+            fs::read_to_string(&report)
+                .unwrap()
+                .contains("files_scanned")
+        );
+
+        let err =
+            run_validation_command(Vec::new(), "usage text", |_| Ok(empty_report())).unwrap_err();
+        assert_eq!(err, "usage text");
+
+        let err =
+            run_validation_command(vec!["--report".to_owned()], "usage", |_| Ok(empty_report()))
+                .unwrap_err();
+        assert!(err.contains("--report requires a path"));
+
+        let err = run_validation_command(vec!["one".to_owned(), "two".to_owned()], "usage", |_| {
+            Ok(empty_report())
+        })
+        .unwrap_err();
+        assert!(err.contains("unexpected argument"));
+
+        let err = run_validation_command(vec!["input".to_owned()], "usage", |_| {
+            Err("validator failed".to_owned())
+        })
+        .unwrap_err();
+        assert_eq!(err, "validator failed");
+    }
+
+    #[test]
+    fn public_validation_and_inspect_commands_cover_successful_argument_branches() {
+        let dir = temp_dir("public-commands");
+        let vcf = dir.join("sample.vcf");
+        fs::write(
+            &vcf,
+            "##fileformat=VCFv4.3\n\
+             #CHROM\tPOS\tID\tREF\tALT\tQUAL\tFILTER\tINFO\tFORMAT\tSAMPLE\n\
+             1\t10\trs10\tA\tG\t.\tPASS\t.\tGT\t0/1\n",
+        )
+        .unwrap();
+        let index = dir.join("sample.vcf.tbi");
+        let reference = dir.join("ref.fa");
+        let reference_index = dir.join("ref.fa.fai");
+        fs::write(&index, b"index").unwrap();
+        fs::write(&reference, b">chr1\nA\n").unwrap();
+        fs::write(&reference_index, b"chr1\t1\t6\t1\t2\n").unwrap();
+
+        run_inspect(vec![
+            vcf.display().to_string(),
+            "--input-index".to_owned(),
+            index.display().to_string(),
+            "--reference-file".to_owned(),
+            reference.display().to_string(),
+            "--reference-index".to_owned(),
+            reference_index.display().to_string(),
+        ])
+        .unwrap();
+
+        let invalid_variant = dir.join("invalid-variant.yaml");
+        fs::write(&invalid_variant, "schema: bioscript:variant:1.0\n").unwrap();
+        let variant_report = dir.join("variant-report.txt");
+        let err = run_validate_variants(vec![
+            invalid_variant.display().to_string(),
+            "--report".to_owned(),
+            variant_report.display().to_string(),
+        ])
+        .unwrap_err();
+        assert!(err.contains("validation found"));
+        assert!(
+            fs::read_to_string(&variant_report)
+                .unwrap()
+                .contains("errors:")
+        );
+
+        let invalid_panel = dir.join("invalid-panel.yaml");
+        fs::write(&invalid_panel, "schema: bioscript:panel:1.0\n").unwrap();
+        let panel_report = dir.join("panel-report.txt");
+        let err = run_validate_panels(vec![
+            invalid_panel.display().to_string(),
+            "--report".to_owned(),
+            panel_report.display().to_string(),
+        ])
+        .unwrap_err();
+        assert!(err.contains("validation found"));
+        assert!(
+            fs::read_to_string(&panel_report)
+                .unwrap()
+                .contains("errors:")
+        );
+    }
+}

rust/bioscript-ffi/Cargo.toml

@@ -10,6 +10,7 @@ crate-type = ["rlib", "staticlib", "cdylib"]
 bioscript-core = { path = "../bioscript-core" }
 bioscript-formats = { path = "../bioscript-formats" }
 bioscript-runtime = { path = "../bioscript-runtime" }
+bioscript-schema = { path = "../bioscript-schema" }
 jni = "0.21"
 monty = { path = "../../monty/crates/monty" }
 serde = { version = "1.0", features = ["derive"] }

rust/bioscript-ffi/src/c_api.rs

@@ -0,0 +1,83 @@
+use std::{
+    ffi::{CStr, CString},
+    os::raw::c_char,
+};
+
+use crate::{RunFileRequest, RunFileResult, run_file_request, types::FfiResult};
+
+#[unsafe(no_mangle)]
+/// Executes a bioscript run request encoded as a UTF-8 JSON C string.
+///
+/// # Safety
+///
+/// `request_json` must either be null or point to a valid, NUL-terminated C
+/// string that remains alive for the duration of this call.
+pub unsafe extern "C" fn bioscript_run_file_json(request_json: *const c_char) -> *mut c_char {
+    let response = unsafe {
+        if request_json.is_null() {
+            FfiResult::<RunFileResult> {
+                ok: false,
+                value: None,
+                error: Some("request_json was null".to_owned()),
+            }
+        } else {
+            parse_and_run_request(request_json)
+        }
+    };
+
+    encode_response(&response)
+}
+
+unsafe fn parse_and_run_request(request_json: *const c_char) -> FfiResult<RunFileResult> {
+    match unsafe { CStr::from_ptr(request_json) }.to_str() {
+        Ok(value) => match serde_json::from_str::<RunFileRequest>(value) {
+            Ok(request) => match run_file_request(request) {
+                Ok(result) => FfiResult {
+                    ok: true,
+                    value: Some(result),
+                    error: None,
+                },
+                Err(error) => FfiResult::<RunFileResult> {
+                    ok: false,
+                    value: None,
+                    error: Some(error),
+                },
+            },
+            Err(error) => FfiResult::<RunFileResult> {
+                ok: false,
+                value: None,
+                error: Some(format!("invalid request JSON: {error}")),
+            },
+        },
+        Err(error) => FfiResult::<RunFileResult> {
+            ok: false,
+            value: None,
+            error: Some(format!("request_json was not valid UTF-8: {error}")),
+        },
+    }
+}
+
+fn encode_response(response: &FfiResult<RunFileResult>) -> *mut c_char {
+    match serde_json::to_string(response) {
+        Ok(json) => match CString::new(json) {
+            Ok(value) => value.into_raw(),
+            Err(_) => std::ptr::null_mut(),
+        },
+        Err(_) => std::ptr::null_mut(),
+    }
+}
+
+#[unsafe(no_mangle)]
+/// Frees a string previously returned by [`bioscript_run_file_json`].
+///
+/// # Safety
+///
+/// `ptr` must be null or a pointer returned by [`CString::into_raw`] from this
+/// library, and it must not be freed more than once.
+pub unsafe extern "C" fn bioscript_free_string(ptr: *mut c_char) {
+    if !ptr.is_null() {
+        unsafe {
+            let _ = CString::from_raw(ptr);
+        }
+    }
+}