Repository: extendr/extendr
Pull request #272 (2021-08-30 10:40:11)

Use single_threaded and panic hook in failing tests.
The test "tests_with_unsuccessful_outcomes" has been failing
occasionally, perhaps because of thread interference.

This PR puts a single_threaded wrapper around the test and installs
a null panic hook to suppress the backtrace and panic message.


Patch:
diff --git a/extendr-api/src/error.rs b/extendr-api/src/error.rs
index 56668b809..553a9c96e 100644
--- a/extendr-api/src/error.rs
+++ b/extendr-api/src/error.rs
@@ -42,7 +42,7 @@ pub enum Error {
     ExpectedLanguage(Robj),
     ExpectedSpecial(Robj),
     ExpectedBuiltin(Robj),
-    ExpectedCharacter(Robj),
+    ExpectedRstr(Robj),
     ExpectedLogical(Robj),
     ExpectedInteger(Robj),
     ExpectedReal(Robj),
@@ -99,8 +99,8 @@ impl std::fmt::Display for Error {
             Error::ExpectedLanguage(robj) => write!(f, "Expected Language got {:?}", robj.rtype()),
             Error::ExpectedSpecial(robj) => write!(f, "Expected Special got {:?}", robj.rtype()),
             Error::ExpectedBuiltin(robj) => write!(f, "Expected Builtin got {:?}", robj.rtype()),
-            Error::ExpectedCharacter(robj) => {
-                write!(f, "Expected Character got {:?}", robj.rtype())
+            Error::ExpectedRstr(robj) => {
+                write!(f, "Expected Rstr got {:?}", robj.rtype())
             }
             Error::ExpectedLogical(robj) => write!(f, "Expected Logical got {:?}", robj.rtype()),
             Error::ExpectedInteger(robj) => write!(f, "Expected Integer got {:?}", robj.rtype()),
diff --git a/extendr-api/src/lib.rs b/extendr-api/src/lib.rs
index c147fc32d..8c56f5093 100644
--- a/extendr-api/src/lib.rs
+++ b/extendr-api/src/lib.rs
@@ -406,7 +406,7 @@ pub enum RType {
     Language,    // LANGSXP
     Special,     // SPECIALSXP
     Builtin,     // BUILTINSXP
-    Character,   // CHARSXP
+    Rstr,        // CHARSXP
     Logical,     // LGLSXP
     Integer,     // INTSXP
     Real,        // REALSXP
@@ -438,7 +438,7 @@ pub fn rtype_to_sxp(rtype: RType) -> i32 {
         Language => LANGSXP,
         Special => SPECIALSXP,
         Builtin => BUILTINSXP,
-        Character => CHARSXP,
+        Rstr => CHARSXP,
         Logical => LGLSXP,
         Integer => INTSXP,
         Real => REALSXP,
@@ -470,7 +470,7 @@ pub fn sxp_to_rtype(sxptype: i32) -> RType {
         LANGSXP => Language,
         SPECIALSXP => Special,
         BUILTINSXP => Builtin,
-        CHARSXP => Character,
+        CHARSXP => Rstr,
         LGLSXP => Logical,
         INTSXP => Integer,
         REALSXP => Real,
diff --git a/extendr-api/src/prelude.rs b/extendr-api/src/prelude.rs
index 593feb247..cb05daff3 100644
--- a/extendr-api/src/prelude.rs
+++ b/extendr-api/src/prelude.rs
@@ -51,8 +51,8 @@ pub use super::thread_safety::{
 };
 
 pub use super::wrapper::{
-    Character, EnvIter, Environment, Expression, FromList, Function, Language, List, ListIter,
-    Nullable, Pairlist, Primitive, Promise, Raw, Symbol,
+    EnvIter, Environment, Expression, FromList, Function, Language, List, ListIter, Nullable,
+    Pairlist, Primitive, Promise, Raw, Rstr, Symbol,
 };
 
 #[cfg(feature = "ndarray")]
diff --git a/extendr-api/src/robj/mod.rs b/extendr-api/src/robj/mod.rs
index f04b858ab..0d1c1b7a2 100644
--- a/extendr-api/src/robj/mod.rs
+++ b/extendr-api/src/robj/mod.rs
@@ -172,7 +172,7 @@ impl Robj {
     ///     assert_eq!(lang!("+", 1, 2).rtype(), RType::Language);
     ///     assert_eq!(r!(Primitive::from_string("if")).rtype(), RType::Special);
     ///     assert_eq!(r!(Primitive::from_string("+")).rtype(), RType::Builtin);
-    ///     assert_eq!(r!(Character::from_string("hello")).rtype(), RType::Character);
+    ///     assert_eq!(r!(Rstr::from_string("hello")).rtype(), RType::Rstr);
     ///     assert_eq!(r!(TRUE).rtype(), RType::Logical);
     ///     assert_eq!(r!(1).rtype(), RType::Integer);
     ///     assert_eq!(r!(1.0).rtype(), RType::Real);
@@ -193,7 +193,7 @@ impl Robj {
             LANGSXP => RType::Language,
             SPECIALSXP => RType::Special,
             BUILTINSXP => RType::Builtin,
-            CHARSXP => RType::Character,
+            CHARSXP => RType::Rstr,
             LGLSXP => RType::Logical,
             INTSXP => RType::Integer,
             REALSXP => RType::Real,
@@ -1024,8 +1024,8 @@ impl std::fmt::Debug for Robj {
             SPECIALSXP => write!(f, "r!(Special())"),
             BUILTINSXP => write!(f, "r!(Builtin())"),
             CHARSXP => {
-                let c = Character::try_from(self.clone()).unwrap();
-                write!(f, "r!(Character::from_string({:?}))", c.as_str())
+                let c = Rstr::try_from(self.clone()).unwrap();
+                write!(f, "r!(Rstr::from_string({:?}))", c.as_str())
             }
             LGLSXP => {
                 let slice = self.as_logical_slice().unwrap();
diff --git a/extendr-api/src/robj/rinternals.rs b/extendr-api/src/robj/rinternals.rs
index f42821114..10a1f5aa3 100644
--- a/extendr-api/src/robj/rinternals.rs
+++ b/extendr-api/src/robj/rinternals.rs
@@ -71,7 +71,7 @@ impl Robj {
     }
 
     /// Convert to a string vector.
-    pub fn as_char(&self) -> Robj {
+    pub fn as_character_vector(&self) -> Robj {
         unsafe { new_owned(Rf_asChar(self.get())) }
     }
 
@@ -386,8 +386,8 @@ impl Robj {
     }
 
     /// Return true if this is CHARSXP.
-    pub fn is_character(&self) -> bool {
-        self.rtype() == RType::Character
+    pub fn is_char(&self) -> bool {
+        self.rtype() == RType::Rstr
     }
 
     /// Check an external pointer tag.
diff --git a/extendr-api/src/wrapper/character.rs b/extendr-api/src/wrapper/char.rs
similarity index 77%
rename from extendr-api/src/wrapper/character.rs
rename to extendr-api/src/wrapper/char.rs
index c075e1b2b..5c9fc7570 100644
--- a/extendr-api/src/wrapper/character.rs
+++ b/extendr-api/src/wrapper/char.rs
@@ -1,27 +1,27 @@
 use super::*;
 
-/// Wrapper for creating character objects.
+/// Wrapper for creating CHARSXP objects.
 /// These are used only as the contents of a character
 /// vector.
 ///
 /// ```
 /// use extendr_api::prelude::*;
 /// test! {
-///     let chr = r!(Character::from_string("xyz"));
-///     assert_eq!(chr.as_character().unwrap().as_str(), "xyz");
+///     let chr = r!(Rstr::from_string("xyz"));
+///     assert_eq!(chr.as_char().unwrap().as_str(), "xyz");
 /// }
 /// ```
 ///
 #[derive(Debug, PartialEq, Clone)]
-pub struct Character {
+pub struct Rstr {
     pub(crate) robj: Robj,
 }
 
-impl Character {
+impl Rstr {
     /// Make a character object from a string.
     pub fn from_string(val: &str) -> Self {
         unsafe {
-            Character {
+            Rstr {
                 robj: new_owned(str_to_character(val)),
             }
         }
diff --git a/extendr-api/src/wrapper/mod.rs b/extendr-api/src/wrapper/mod.rs
index 02720c185..fdfecbe65 100644
--- a/extendr-api/src/wrapper/mod.rs
+++ b/extendr-api/src/wrapper/mod.rs
@@ -5,7 +5,7 @@ use crate::*;
 use libR_sys::*;
 
 pub mod altrep;
-pub mod character;
+pub mod char;
 pub mod environment;
 pub mod expr;
 pub mod function;
@@ -20,11 +20,11 @@ pub mod raw;
 pub mod s4;
 pub mod symbol;
 
+pub use self::char::Rstr;
 pub use altrep::{
     AltComplexImpl, AltIntegerImpl, AltLogicalImpl, AltRawImpl, AltRealImpl, AltStringImpl, Altrep,
     AltrepImpl,
 };
-pub use character::Character;
 pub use environment::{EnvIter, Environment};
 pub use expr::Expression;
 pub use function::Function;
@@ -117,12 +117,7 @@ make_conversions!(
 
 make_conversions!(Raw, ExpectedRaw, is_raw, "Not a raw object");
 
-make_conversions!(
-    Character,
-    ExpectedCharacter,
-    is_character,
-    "Not a character object"
-);
+make_conversions!(Rstr, ExpectedRstr, is_char, "Not a character object");
 
 make_conversions!(
     Environment,
@@ -175,19 +170,19 @@ impl Robj {
         Symbol::try_from(self.clone()).ok()
     }
 
-    /// Convert a character object to a Character wrapper.
+    /// Convert a CHARSXP object to a Rstr wrapper.
     /// ```
     /// use extendr_api::prelude::*;
     /// test! {
-    ///     let fred = r!(Character::from_string("fred"));
-    ///     assert_eq!(fred.as_character(), Some(Character::from_string("fred")));
+    ///     let fred = r!(Rstr::from_string("fred"));
+    ///     assert_eq!(fred.as_char(), Some(Rstr::from_string("fred")));
     /// }
     /// ```
-    pub fn as_character(&self) -> Option<Character> {
-        Character::try_from(self.clone()).ok()
+    pub fn as_char(&self) -> Option<Rstr> {
+        Rstr::try_from(self.clone()).ok()
     }
 
-    /// Convert a raw object to a Character wrapper.
+    /// Convert a raw object to a Rstr wrapper.
     /// ```
     /// use extendr_api::prelude::*;
     /// test! {


Test patch:
diff --git a/extendr-api/src/robj/tests.rs b/extendr-api/src/robj/tests.rs
index 919127876..19df5e5d3 100644
--- a/extendr-api/src/robj/tests.rs
+++ b/extendr-api/src/robj/tests.rs
@@ -22,7 +22,7 @@ fn test_debug() {
 
         // Wrappers
         assert_eq!(format!("{:?}", r!(Symbol::from_string("x"))), "sym!(x)");
-        assert_eq!(format!("{:?}", r!(Character::from_string("x"))), "r!(Character::from_string(\"x\"))");
+        assert_eq!(format!("{:?}", r!(Rstr::from_string("x"))), "r!(Rstr::from_string(\"x\"))");
         assert_eq!(
             format!("{:?}", r!(Language::from_values(&[r!(Symbol::from_string("x"))]))),
             "r!(Language::from_values([sym!(x)]))"
diff --git a/extendr-api/tests/extendr_macro.rs b/extendr-api/tests/extendr_macro.rs
index 5e3bc8b58..e40cf5cca 100644
--- a/extendr-api/tests/extendr_macro.rs
+++ b/extendr-api/tests/extendr_macro.rs
@@ -93,8 +93,15 @@ fn tests_with_successful_outcomes() {
 #[cfg(not(target_arch = "x86"))]
 #[test]
 fn tests_with_unsuccessful_outcomes() {
-    unsafe {
+    // Using [single_threaded] here may help with sporadic test failures.
+    single_threaded(|| unsafe {
         test! {
+            let old_hook = std::panic::take_hook();
+
+            // Suppress backtrace with a custom hook.
+            std::panic::set_hook(Box::new(|_| {
+            }));
+
             // These should throw R errors.
             // They may cause stack traces, but this is harmless.
             assert!(catch_r_error(|| wrap__test_i32(r!("xyz").get())).is_err());
@@ -103,8 +110,9 @@ fn tests_with_unsuccessful_outcomes() {
 
             // TODO: check for overflow.
             // assert!(catch_r_error(|| wrap__test_i16(r!(1234567890).get())).is_err());
+            std::panic::set_hook(old_hook);
         }
-    }
+    });
 }
 
 #[test]