From 5c0ba396e47a286d0d59d43a7e46571bc5cfdd5d Mon Sep 17 00:00:00 2001
From: d3v-null <dev.null@curtin.edu.au>
Date: Fri, 24 Jan 2025 03:17:43 +0000
Subject: [PATCH] refactor for better coverage

---
 src/cli/error.rs      |  3 ++-
 src/cli/peel/error.rs |  5 +++++
 src/cli/peel/mod.rs   | 31 ++++++++++++++-----------------
 src/cli/peel/tests.rs | 18 +++++++++++++++++-
 4 files changed, 38 insertions(+), 19 deletions(-)

diff --git a/src/cli/error.rs b/src/cli/error.rs
index 3a06797d..c1b3adb2 100644
--- a/src/cli/error.rs
+++ b/src/cli/error.rs
@@ -41,7 +41,7 @@ pub enum HyperdriveError {
     DiCalibrate(String),
 
     /// An error related to peeling.
-    #[error("{0}\n\nSee for more info: {URL}/*****.html")]
+    #[error("{0}\n\nSee for more info: {URL}/user/peel/intro.html")]
     Peel(String),
 
     /// An error related to solutions-apply.
@@ -173,6 +173,7 @@ impl From<PeelArgsError> for HyperdriveError {
     fn from(e: PeelArgsError) -> Self {
         match e {
             PeelArgsError::NoOutput
+            | PeelArgsError::TooManyIonoSub { .. }
             | PeelArgsError::ZeroPasses
             | PeelArgsError::ZeroLoops
             | PeelArgsError::ParseIonoTimeAverageFactor(_)
diff --git a/src/cli/peel/error.rs b/src/cli/peel/error.rs
index 66bcb7d2..91e83a54 100644
--- a/src/cli/peel/error.rs
+++ b/src/cli/peel/error.rs
@@ -7,6 +7,11 @@ pub(crate) enum PeelArgsError {
     #[error("No calibration output was specified. There must be at least one calibration solution file.")]
     NoOutput,
 
+    #[error(
+        "The number of sources to subtract ({total}) is less than the number of sources to iono subtract ({iono})"
+    )]
+    TooManyIonoSub { total: usize, iono: usize },
+
     #[error("The number of iono sub passes cannot be 0")]
     ZeroPasses,
 
diff --git a/src/cli/peel/mod.rs b/src/cli/peel/mod.rs
index ccf4d0bd..9d10742e 100644
--- a/src/cli/peel/mod.rs
+++ b/src/cli/peel/mod.rs
@@ -265,25 +265,22 @@ impl PeelArgs {
                 input_vis_params.timeblocks.first().median,
                 input_vis_params.dut1,
             );
-            if apply_precession {
-                let srclist = srclist_args.parse(
-                    obs_context.phase_centre,
+            let (lst_rad, lat_rad) = if apply_precession {
+                (
                     precession_info.lmst_j2000,
                     precession_info.array_latitude_j2000,
-                    &obs_context.get_veto_freqs(),
-                    &*beam,
-                )?;
-                (srclist, precession_info.lmst_j2000)
+                )
             } else {
-                let srclist = srclist_args.parse(
-                    obs_context.phase_centre,
-                    precession_info.lmst,
-                    latitude_rad,
-                    &obs_context.get_veto_freqs(),
-                    &*beam,
-                )?;
-                (srclist, precession_info.lmst)
-            }
+                (precession_info.lmst, latitude_rad)
+            };
+            let srclist = srclist_args.parse(
+                obs_context.phase_centre,
+                lst_rad,
+                lat_rad,
+                &obs_context.get_veto_freqs(),
+                &*beam,
+            )?;
+            (srclist, lst_rad)
         };
 
         // Check that the number of sources to peel, iono subtract and subtract
@@ -292,7 +289,7 @@ impl PeelArgs {
         let _max_num_sources = match (num_sources_to_iono_subtract, num_sources_to_subtract) {
             (Some(is), Some(s)) => {
                 if s < is {
-                    panic!("The number of sources to subtract ({s}) must be at least equal to the number of sources to iono subtract ({is})");
+                    return Err(PeelArgsError::TooManyIonoSub { total: s, iono: is }.into());
                 }
                 Some(s)
             }
diff --git a/src/cli/peel/tests.rs b/src/cli/peel/tests.rs
index c45c2204..ec927e4c 100644
--- a/src/cli/peel/tests.rs
+++ b/src/cli/peel/tests.rs
@@ -9,9 +9,10 @@ use crate::{
     cli::common::{InputVisArgs, SkyModelWithVetoArgs},
     params::{InputVisParams, OutputVisParams, PeelParams},
     tests::{get_reduced_1090008640_raw, DataAsStrings},
+    HyperdriveError,
 };
 
-use super::PeelArgs;
+use super::{PeelArgs, PeelCliArgs};
 
 #[track_caller]
 fn get_reduced_1090008640() -> PeelArgs {
@@ -203,3 +204,18 @@ fn time_averaging_explicit_output_clip() {
 //   --output-vis-time-average - output averaging settings
 //
 // this requires test data with more than one timestep
+
+// testing that parse() catches invalid number of iono sources
+#[test]
+fn handle_iono_greater_than_total() {
+    let mut args = get_reduced_1090008640();
+    args.peel_args = PeelCliArgs {
+        num_sources_to_iono_subtract: Some(2),
+        num_sources_to_subtract: Some(1),
+        ..Default::default()
+    };
+    match args.parse() {
+        Err(HyperdriveError::Generic(_)) => {} // expected
+        _ => panic!("Expected TooManyIonoSub"),
+    };
+}