Repository: extendr/extendr
Pull request #169 (2021-02-02 12:02:01)

Convert Vec<Robj> to Robj
Not sure if this is useful at all or if we want to have this kind of wrappers. 
It took me a while to realize it was this simple :)


andy-thomason: This is a side effect of the List() wrapper which will convert many types, including Vec<Robj> to lists.

I would like to finish the NamedList wrapper if anyone wants to give it a go. The same deal, but generate
two vectors, one for the names and the other for the data from one (&str, Into<Robj>) iterator.
andy-thomason: I'm trying to get time to document the wrappers in the book. Forgive me if I'm a bit slow.
andy-thomason: I'll try to put a List example into the main module documentation.

Patch:
diff --git a/ci-cargo.ps1 b/ci-cargo.ps1
new file mode 100644
index 000000000..0e5adfd3c
--- /dev/null
+++ b/ci-cargo.ps1
@@ -0,0 +1,71 @@
+function ci-cargo {
+
+    param(
+        [Parameter(Position = 0, ValueFromRemainingArguments)]
+        [String[]]
+        $CargoArgs,
+        [String]
+        $ActionName
+    )
+
+
+    try {    
+        echo "::group::$ActionName"
+        echo "Running cargo $CargoArgs"
+        cargo $CargoArgs
+        if($LASTEXITCODE -ne  0) {
+            throw $LASTEXITCODE
+        }
+    }
+    catch {
+        if ($ActionName -ne $null -and $ActionName -ne "") {
+            $ActionName = "'$ActionName': "
+        }
+        $err_msg = "$($ActionName)cargo failed with code $LASTEXITCODE (args: $CargoArgs)"
+        echo "::error::$err_msg"
+        Write-Error -Message "$err_msg" -ErrorAction Stop 
+    }
+    finally {
+        echo "::endgroup::"
+    }
+
+    <#
+        .SYNOPSIS
+        Runs cargo with specified args, adapting error handling and output to CI.
+
+        .DESCRIPTION
+        Runs cargo in a `try` block, catches exceptions and non-zero exit codes.
+        Explicitly logs the beginning and the end of cargo execution, as well as the error message.
+
+        .PARAMETER CargoArgs
+        Arguments passed to cargo, as-is.
+        Note that `--` separator is handled by powershell itself,
+        so it should be wrapped in quotes `'--'` and passed as string.
+
+        .PARAMETER ActionName
+        Optional string that is used to format logs and error messages.
+
+        .INPUTS
+        None. You cannot pipe objects.
+
+        .OUTPUTS
+        No explicit output.
+
+        .EXAMPLE
+        PS> ci-cargo --version
+            ::group::
+            Running cargo --version
+            cargo 1.49.0 (d00d64df9 2020-12-05)
+            ::endgroup::
+
+        .EXAMPLE
+        PS> ci-cargo -ActioName "Build"  build
+
+        .EXAMPLE
+        PS> ci-cargo +stable-x86_64-pc-windows-gnu test --features tests-all --target i686-pc-windows-gnu '--' --nocapture -ActionName "Called from documentation"
+
+        .LINK
+        Used by: https://github.com/extendr/extendr
+    #>
+    
+}
diff --git a/extendr-api/src/functions.rs b/extendr-api/src/functions.rs
index 8363a0570..8ed122986 100644
--- a/extendr-api/src/functions.rs
+++ b/extendr-api/src/functions.rs
@@ -34,8 +34,7 @@ pub fn new_env_with_capacity(capacity: usize) -> Robj {
         // Unhashed envirnment
         call!("new.env", FALSE, global_env(), 0).unwrap()
     } else {
-        // Hashed environment, the pidgeon hole principle
-        // ensures there are empty slots in the hash table.
+        // Hashed environment for larger hashmaps.
         call!("new.env", TRUE, global_env(), capacity as i32 * 2 + 1).unwrap()
     }
 }
diff --git a/extendr-api/src/iter.rs b/extendr-api/src/iter.rs
index 3606c6153..e9f16db11 100644
--- a/extendr-api/src/iter.rs
+++ b/extendr-api/src/iter.rs
@@ -1,6 +1,24 @@
 use crate::*;
 
-// Iterator over the objects in a VECSXP, EXPRSXP or WEAKREFSXP.
+/// Iterator over the objects in a VECSXP, EXPRSXP or WEAKREFSXP.
+///
+/// ```
+/// use extendr_api::prelude::*;
+/// test! {
+///     let my_list = list!(a = 1, b = 2);
+///     let mut total = 0;
+///     for robj in my_list.as_list_iter().unwrap() {
+///       if let Some(val) = robj.as_integer() {
+///         total += val;
+///       }
+///     }
+///     assert_eq!(total, 3);
+///    
+///     for name in my_list.names().unwrap() {
+///        assert!(name == "a" || name == "b")
+///     }
+/// }
+/// ```
 #[derive(Clone)]
 pub struct ListIter {
     vector: Robj,
@@ -42,37 +60,95 @@ impl Iterator for ListIter {
     }
 }
 
+/// Generalised iterator of numbers and logical. See Int, Real and Logical.
+pub struct SliceIter<T> {
+    // Control lifetime of vector to make sure the memory is not freed.
+    #[allow(dead_code)]
+    vector: Robj,
+    i: usize,
+    len: usize,
+    ptr: *const T,
+}
+
+impl<T> SliceIter<T> {
+    // A new, empty list iterator.
+    pub fn new() -> Self {
+        SliceIter {
+            vector: ().into(),
+            i: 0,
+            len: 0,
+            ptr: std::ptr::null(),
+        }
+    }
+
+    pub fn from_slice(vector: Robj, slice: &[T]) -> Self {
+        SliceIter {
+            vector,
+            i: 0,
+            len: slice.len(),
+            ptr: slice.as_ptr(),
+        }
+    }
+}
+
+/// Basis of Int, Real and Logical.
+impl<T: Copy> Iterator for SliceIter<T> {
+    type Item = T;
+
+    fn size_hint(&self) -> (usize, Option<usize>) {
+        (self.len, Some(self.len))
+    }
+
+    fn next(&mut self) -> Option<Self::Item> {
+        let i = self.i;
+        self.i += 1;
+        if i >= self.len {
+            self.i = self.len;
+            None
+        } else {
+            unsafe { Some(*self.ptr.offset(i as isize)) }
+        }
+    }
+
+    fn nth(&mut self, n: usize) -> Option<Self::Item> {
+        self.i += n;
+        self.next()
+    }
+}
+
 /// Iterator over name-value pairs in lists.
 pub type NamedListIter = std::iter::Zip<StrIter, ListIter>;
 
 /// Iterator over primitives in integer objects.
-pub type IntegerIter<'a> = std::iter::Cloned<std::slice::Iter<'a, i32>>;
+/// ```
+/// use extendr_api::prelude::*;
+///
+/// fn add(a: Int, b: Int) -> Robj {
+///     a.zip(b).map(|(a, b)| a+b).collect_robj()
+/// }
+/// ```
+pub type Int = SliceIter<i32>;
 
 /// Iterator over primitives in real objects.
-pub type RealIter<'a> = std::iter::Cloned<std::slice::Iter<'a, f64>>;
+/// ```
+/// use extendr_api::prelude::*;
+///
+/// fn add1(a: Real) -> Robj {
+///     a.map(|a| a + 1.0).collect_robj()
+/// }
+/// ```
+pub type Real = SliceIter<f64>;
 
 /// Iterator over primitives in logical objects.
-pub type LogicalIter<'a> = std::iter::Cloned<std::slice::Iter<'a, Bool>>;
-
-/// Iterator over the objects in a vector or string.
-///
 /// ```
 /// use extendr_api::prelude::*;
-/// test! {
-///     let my_list = list!(a = 1, b = 2);
-///     let mut total = 0;
-///     for robj in my_list.as_list_iter().unwrap() {
-///       if let Some(val) = robj.as_integer() {
-///         total += val;
-///       }
-///     }
-///     assert_eq!(total, 3);
-///    
-///     for name in my_list.names().unwrap() {
-///        assert!(name == "a" || name == "b")
-///     }
+///
+/// fn all_true(mut a: Logical) -> bool {
+///     a.all(|a| a.is_true())
 /// }
 /// ```
+pub type Logical = SliceIter<Bool>;
+
 #[derive(Clone)]
 pub struct PairlistIter {
     root_obj: Robj,
diff --git a/extendr-api/src/lib.rs b/extendr-api/src/lib.rs
index 8f42ef53c..0d07fe4cc 100644
--- a/extendr-api/src/lib.rs
+++ b/extendr-api/src/lib.rs
@@ -41,6 +41,7 @@
 //! test! {
 //!     // An R object with a single string "hello"
 //!     let character = r!("hello");
+//!     let character = r!(["hello", "goodbye"]);
 //!    
 //!     // An R integer object with a single number 1L.
 //!     // Note that in Rust, 1 is an integer and 1.0 is a real.
@@ -52,6 +53,8 @@
 //!    
 //!     // An R real vector.
 //!     let real_vector = r!([1.0, 2.0]);
+//!     let real_vector = &[1.0, 2.0].iter().collect_robj();
+//!     let real_vector = r!(vec![1.0, 2.0]);
 //!    
 //!     // An R function object.
 //!     let function = R!(function(x, y) { x + y })?;
@@ -59,8 +62,16 @@
 //!     // A named list using the list! macro.
 //!     let list = list!(a = 1, b = 2);
 //!    
+//!     // An unnamed list (of R objects) using the List wrapper.
+//!     let list = r!(List(vec![1, 2, 3]));
+//!     let list = r!(List(vec!["a", "b", "c"]));
+//!     let list = r!(List(&[r!("a"), r!(1), r!(2.0)]));
+//!
 //!     // A symbol
 //!     let sym = sym!(wombat);
+//!
+//!     // A R vector using collect_robj()
+//!     let vector = (0..3).map(|x| x * 10).collect_robj();
 //! }
 //! ```
 //!
@@ -484,17 +495,17 @@ mod tests {
     }
 
     #[extendr]
-    pub fn f64_iter(x: RealIter) -> RealIter {
+    pub fn f64_iter(x: Real) -> Real {
         x
     }
 
     #[extendr]
-    pub fn i32_iter(x: IntegerIter) -> IntegerIter {
+    pub fn i32_iter(x: Int) -> Int {
         x
     }
 
     #[extendr]
-    pub fn bool_iter(x: LogicalIter) -> LogicalIter {
+    pub fn bool_iter(x: Logical) -> Logical {
         x
     }
 
@@ -641,19 +652,19 @@ mod tests {
                 assert_eq!(new_owned(wrap__bool_slice(robj.get())), robj);
 
                 // #[extendr]
-                // pub fn f64_iter(x: RealIter) -> RealIter { x }
+                // pub fn f64_iter(x: Real) -> Real { x }
 
                 let robj = r!([1., 2., 3.]);
                 assert_eq!(new_owned(wrap__f64_iter(robj.get())), robj);
 
                 // #[extendr]
-                // pub fn i32_iter(x: IntegerIter) -> IntegerIter { x }
+                // pub fn i32_iter(x: Int) -> Int { x }
 
                 let robj = r!([1, 2, 3]);
                 assert_eq!(new_owned(wrap__i32_iter(robj.get())), robj);
 
                 // #[extendr]
-                // pub fn bool_iter(x: LogicalIter) -> LogicalIter { x }
+                // pub fn bool_iter(x: Logical) -> Logical { x }
 
                 let robj = r!([TRUE, FALSE, TRUE]);
                 assert_eq!(new_owned(wrap__bool_iter(robj.get())), robj);
diff --git a/extendr-api/src/logical.rs b/extendr-api/src/logical.rs
index c8660331d..a87ae26f7 100644
--- a/extendr-api/src/logical.rs
+++ b/extendr-api/src/logical.rs
@@ -16,7 +16,7 @@ impl Bool {
 
     /// Test if TRUE
     pub fn is_true(&self) -> bool {
-        self.0 != 0
+        self.0 == 1
     }
 
     /// Test if FALSE
diff --git a/extendr-api/src/prelude.rs b/extendr-api/src/prelude.rs
index a2b2c4291..e4bc2e65c 100644
--- a/extendr-api/src/prelude.rs
+++ b/extendr-api/src/prelude.rs
@@ -51,5 +51,5 @@ pub use ndarray::*;
 pub use extendr_macros::{extendr, extendr_module};
 
 pub use super::iter::{
-    EnvIter, IntegerIter, ListIter, LogicalIter, PairlistIter, PairlistTagIter, RealIter, StrIter,
+    EnvIter, Int, ListIter, Logical, PairlistIter, PairlistTagIter, Real, StrIter,
 };
diff --git a/extendr-api/src/robj/from_robj.rs b/extendr-api/src/robj/from_robj.rs
index 9983eb1b1..f00898144 100644
--- a/extendr-api/src/robj/from_robj.rs
+++ b/extendr-api/src/robj/from_robj.rs
@@ -153,9 +153,9 @@ macro_rules! impl_iter_from_robj {
 
 impl_iter_from_robj!(StrIter, as_str_iter, "Not a character vector.");
 impl_iter_from_robj!(ListIter, as_list_iter, "Not a list.");
-impl_iter_from_robj!(IntegerIter<'a>, as_integer_iter, "Not an integer vector.");
-impl_iter_from_robj!(RealIter<'a>, as_real_iter, "Not a real vector.");
-impl_iter_from_robj!(LogicalIter<'a>, as_logical_iter, "Not a logical vector.");
+impl_iter_from_robj!(Int, as_integer_iter, "Not an integer vector.");
+impl_iter_from_robj!(Real, as_real_iter, "Not a real vector.");
+impl_iter_from_robj!(Logical, as_logical_iter, "Not a logical vector.");
 
 /// Pass-through Robj conversion.
 impl<'a> FromRobj<'a> for Robj {
diff --git a/extendr-api/src/robj/into_robj.rs b/extendr-api/src/robj/into_robj.rs
index 4791a2d52..24221772e 100644
--- a/extendr-api/src/robj/into_robj.rs
+++ b/extendr-api/src/robj/into_robj.rs
@@ -497,23 +497,23 @@ impl_from_into_iter! {&'a [T]}
 impl_from_as_iterator! {Range<T>}
 impl_from_as_iterator! {RangeInclusive<T>}
 
-impl<'a> From<RealIter<'a>> for Robj {
+impl From<Real> for Robj {
     /// Convert a real iterator into a vector.
-    fn from(val: RealIter) -> Self {
+    fn from(val: Real) -> Self {
         val.collect_robj()
     }
 }
 
-impl<'a> From<IntegerIter<'a>> for Robj {
+impl From<Int> for Robj {
     /// Convert an integer iterator into a vector.
-    fn from(val: IntegerIter) -> Self {
+    fn from(val: Int) -> Self {
         val.collect_robj()
     }
 }
 
-impl<'a> From<LogicalIter<'a>> for Robj {
+impl From<Logical> for Robj {
     /// Convert a logical iterator into a vector.
-    fn from(val: LogicalIter) -> Self {
+    fn from(val: Logical) -> Self {
         val.collect_robj()
     }
 }
@@ -527,3 +527,10 @@ impl<'a> From<HashMap<&'a str, Robj>> for Robj {
         res
     }
 }
+
+impl<'a> From<Vec<Robj>> for Robj {
+    /// Convert a vector of Robj into a list.
+    fn from(val: Vec<Robj>) -> Self {
+        List(val.iter()).into()
+    }
+}
diff --git a/extendr-api/src/robj/mod.rs b/extendr-api/src/robj/mod.rs
index 7b4602ff9..5fcdc066b 100644
--- a/extendr-api/src/robj/mod.rs
+++ b/extendr-api/src/robj/mod.rs
@@ -299,12 +299,9 @@ impl Robj {
     /// assert_eq!(tot, 6);
     /// }
     /// ```
-    pub fn as_integer_iter<'a>(&self) -> Option<IntegerIter<'a>>
-    where
-        Self: 'a,
-    {
+    pub fn as_integer_iter(&self) -> Option<Int> {
         if let Some(slice) = self.as_integer_slice() {
-            Some(slice.iter().cloned())
+            Some(Int::from_slice(self.to_owned(), slice))
         } else {
             None
         }
@@ -375,9 +372,9 @@ impl Robj {
     ///     assert_eq!((nt, nf, nna), (1, 1, 1));
     /// }
     /// ```
-    pub fn as_logical_iter(&self) -> Option<LogicalIter> {
+    pub fn as_logical_iter(&self) -> Option<Logical> {
         if let Some(slice) = self.as_logical_slice() {
-            Some(slice.iter().cloned())
+            Some(Logical::from_slice(self.to_owned(), slice))
         } else {
             None
         }
@@ -417,9 +414,9 @@ impl Robj {
     ///     assert_eq!(tot, 6.);
     /// }
     /// ```
-    pub fn as_real_iter(&self) -> Option<RealIter> {
+    pub fn as_real_iter(&self) -> Option<Real> {
         if let Some(slice) = self.as_real_slice() {
-            Some(slice.iter().cloned())
+            Some(Real::from_slice(self.to_owned(), slice))
         } else {
             None
         }
@@ -904,11 +901,7 @@ impl Robj {
     ///    assert_eq!(dim, vec![2, 2]);
     /// }
     /// ```
-    pub fn dim<'a>(&self) -> Option<IntegerIter<'a>>
-    where
-        Self: 'a,
-        Robj: 'a,
-    {
+    pub fn dim(&self) -> Option<Int> {
         if let Some(dim) = self.get_attrib(dim_symbol()) {
             dim.as_integer_iter()
         } else {
diff --git a/extendr-api/src/robj/rinternals.rs b/extendr-api/src/robj/rinternals.rs
index 0c0a7bf08..ba7476608 100644
--- a/extendr-api/src/robj/rinternals.rs
+++ b/extendr-api/src/robj/rinternals.rs
@@ -111,7 +111,9 @@ impl Robj {
     /// test! {
     ///    let my_fun = base_env().find_function(sym!(ls)).unwrap();
     ///    assert_eq!(my_fun.is_function(), true);
-    ///    assert!(base_env().find_function(sym!(qwertyuiop)).is_none());
+    ///
+    ///    // Note: this may crash on some versions of windows which don't support unwinding.
+    ///    // assert!(base_env().find_function(sym!(qwertyuiop)).is_none());
     /// }
     /// ```
     pub fn find_function<K: Into<Robj>>(&self, key: K) -> Option<Robj> {
@@ -157,7 +159,9 @@ impl Robj {
     ///        .find_var(sym!(iris)).unwrap().eval_promise().unwrap();
     ///    assert_eq!(iris_dataframe.is_frame(), true);
     ///    assert_eq!(iris_dataframe.len(), 5);
-    ///    assert_eq!(global_env().find_var(sym!(imnotasymbol)), None);
+    ///
+    ///    // Note: this may crash on some versions of windows which don't support unwinding.
+    ///    //assert_eq!(global_env().find_var(sym!(imnotasymbol)), None);
     /// }
     /// ```
     pub fn find_var<K: Into<Robj>>(&self, key: K) -> Option<Robj> {
diff --git a/extendr-api/src/thread_safety.rs b/extendr-api/src/thread_safety.rs
index 65a0080b3..ded9a44df 100644
--- a/extendr-api/src/thread_safety.rs
+++ b/extendr-api/src/thread_safety.rs
@@ -96,7 +96,7 @@ pub fn throw_r_error<S: AsRef<str>>(s: S) {
 }
 
 /// Wrap an R function such as Rf_findFunction and convert errors and panics into results.
-/// ```
+/// ```ignore
 /// use extendr_api::prelude::*;
 /// test! {
 ///    let res = catch_r_error(|| unsafe {
@@ -138,6 +138,8 @@ where
         let cleandata = std::mem::transmute(&x);
         let cont = R_MakeUnwindCont();
         Rf_protect(cont);
+
+        // Note that catch_unwind does not work for 32 bit windows targets.
         let res = match std::panic::catch_unwind(|| {
             R_UnwindProtect(fun, data, cleanfun, cleandata, cont)
         }) {
diff --git a/extendr-api/src/wrapper.rs b/extendr-api/src/wrapper.rs
index 8c7ff3cbe..3dce4cfdc 100644
--- a/extendr-api/src/wrapper.rs
+++ b/extendr-api/src/wrapper.rs
@@ -1,10 +1,10 @@
 //! Wrappers are lightweight proxies for references to R datatypes.
 //! They do not contain an Robj (see array.rs for an example of this).
 
+#[doc(hidden)]
 use crate::*;
 #[doc(hidden)]
 use libR_sys::*;
-#[doc(hidden)]
 
 /// Wrapper for creating symbols.
 ///
@@ -195,7 +195,7 @@ where
     T::IntoIter: ExactSizeIterator,
     T::Item: Into<Robj>,
 {
-    /// Make a list object from an array of Robjs.
+    /// Make a list object from an iterator of Robjs.
     /// ```
     /// use extendr_api::prelude::*;
     /// test! {
@@ -214,7 +214,7 @@ where
     T::IntoIter: ExactSizeIterator,
     T::Item: Into<Robj>,
 {
-    /// Make an expression object from an array of Robjs.
+    /// Make an expression object from an iterator of Robjs.
     /// ```
     /// use extendr_api::prelude::*;
     /// test! {


Test patch:
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index 82d0000ee..2c2be3cc1 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -10,6 +10,11 @@ on:
       - main
       - master
 
+# cargo error handling is wrapped by ci-cargo, for simplicity and better logging.
+# To use ci-cargo in a step, source ci-cargo.ps1 (using `. ./ci-cargo.ps`, note the extra dot in front).
+# ci-cargo does not handle correctly -- separator, so wrap it in quotes ('--')
+# A special named parameter called `ActionName` can be used to set the name of current ci-cargo action,
+# used as `ci-cargo .... -ActionName "Called for documentation purposes"`.
 jobs:
   # All tests under this job are run with pre-computed libR-sys bindings.
   tests_no_bindgen:
@@ -113,7 +118,6 @@ jobs:
         run: |
           Del alias:R
           echo "LD_LIBRARY_PATH=$(R -s -e 'cat(normalizePath(R.home()))')/lib" | Out-File -FilePath $env:GITHUB_ENV -Encoding utf8 -Append
-
       # Check code formatting. As this doesn't depend on the platform, do this only on one platform.
       - name: Check code formatting
         if: matrix.config.check_fmt
@@ -123,34 +127,19 @@ jobs:
       # Required by Windows builds, does not affect other platforms
       - name: Build
         run: |
+          . ./ci-cargo.ps1
           foreach($target in ($env:BUILD_TARGETS).Split(',')) {
-            echo "::group::Build for $target target"
-
-            cargo build $(if($target -ne 'default') {"--target=$target"} ) 
-            if (!$?) { 
-              echo "::error::Build for $target target" ;
-              throw "Last exit code $LASTEXITCODE"
-            }
-
-            echo "::endgroup::"
+            ci-cargo build $(if($target -ne 'default') {"--target=$target"} ) -ActionName "Building for $target target"
           }
-
       # For each target in the BUILD_TARGETS comma-separated list, run cargo test with appropriate target
       # Required by Windows builds, does not affect other platforms
+      # ! ci-cargo requires '--' to be wrapped in quotes (passed as an explicit string)
       - name: Run tests
         run: |
+          . ./ci-cargo.ps1
           foreach($target in ($env:BUILD_TARGETS).Split(',')) {
-            echo "::group::Test for $target target"
-            
-            cargo test $(if($target -ne 'default') {"--target=$target"} ) -- --nocapture
-            if (!$?) { 
-              echo "::error::Test for $target target" ;
-              throw "Last exit code $LASTEXITCODE"
-            }
-
-            echo "::endgroup::"
+            ci-cargo test $(if($target -ne 'default') {"--target=$target"} ) '--' --nocapture -ActionName "Testing for $target target"
           }
-
       - name: Query dependencies for integration testing
         run: |
           install.packages('remotes')
@@ -199,7 +188,7 @@ jobs:
   bindgen_no_cross_compile:
     runs-on: ${{ matrix.config.os }}
 
-    name: ${{ matrix.config.os }} w/ bindgen no cross-compile (R-${{ matrix.config.r }} rust-${{ matrix.config.rust-version }})
+    name: ${{ matrix.config.os }} w/ bindgen (R-${{ matrix.config.r }} rust-${{ matrix.config.rust-version }})
             
     strategy:
       fail-fast: false
@@ -223,8 +212,6 @@ jobs:
     steps:
       - uses: actions/checkout@v2
       
-      # Default action allows to install only one toolchain,
-      # so Rust is set up manually.
       # 1. Update rustup
       # 2. For each target add respective toolchain &
       #    update target for that toolchain
@@ -269,7 +256,6 @@ jobs:
           r-version: ${{ matrix.config.r }}
           windows-path-include-mingw: false
       
-
       # 1. Inspect targets. If empty, assume 'x86_64' arch
       # 1.5 Make symlink to please i686 toolchain (https://github.com/rust-lang/cargo/issues/8990)
       # 2. Install msys2 packages (--needed skips already installed)
@@ -318,6 +304,7 @@ jobs:
       # 3.x Run build commands, checking for errors, providing explicit toolchain & target
       - name: Build
         run: |
+          . ./ci-cargo.ps1
           $targets = $env:BUILD_TARGETS.Split(',')
           $libclang_paths = $env:LIBCLANG_PATHS.Split([System.IO.Path]::PathSeparator)
           $itt = 0..($targets.Length - 1)
@@ -337,33 +324,21 @@ jobs:
               $env:LIBCLANG_PATH = $null
             }
 
-            echo "::group::Build extendr-api for $toolchain"
-
-            cargo +$toolchain build --manifest-path extendr-api/Cargo.toml --features tests-all $(if($target -ne 'default') {"--target=$target"} )
-            if (!$?) { 
-              echo "::error::Build extendr-api for $toolchain" 
-              throw "Last exit code $LASTEXITCODE"
-            }
-
-            echo "::endgroup::"
-          
-            echo "::group::Build extendr-engine for $toolchain"
-
-            cargo +$toolchain build --manifest-path extendr-engine/Cargo.toml --features tests-all $(if($target -ne 'default') {"--target=$target"} ) 
-            if (!$?) { 
-              echo "::error::Build extendr-engine for $toolchain"
-              throw "Last exit code $LASTEXITCODE"
-            }
 
-            echo "::endgroup::"
+            ci-cargo +$toolchain build --manifest-path extendr-api/Cargo.toml --features tests-all $(if($target -ne 'default') {"--target=$target"} ) -ActionName "Build extendr-api for $toolchain"
+         
+            ci-cargo +$toolchain build --manifest-path extendr-engine/Cargo.toml --features tests-all $(if($target -ne 'default') {"--target=$target"} ) -ActionName "Build extendr-engine for $toolchain"
+            
           }
         env: 
           BUILD_TARGETS: ${{ join(matrix.config.rust-targets, ',') }}
           RUST_TOOLCHAIN: ${{ matrix.config.rust-version }}
 
       # Identical to the build process, but running more test commands
+      # ! ci-cargo requires '--' to be wrapped in quotes (passed as an explicit string)
       - name: Run tests
         run: |
+          . ./ci-cargo.ps1
           $targets = $env:BUILD_TARGETS.Split(',')
           $libclang_paths = $env:LIBCLANG_PATHS.Split([System.IO.Path]::PathSeparator)
           $itt = 0..($targets.Length - 1)
@@ -383,45 +358,14 @@ jobs:
               $env:LIBCLANG_PATH = $null
             }
 
-            echo "::group::Test extendr-engine \w tests-all for $target target"
-
-            cargo +$toolchain test --manifest-path extendr-engine/Cargo.toml --features tests-all $(if($target -ne 'default') {"--target=$target"} ) -- --nocapture 
-            if (!$?) { 
-              echo "::error::Test extendr-engine \w tests-all for $target target" ;
-              throw "Last exit code $LASTEXITCODE"
-            }
-
-            echo "::endgroup::"
-          
-            echo "::group::Test extendr-api \w tests-all for $target target"
-
-            cargo +$toolchain test --manifest-path extendr-api/Cargo.toml --features tests-all $(if($target -ne 'default') {"--target=$target"} ) -- --nocapture 
-            if (!$?) { 
-              echo "::error::Test extendr-api \w tests-all for $target target" ;
-              throw "Last exit code $LASTEXITCODE"
-            }
-
-            echo "::endgroup::"
-                    
-            echo "::group::Test extendr-api \w tests-minimal for $target target"
-
-            cargo +$toolchain test --manifest-path extendr-api/Cargo.toml --features tests-minimal $(if($target -ne 'default') {"--target=$target"} ) -- --nocapture
-            if (!$?) { 
-              echo "::error::Test extendr-api \w tests-minimal for $target target" ;
-              throw "Last exit code $LASTEXITCODE"
-            }
-
-            echo "::endgroup::"
-          
-            echo "::group::Test extendr-macros for $target target"
+            ci-cargo +$toolchain test --manifest-path extendr-engine/Cargo.toml --features tests-all $(if($target -ne 'default') {"--target=$target"} ) '--' --nocapture -ActionName "Test extendr-engine \w tests-all for $target target"
+            
+            ci-cargo +$toolchain test --manifest-path extendr-api/Cargo.toml --features tests-all $(if($target -ne 'default') {"--target=$target"} ) '--' --nocapture -ActionName "Test extendr-api \w tests-all for $target target"
 
-            cargo +$toolchain test --manifest-path extendr-macros/Cargo.toml  $(if($target -ne 'default') {"--target=$target"} ) -- --nocapture
-            if (!$?) { 
-              echo "::error::Test extendr-macros for $target target" ;
-              throw "Last exit code $LASTEXITCODE"
-            }
+            ci-cargo +$toolchain test --manifest-path extendr-api/Cargo.toml --features tests-minimal $(if($target -ne 'default') {"--target=$target"} ) '--' --nocapture -ActionName "Test extendr-api \w tests-minimal for $target target"
+                      
+            ci-cargo +$toolchain test --manifest-path extendr-macros/Cargo.toml  $(if($target -ne 'default') {"--target=$target"} ) '--' --nocapture -ActionName "Test extendr-macros for $target target"
             
-            echo "::endgroup::"
           }
         env: 
           BUILD_TARGETS: ${{ join(matrix.config.rust-targets, ',') }}
diff --git a/extendr-api/src/robj/tests.rs b/extendr-api/src/robj/tests.rs
index b2872283d..de75734ab 100644
--- a/extendr-api/src/robj/tests.rs
+++ b/extendr-api/src/robj/tests.rs
@@ -227,7 +227,7 @@ fn output_iterator_test() {
 // Test that we can use Iterators as the input to functions.
 // eg.
 // #[extendr]
-// fn fred(a: RealIter, b: RealIter) -> RealIter {
+// fn fred(a: Real, b: Real) -> Robj {
 // }
 #[test]
 fn input_iterator_test() {
@@ -244,18 +244,18 @@ fn input_iterator_test() {
 
         let src: &[i32] = &[1, 2, 3];
         let robj = Robj::from(src);
-        let iter = <IntegerIter>::from_robj(&robj).unwrap();
+        let iter = <Int>::from_robj(&robj).unwrap();
         assert_eq!(iter.collect::<Vec<_>>(), src);
 
         let src: &[f64] = &[1., 2., 3.];
         let robj = Robj::from(src);
-        let iter = <RealIter>::from_robj(&robj).unwrap();
+        let iter = <Real>::from_robj(&robj).unwrap();
         assert_eq!(iter.collect::<Vec<_>>(), src);
 
         /*
         let src: &[Bool] = &[TRUE, FALSE, TRUE];
         let robj = Robj::from(src);
-        let iter = <LogicalIter>::from_robj(&robj).unwrap();
+        let iter = <Logical>::from_robj(&robj).unwrap();
         assert_eq!(iter.collect::<Vec<_>>(), src);
         */
     }