Merge pull request #63 from OpenMined/madhava/more-coverage

Madhava/more coverage

Author: madhavajay

coverage.sh

@@ -25,7 +25,7 @@ Usage: ./coverage.sh [--full-clean|-c] [--open] [--large] [--all-tests] [--no-li
   --no-lint         Skip cargo fmt and clippy checks
   --focused-test    Run one focused integration test target:
                     file_formats, formats_lib, inspect, prepare, cli, cli_bin, schema, core,
-                    runtime_lib, runtime_security, or runtime_resources
+                    reporting_lib, runtime_lib, runtime_security, or runtime_resources
 
 Environment:
   AUTO_INSTALL_LLVM_COV=0    Do not auto-install cargo-llvm-cov
@@ -83,6 +83,7 @@ PACKAGES=(
   bioscript-cli
   bioscript-core
   bioscript-formats
+  bioscript-reporting
   bioscript-runtime
   bioscript-schema
 )
@@ -176,12 +177,16 @@ if [[ -n "$FOCUSED_TEST" ]]; then
       env "${COV_ENV[@]}" cargo llvm-cov --no-report -p bioscript-cli --bin bioscript
       ;;
     schema)
+      env "${COV_ENV[@]}" cargo llvm-cov --no-report -p bioscript-schema --lib
       env "${COV_ENV[@]}" cargo llvm-cov --no-report -p bioscript-schema --test validate_variants -- --nocapture --test-threads="$TEST_THREADS"
       ;;
     core)
       env "${COV_ENV[@]}" cargo llvm-cov --no-report -p bioscript-core --lib
       env "${COV_ENV[@]}" cargo llvm-cov --no-report -p bioscript-core --test source_size -- --nocapture --test-threads="$TEST_THREADS"
       ;;
+    reporting_lib)
+      env "${COV_ENV[@]}" cargo llvm-cov --no-report -p bioscript-reporting --lib
+      ;;
     runtime_lib)
       env "${COV_ENV[@]}" cargo llvm-cov --no-report -p bioscript-runtime --lib
       ;;
@@ -206,9 +211,11 @@ else
   env "${COV_ENV[@]}" cargo llvm-cov --no-report -p bioscript-formats --test prepare -- --nocapture --test-threads="$TEST_THREADS"
   env "${COV_ENV[@]}" cargo llvm-cov --no-report -p bioscript-cli --test cli -- --nocapture --test-threads="$TEST_THREADS"
   env "${COV_ENV[@]}" cargo llvm-cov --no-report -p bioscript-cli --bin bioscript
+  env "${COV_ENV[@]}" cargo llvm-cov --no-report -p bioscript-schema --lib
   env "${COV_ENV[@]}" cargo llvm-cov --no-report -p bioscript-schema --test validate_variants -- --nocapture --test-threads="$TEST_THREADS"
   env "${COV_ENV[@]}" cargo llvm-cov --no-report -p bioscript-core --lib
   env "${COV_ENV[@]}" cargo llvm-cov --no-report -p bioscript-core --test source_size -- --nocapture --test-threads="$TEST_THREADS"
+  env "${COV_ENV[@]}" cargo llvm-cov --no-report -p bioscript-reporting --lib
   env "${COV_ENV[@]}" cargo llvm-cov --no-report -p bioscript-runtime --lib
   env "${COV_ENV[@]}" cargo llvm-cov --no-report -p bioscript-runtime --test security -- --nocapture --test-threads="$TEST_THREADS"
   env "${COV_ENV[@]}" cargo llvm-cov --no-report -p bioscript-runtime --test resources_coverage -- --nocapture --test-threads="$TEST_THREADS"

rust/bioscript-cli/src/cli_bootstrap.rs

@@ -417,3 +417,162 @@ fn write_timing_report(path: &PathBuf, timings: &[StageTiming]) -> Result<(), St
     fs::write(path, output)
         .map_err(|err| format!("failed to write timing report {}: {err}", path.display()))
 }
+
+#[cfg(test)]
+mod cli_bootstrap_tests {
+    use super::*;
+    use std::time::{SystemTime, UNIX_EPOCH};
+
+    fn temp_dir(name: &str) -> PathBuf {
+        let unique = SystemTime::now()
+            .duration_since(UNIX_EPOCH)
+            .unwrap()
+            .as_nanos();
+        let dir = env::temp_dir().join(format!(
+            "bioscript-cli-bootstrap-{name}-{}-{unique}",
+            std::process::id()
+        ));
+        fs::create_dir_all(&dir).unwrap();
+        dir
+    }
+
+    #[test]
+    fn parse_cli_options_consumes_paths_loader_limits_and_filters() {
+        let options = parse_cli_options(vec![
+            "script.bs".to_owned(),
+            "--root".to_owned(),
+            "root".to_owned(),
+            "--input-file".to_owned(),
+            "input.txt".to_owned(),
+            "--output-file".to_owned(),
+            "output.txt".to_owned(),
+            "--participant-id".to_owned(),
+            "p1".to_owned(),
+            "--trace-report".to_owned(),
+            "trace.tsv".to_owned(),
+            "--timing-report".to_owned(),
+            "timing.tsv".to_owned(),
+            "--filter".to_owned(),
+            "tag=pgx".to_owned(),
+            "--cache-dir".to_owned(),
+            "cache".to_owned(),
+            "--input-format".to_owned(),
+            "text".to_owned(),
+            "--input-index".to_owned(),
+            "input.idx".to_owned(),
+            "--reference-file".to_owned(),
+            "ref.fa".to_owned(),
+            "--reference-index".to_owned(),
+            "ref.fa.fai".to_owned(),
+            "--max-duration-ms".to_owned(),
+            "250".to_owned(),
+            "--max-memory-bytes".to_owned(),
+            "1024".to_owned(),
+            "--max-allocations".to_owned(),
+            "2000".to_owned(),
+            "--max-recursion-depth".to_owned(),
+            "50".to_owned(),
+            "--auto-index".to_owned(),
+        ])
+        .unwrap();
+
+        assert_eq!(options.script_path, Some(PathBuf::from("script.bs")));
+        assert_eq!(options.root, Some(PathBuf::from("root")));
+        assert_eq!(options.input_file.as_deref(), Some("input.txt"));
+        assert_eq!(options.output_file.as_deref(), Some("output.txt"));
+        assert_eq!(options.participant_id.as_deref(), Some("p1"));
+        assert_eq!(options.trace_report, Some(PathBuf::from("trace.tsv")));
+        assert_eq!(options.timing_report, Some(PathBuf::from("timing.tsv")));
+        assert_eq!(options.filters, vec!["tag=pgx"]);
+        assert_eq!(options.cache_dir, Some(PathBuf::from("cache")));
+        assert_eq!(options.loader.format, Some(GenotypeSourceFormat::Text));
+        assert_eq!(options.loader.input_index, Some(PathBuf::from("input.idx")));
+        assert_eq!(options.loader.reference_file, Some(PathBuf::from("ref.fa")));
+        assert_eq!(
+            options.loader.reference_index,
+            Some(PathBuf::from("ref.fa.fai"))
+        );
+        assert!(options.auto_index);
+    }
+
+    #[test]
+    fn parse_cli_options_reports_missing_values_and_unexpected_arguments() {
+        for (flag, message) in [
+            ("--root", "--root requires"),
+            ("--input-file", "--input-file requires"),
+            ("--output-file", "--output-file requires"),
+            ("--participant-id", "--participant-id requires"),
+            ("--trace-report", "--trace-report requires"),
+            ("--timing-report", "--timing-report requires"),
+            ("--filter", "--filter requires"),
+            ("--cache-dir", "--cache-dir requires"),
+            ("--input-format", "--input-format requires"),
+            ("--input-index", "--input-index requires"),
+            ("--reference-file", "--reference-file requires"),
+            ("--reference-index", "--reference-index requires"),
+            ("--max-duration-ms", "--max-duration-ms requires"),
+            ("--max-memory-bytes", "--max-memory-bytes requires"),
+            ("--max-allocations", "--max-allocations requires"),
+            ("--max-recursion-depth", "--max-recursion-depth requires"),
+        ] {
+            assert!(parse_err(vec![flag.to_owned()]).contains(message));
+        }
+        assert!(parse_err(vec![
+            "script.bs".to_owned(),
+            "extra.bs".to_owned(),
+        ])
+        .contains("unexpected argument"));
+        assert!(parse_err(vec![
+            "--input-format".to_owned(),
+            "bad".to_owned(),
+        ])
+        .contains("invalid --input-format"));
+        assert!(parse_err(vec![
+            "--max-duration-ms".to_owned(),
+            "bad".to_owned(),
+        ])
+        .contains("invalid --max-duration-ms"));
+    }
+
+    #[test]
+    fn write_timing_report_creates_parent_and_sanitizes_tabs() {
+        let dir = temp_dir("timing");
+        let path = dir.join("nested/timing.tsv");
+        write_timing_report(
+            &path,
+            &[
+                StageTiming {
+                    stage: "stage1".to_owned(),
+                    duration_ms: 12,
+                    detail: "a\tb".to_owned(),
+                },
+                StageTiming {
+                    stage: "stage2".to_owned(),
+                    duration_ms: 0,
+                    detail: "ok".to_owned(),
+                },
+            ],
+        )
+        .unwrap();
+        let text = fs::read_to_string(&path).unwrap();
+        assert!(text.starts_with("stage\tduration_ms\tdetail\n"));
+        assert!(text.contains("stage1\t12\ta b"));
+
+        fs::remove_dir_all(dir).unwrap();
+    }
+
+    #[test]
+    fn prepare_cli_indexes_noops_when_auto_index_is_disabled() {
+        let mut options = default_cli_options();
+        let timings = prepare_cli_indexes(Path::new("."), &mut options).unwrap();
+        assert!(timings.is_empty());
+        assert!(options.loader.input_index.is_none());
+    }
+
+    fn parse_err(args: Vec<String>) -> String {
+        match parse_cli_options(args) {
+            Ok(_) => panic!("expected CLI parse to fail"),
+            Err(err) => err,
+        }
+    }
+}

rust/bioscript-cli/src/cli_commands.rs

@@ -258,3 +258,178 @@ fn run_validate_assays(args: Vec<String>) -> Result<(), String> {
 
     Ok(())
 }
+
+#[cfg(test)]
+mod cli_command_tests {
+    use super::*;
+    use std::time::{SystemTime, UNIX_EPOCH};
+
+    fn temp_dir(name: &str) -> PathBuf {
+        let unique = SystemTime::now()
+            .duration_since(UNIX_EPOCH)
+            .unwrap()
+            .as_nanos();
+        let dir = env::temp_dir().join(format!(
+            "bioscript-cli-command-{name}-{}-{unique}",
+            std::process::id()
+        ));
+        fs::create_dir_all(&dir).unwrap();
+        dir
+    }
+
+    fn valid_variant_yaml() -> &'static str {
+        r#"
+schema: bioscript:variant:1.0
+version: "1.0"
+name: Test variant
+gene: ABC
+identifiers:
+  rsids: [rs1]
+coordinates:
+  grch38:
+    chrom: "1"
+    pos: 100
+alleles:
+  kind: snv
+  ref: G
+  alts: [A]
+"#
+    }
+
+    #[test]
+    fn prepare_and_inspect_commands_validate_arguments() {
+        assert!(run_prepare(vec!["--root".to_owned()])
+            .unwrap_err()
+            .contains("--root requires"));
+        assert!(run_prepare(vec!["--input-file".to_owned()])
+            .unwrap_err()
+            .contains("--input-file requires"));
+        assert!(run_prepare(vec![
+            "--input-format".to_owned(),
+            "not-a-format".to_owned(),
+        ])
+        .unwrap_err()
+        .contains("invalid --input-format"));
+        assert!(run_prepare(vec!["--unexpected".to_owned()])
+            .unwrap_err()
+            .contains("unexpected argument"));
+
+        assert!(run_inspect(Vec::new()).unwrap_err().contains("usage"));
+        assert!(run_inspect(vec!["a.txt".to_owned(), "b.txt".to_owned()])
+            .unwrap_err()
+            .contains("unexpected argument"));
+        assert!(run_inspect(vec!["sample.cram".to_owned(), "--input-index".to_owned()])
+            .unwrap_err()
+            .contains("--input-index requires"));
+    }
+
+    #[test]
+    fn yaml_manifest_extension_matching_is_case_sensitive_by_contract() {
+        assert!(is_yaml_manifest(Path::new("panel.yaml")));
+        assert!(is_yaml_manifest(Path::new("panel.yml")));
+        assert!(!is_yaml_manifest(Path::new("panel.YAML")));
+        assert!(!is_yaml_manifest(Path::new("panel.json")));
+    }
+
+    #[test]
+    fn validate_variants_writes_report_and_surfaces_errors() {
+        let dir = temp_dir("variants");
+        let valid = dir.join("variant.yaml");
+        let report = dir.join("reports/variant.txt");
+        fs::write(&valid, valid_variant_yaml()).unwrap();
+
+        run_validate_variants(vec![
+            valid.display().to_string(),
+            "--report".to_owned(),
+            report.display().to_string(),
+        ])
+        .unwrap();
+        assert!(fs::read_to_string(&report).unwrap().contains("files_scanned"));
+
+        let invalid = dir.join("invalid.yaml");
+        fs::write(&invalid, "schema: bioscript:variant:1.0\n").unwrap();
+        let err = run_validate_variants(vec![invalid.display().to_string()]).unwrap_err();
+        assert!(err.contains("validation found"));
+
+        assert!(run_validate_variants(Vec::new()).unwrap_err().contains("usage"));
+        assert!(run_validate_variants(vec![valid.display().to_string(), "--report".to_owned()])
+            .unwrap_err()
+            .contains("--report requires"));
+        assert!(run_validate_variants(vec![
+            valid.display().to_string(),
+            "extra".to_owned(),
+        ])
+        .unwrap_err()
+        .contains("unexpected argument"));
+
+        fs::remove_dir_all(dir).unwrap();
+    }
+
+    #[test]
+    fn validate_panels_and_assays_cover_report_and_error_paths() {
+        let dir = temp_dir("panels-assays");
+        let variant = dir.join("variant.yaml");
+        fs::write(&variant, valid_variant_yaml()).unwrap();
+
+        let panel = dir.join("panel.yaml");
+        fs::write(
+            &panel,
+            r#"
+schema: bioscript:panel:1.0
+version: "1.0"
+name: Test panel
+members:
+  - kind: variant
+    path: variant.yaml
+"#,
+        )
+        .unwrap();
+        let panel_report = dir.join("reports/panel.txt");
+        run_validate_panels(vec![
+            panel.display().to_string(),
+            "--report".to_owned(),
+            panel_report.display().to_string(),
+        ])
+        .unwrap();
+        assert!(panel_report.exists());
+
+        let assay = dir.join("assay.yaml");
+        fs::write(
+            &assay,
+            r#"
+schema: bioscript:assay:1.0
+version: "1.0"
+name: Test assay
+members:
+  - kind: variant
+    path: variant.yaml
+"#,
+        )
+        .unwrap();
+        let assay_report = dir.join("reports/assay.txt");
+        run_validate_assays(vec![
+            assay.display().to_string(),
+            "--report".to_owned(),
+            assay_report.display().to_string(),
+        ])
+        .unwrap();
+        assert!(assay_report.exists());
+
+        assert!(run_validate_panels(Vec::new()).unwrap_err().contains("usage"));
+        assert!(run_validate_panels(vec![panel.display().to_string(), "--report".to_owned()])
+            .unwrap_err()
+            .contains("--report requires"));
+        assert!(run_validate_panels(vec![panel.display().to_string(), "extra".to_owned()])
+            .unwrap_err()
+            .contains("unexpected argument"));
+        assert!(run_validate_assays(Vec::new()).unwrap_err().contains("usage"));
+        assert!(run_validate_assays(vec![assay.display().to_string(), "--report".to_owned()])
+            .unwrap_err()
+            .contains("--report requires"));
+        assert!(run_validate_assays(vec![assay.display().to_string(), "extra".to_owned()])
+            .unwrap_err()
+            .contains("unexpected argument"));
+
+        fs::remove_dir_all(dir).unwrap();
+    }
+}