Adds ion-tests test file integration for Loader.

* Adds `test-generator` create dev dependency to get test per file. * Adds a test suite with `good`/`bad` files with skip lists. - `equivs` and `non-equivs` are also supported with `embedded_document` parsing. * Adds `Loader::load_all` convenience method. * Adds doc test example in `loader` module docs and fixes the iterator example. Additional notes: * Added amazon-ion#216 as a result of non-equivs testing and it definitely exposes a weakness in our unit tests and bug in our `PartialEq` for `...Struct`. * Added amazon-ion/ion-c#235 as these tests run into problems with negative binary integer syntax (but not hex or decimal), indicating a potential issue there. Resolves amazon-ion#214.
almann · Apr 25, 2021 · 78953ab · 78953ab
1 parent f2f8a34
commit 78953ab
Show file tree

Hide file tree

Showing 4 changed files with 265 additions and 2 deletions.
diff --git a/Cargo.toml b/Cargo.toml
@@ -44,6 +44,7 @@ ion-c-sys = { path = "./ion-c-sys", version = "0.4" }
 [dev-dependencies]
 # Used by ion-tests integration
 walkdir = "^2.3"
+test-generator = "0.3"
 
 [profile.release]
 lto = true

diff --git a/src/value/loader.rs b/src/value/loader.rs
@@ -40,6 +40,12 @@ pub trait Loader {
         &'a self,
         data: &'b [u8],
     ) -> IonResult<Box<dyn Iterator<Item = IonResult<OwnedElement>> + 'b>>;
+
+    /// Parses given Ion over a given slice into an [`Vec`] returning an
+    /// [`IonError`](crate::result::IonError) if any error occurs during the parse.
+    fn load_all(&self, data: &[u8]) -> IonResult<Vec<OwnedElement>> {
+        self.iterate_over(data)?.collect()
+    }
 }
 
 struct IonCReaderIterator<'a> {

diff --git a/src/value/mod.rs b/src/value/mod.rs
@@ -14,7 +14,7 @@
 //!   instances.
 //!
 //! ## Examples
-//! In general, users will use the [`loader::Loader`] trait to load in data:
+//! In general, users will use the [`Loader`](loader::Loader) trait to load in data:
 //!
 //! ```
 //! use ion_rs::IonType;
@@ -24,15 +24,40 @@
 //! use ion_rs::value::loader::Loader;
 //!
 //! fn main() -> IonResult<()> {
-//!     let mut iter = loader().iterate_over(br#""hello!"#)?;
+//!     let mut iter = loader().iterate_over(br#""hello!""#)?;
 //!     if let Some(Ok(elem)) = iter.next() {
 //!         assert_eq!(IonType::String, elem.ion_type());
 //!         assert_eq!("hello!", elem.as_str().unwrap());
+//!     } else {
+//!         panic!("Expected an element!");
 //!     }
+//!     assert!(iter.next().is_none());
 //!     Ok(())
 //! }
 //! ```
 //!
+//! [`Loader::load_all`](loader::Loader::load_all) is a convenient way to put all of the
+//! parsed [`Element`] into a [`Vec`], with a single [`IonError`](crate::result::IonError) wrapper:
+//!
+//! ```
+//! # use ion_rs::IonType;
+//! # use ion_rs::result::IonResult;
+//! # use ion_rs::value::{Element, Struct};
+//! # use ion_rs::value::loader::loader;
+//! # use ion_rs::value::loader::Loader;
+//! # fn main() -> IonResult<()> {
+//! #
+//! let elems = loader().load_all(br#""hello" world"#)?;
+//! assert_eq!(2, elems.len());
+//! assert_eq!(IonType::String, elems[0].ion_type());
+//! assert_eq!("hello", elems[0].as_str().unwrap());
+//! assert_eq!(IonType::Symbol, elems[1].ion_type());
+//! assert_eq!("world", elems[1].as_str().unwrap());
+//! #
+//! #    Ok(())
+//! # }
+//! ```
+//!
 //! Users should use the traits in this module to make their code work
 //! in contexts that have either [`borrowed`] or [`owned`] values.  This can be done
 //! most easily by writing generic functions that can work with a reference of any type.

diff --git a/tests/loader_test_vectors.rs b/tests/loader_test_vectors.rs
@@ -0,0 +1,231 @@
+// Copyright Amazon.com, Inc. or its affiliates.
+
+use ion_rs::result::{decoding_error, IonResult};
+use ion_rs::value::loader::{loader, Loader};
+use ion_rs::value::owned::OwnedElement;
+use ion_rs::value::{Element, Sequence, SymbolToken};
+use std::fs::read;
+use std::path::MAIN_SEPARATOR as PATH_SEPARATOR;
+use test_generator::test_resources;
+
+/// Files that should always be skipped for some reason.
+const ALL_SKIP_LIST: &[&str] = &[
+    // ion-c does not currently support these
+    // see: https://github.com/amzn/ion-c/blob/master/test/gather_vectors.cpp
+    "ion-tests/iontestdata/good/utf16.ion",
+    "ion-tests/iontestdata/good/utf32.ion",
+    "ion-tests/iontestdata/good/subfieldVarUInt32bit.ion",
+    "ion-tests/iontestdata/good/typecodes/T6-large.10n",
+    "ion-tests/iontestdata/good/typecodes/T7-large.10n",
+    "ion-tests/iontestdata/good/typecodes/type_6_length_0.10n",
+    // these appear to have a problem specific to how we're calling ion-c and should be investigated
+    "ion-tests/iontestdata/good/equivs/intsLargeNegative1.10n",
+    "ion-tests/iontestdata/good/equivs/intsLargePositive1.10n",
+    "ion-tests/iontestdata/good/intLongMaxValuePlusOne.10n",
+    "ion-tests/iontestdata/good/item1.10n",
+    "ion-tests/iontestdata/good/subfieldVarInt.ion",
+    "ion-tests/iontestdata/good/typecodes/T2.10n",
+    "ion-tests/iontestdata/good/typecodes/T3.10n",
+    // these are symbols with unknown text
+    "ion-tests/iontestdata/good/symbolExplicitZero.10n",
+    "ion-tests/iontestdata/good/symbolImplicitZero.10n",
+    "ion-tests/iontestdata/good/symbolZero.ion",
+    "ion-tests/iontestdata/good/typecodes/T7-small.10n",
+];
+
+/// Files that should only be skipped in equivalence file testing
+const EQUIVS_SKIP_LIST: &[&str] = &[
+    // ion-c seems to have a problem with negative binary literals amzn/ion-c#235
+    "ion-tests/iontestdata/good/equivs/binaryInts.ion",
+    "ion-tests/iontestdata/good/equivs/intsWithUnderscores.ion",
+];
+
+/// Files that should only be skipped in non-equivalence file testing
+const NON_EQUIVS_SKIP_LIST: &[&str] = &[
+    // we need an structural equality (IonEq) for these
+    "ion-tests/iontestdata/good/non-equivs/decimals.ion",
+    "ion-tests/iontestdata/good/non-equivs/floatsVsDecimals.ion",
+    "ion-tests/iontestdata/good/non-equivs/floats.ion",
+    // these have symbols with unknown text
+    "ion-tests/iontestdata/good/non-equivs/symbolTablesUnknownText.ion",
+    // this is a bug in our PartialEq amzn/ion-rust#216
+    "ion-tests/iontestdata/good/non-equivs/structs.ion",
+];
+
+/// Concatenates two slices of string slices together.
+#[inline]
+fn concat<'a>(left: &[&'a str], right: &[&'a str]) -> Vec<&'a str> {
+    left.into_iter()
+        .chain(right.into_iter())
+        .map(|p| *p)
+        .collect()
+}
+
+fn load_file<L: Loader>(loader: &L, file_name: &str) -> IonResult<Vec<OwnedElement>> {
+    // TODO have a better API that doesn't require buffering into memory everything...
+    let data = read(file_name)?;
+    loader.load_all(&data)
+}
+
+fn assert_file<T, F: FnOnce() -> IonResult<T>>(skip_list: &[&str], file_name: &str, asserter: F) {
+    if skip_list
+        .into_iter()
+        // TODO construct the paths in a not so hacky way
+        .map(|p| p.replace("/", &PATH_SEPARATOR.to_string()))
+        .find(|p| p == file_name)
+        .is_some()
+    {
+        println!("IGNORING: {}", file_name);
+    } else {
+        println!("TESTING: {}", file_name);
+        asserter().unwrap();
+    }
+}
+
+#[test_resources("ion-tests/iontestdata/good/**/*.ion")]
+#[test_resources("ion-tests/iontestdata/good/**/*.10n")]
+fn good(file_name: &str) {
+    assert_file(ALL_SKIP_LIST, file_name, || load_file(&loader(), file_name));
+}
+
+#[test_resources("ion-tests/iontestdata/bad/**/*.ion")]
+#[test_resources("ion-tests/iontestdata/bad/**/*.10n")]
+fn bad(file_name: &str) {
+    assert_file(ALL_SKIP_LIST, file_name, || {
+        match load_file(&loader(), file_name) {
+            Ok(items) => panic!("Expected error, got: {:?}", items),
+            Err(_) => Ok(()),
+        }
+    });
+}
+
+/// Parses the elements of a given sequence as text Ion data and tests a grouping on the loaded
+/// documents.
+///
+/// For example, for the given group:
+///
+/// ```plain
+/// embedded_documents::(
+///   "a b c"
+///   "'a' 'b' 'c'"
+/// )
+/// ```
+///
+/// This will parse each string as a [`Vec`] of [`Element`] and apply the `group_assert` function
+/// for every pair of the parsed data including the identity case (a parsed document is
+/// compared against itself).
+fn load_group_embedded<L, S, F>(loader: &L, raw_group: &S, group_assert: &F) -> IonResult<()>
+where
+    L: Loader,
+    S: Sequence,
+    F: Fn(&Vec<OwnedElement>, &Vec<OwnedElement>) -> (),
+{
+    let group_res: IonResult<Vec<_>> = raw_group
+        .iter()
+        .map(|elem| loader.load_all(elem.as_str().unwrap().as_bytes()))
+        .collect();
+    let group = group_res?;
+    for this in group.iter() {
+        for that in group.iter() {
+            group_assert(this, that);
+        }
+    }
+    Ok(())
+}
+
+/// Parses a document that has top-level `list`/`sexp` values that represent a *group*.
+/// If this top-level value is annotated with `embedded_documents`, then [`load_group_embedded`]
+/// is executed for that grouping.  Otherwise, the `value_assert` function is invoked for
+/// every pair of values in a group including the identity case (a value in a group is compared
+/// against itself).
+///
+/// Here is an example with two groups that might be used for equivalence testing:
+///
+/// ```plain
+/// (name $2 'name')
+/// embedded_documents::(
+///   "a name"
+///   "a $2"
+///   "'a' 'name'"
+/// )
+/// ```
+///
+/// This would have two groups, one with direct values that will be compared and another
+/// with embedded Ion text that will be parsed and compared.
+fn load_group<L, F1, F2>(
+    loader: L,
+    file_name: &str,
+    value_assert: F1,
+    group_assert: F2,
+) -> IonResult<()>
+where
+    L: Loader,
+    F1: Fn(&OwnedElement, &OwnedElement) -> (),
+    F2: Fn(&Vec<OwnedElement>, &Vec<OwnedElement>) -> (),
+{
+    let group_lists = load_file(&loader, file_name)?;
+    for group_list in group_lists.iter() {
+        // every grouping set is a list/sexp
+        // look for the embbedded annotation to parse/test as the underlying value
+        let is_embedded = group_list
+            .annotations()
+            .find(|annotation| annotation.text() == Some("embedded_documents"))
+            .is_some();
+        match group_list.as_sequence() {
+            Some(group) => {
+                if is_embedded {
+                    load_group_embedded(&loader, group, &group_assert)?;
+                } else {
+                    for this in group.iter() {
+                        for that in group.iter() {
+                            value_assert(this, that);
+                        }
+                    }
+                }
+            }
+            _ => return decoding_error(format!("Expected a sequence for group: {:?}", group_list)),
+        };
+    }
+    Ok(())
+}
+
+#[test_resources("ion-tests/iontestdata/good/equivs/**/*.ion")]
+#[test_resources("ion-tests/iontestdata/good/equivs/**/*.10n")]
+fn equivs(file_name: &str) {
+    let skip_list = concat(ALL_SKIP_LIST, EQUIVS_SKIP_LIST);
+    assert_file(&skip_list[..], file_name, || {
+        load_group(
+            loader(),
+            file_name,
+            |this, that| assert_eq!(this, that),
+            |this_group, that_group| assert_eq!(this_group, that_group),
+        )
+    });
+}
+
+#[test_resources("ion-tests/iontestdata/good/non-equivs/**/*.ion")]
+// no binary files exist and the macro doesn't like empty globs...
+//#[test_resources("ion-tests/iontestdata/good/non-equivs/**/*.10n")]
+fn non_equivs(file_name: &str) {
+    let skip_list = concat(ALL_SKIP_LIST, NON_EQUIVS_SKIP_LIST);
+    assert_file(&skip_list[..], file_name, || {
+        load_group(
+            loader(),
+            file_name,
+            |this, that| {
+                if std::ptr::eq(this, that) {
+                    assert_eq!(this, that);
+                } else {
+                    assert_ne!(this, that);
+                }
+            },
+            |this_group, that_group| {
+                if std::ptr::eq(this_group, that_group) {
+                    assert_eq!(this_group, that_group);
+                } else {
+                    assert_ne!(this_group, that_group);
+                }
+            },
+        )
+    });
+}