Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update conformance testing DSL to support unknown symbols #833

Merged
merged 4 commits into from
Oct 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 15 additions & 10 deletions src/lazy/binary/raw/v1_1/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,13 +135,7 @@ impl<'top> HasRange for &'top LazyRawBinaryValue_1_1<'top> {

impl<'top> LazyRawValue<'top, BinaryEncoding_1_1> for &'top LazyRawBinaryValue_1_1<'top> {
fn ion_type(&self) -> IonType {
// Handle retrieving the type for a typed null.
if self.encoded_value.header.type_code() == OpcodeType::TypedNull {
let body = self.value_body();
ION_1_1_TYPED_NULL_TYPES[body[0] as usize]
} else {
self.encoded_value.ion_type()
}
self.encoded_value.ion_type()
}

fn is_null(&self) -> bool {
Expand All @@ -167,7 +161,7 @@ impl<'top> LazyRawValue<'top, BinaryEncoding_1_1> for &'top LazyRawBinaryValue_1
}

if self.is_null() {
return Ok(RawValueRef::Null(self.ion_type()));
return Ok(RawValueRef::Null(self.read_null()?));
}

match self.ion_type() {
Expand All @@ -187,6 +181,7 @@ impl<'top> LazyRawValue<'top, BinaryEncoding_1_1> for &'top LazyRawBinaryValue_1
}
}


/// This is a fast path for reading values that we know need to be resolved.
///
/// When a `LazyValue` wrapping a raw binary value calls `read()`, it's clear that the `RawValueRef` will
Expand All @@ -210,7 +205,7 @@ impl<'top> LazyRawValue<'top, BinaryEncoding_1_1> for &'top LazyRawBinaryValue_1
return Ok(ValueRef::Int(int));
}
if self.is_null() {
return Ok(ValueRef::Null(self.ion_type()));
return Ok(ValueRef::Null(self.read_null()?));
}
// Anecdotally, string and integer values are very common in Ion streams. This `match` creates
// an inlineable fast path for them while other types go through the general case impl.
Expand All @@ -234,7 +229,7 @@ impl<'top> LazyRawValue<'top, BinaryEncoding_1_1> for &'top LazyRawBinaryValue_1
}

if value.is_null() {
return Ok(ValueRef::Null(value.ion_type()));
return Ok(ValueRef::Null(value.read_null()?));
}

let value_ref =
Expand Down Expand Up @@ -370,6 +365,16 @@ impl<'top> LazyRawBinaryValue_1_1<'top> {
<&'top Self as LazyRawValue<'top, BinaryEncoding_1_1>>::read(&self)
}

fn read_null(&self) -> IonResult<IonType> {
let ion_type = if self.encoded_value.header.type_code() == OpcodeType::TypedNull {
let body = self.value_body();
ION_1_1_TYPED_NULL_TYPES[body[0] as usize]
} else {
self.encoded_value.ion_type()
};
Ok(ion_type)
}

pub fn is_delimited(&self) -> bool {
self.encoded_value.header.ion_type_code.is_delimited_start()
}
Expand Down
33 changes: 29 additions & 4 deletions tests/conformance_dsl/context.rs
Original file line number Diff line number Diff line change
@@ -1,24 +1,27 @@
use crate::conformance_dsl::*;

use ion_rs::{Element, ElementReader, Sequence, Reader, IonSlice};
use ion_rs::{Catalog, Element, ElementReader, Sequence, Reader, IonSlice, SharedSymbolTable, Symbol, SymbolId};
use ion_rs::{v1_0, v1_1};

/// A Context forms a scope for tracking all of the document fragments and any parent Contexts that
/// also need to be considered. Through this context the ability to generate the full test
/// document input, and evaluate any forced encodings or Ion versions, is provided.
#[derive(Clone, Copy, Debug)]
#[derive(Clone)]
pub(crate) struct Context<'a> {
version: IonVersion,
encoding: IonEncoding,
fragments: &'a Vec<Fragment>,
parent_ctx: Option<&'a Context<'a>>,
symbol_tables: Vec<SharedSymbolTable>, // To build Catalogs when needed, Catalog doesn't
// require Clone.
Comment on lines +15 to +16
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand the comment. Doesn't require Clone for...? Is it something we should change or track?

Should this be a MapCatalog instead so you can clone() it?

}

impl<'a> Context<'a> {
/// Creates a new Context with the provided version, encoding and fragments. A parent context
/// is not set.
pub fn new(version: IonVersion, encoding: IonEncoding, fragments: &'a Vec<Fragment>) -> Self {
Self { version, encoding, fragments, parent_ctx: None}
pub fn new(version: IonVersion, encoding: IonEncoding, fragments: &'a Vec<Fragment>) -> Result<Self> {
let symbol_tables = build_ion_tests_symtables()?;
Ok(Self { version, encoding, fragments, parent_ctx: None, symbol_tables })
}

/// Creates a new Context with the provided fragments, based on the supplied `parent`. The
Expand All @@ -29,6 +32,7 @@ impl<'a> Context<'a> {
encoding: parent.encoding,
parent_ctx: Some(parent),
fragments,
symbol_tables: parent.symbol_tables.clone(),
}
}

Expand Down Expand Up @@ -101,6 +105,26 @@ impl<'a> Context<'a> {
}
}

/// Build all of the symbol tables loaded from ion-tests/catalog into a usuable Catalog.
pub fn build_catalog(&self) -> impl Catalog {
let mut catalog = MapCatalog::new();

for sym_table in self.symbol_tables.iter() {
catalog.insert_table(sym_table.clone());
}

catalog
}

/// Returns the symbol at the provided offset `offset` within the symbol table named `symtab`,
/// if either the symbol table, or the offset, is not valid then None is returned.
pub fn get_symbol_from_table<S: AsRef<str>>(&self, symtab: S, offset: SymbolId) -> Option<Symbol> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pub fn get_symbol_from_table<S: AsRef<str>>(&self, symtab: S, offset: SymbolId) -> Option<Symbol> {
pub fn get_symbol_from_table<S: AsRef<str>>(&self, symtab_name: S, offset: SymbolId) -> Option<Symbol> {

self.symbol_tables.iter()
.filter(|st| st.name() == symtab.as_ref())
.max_by_key(|sst| sst.version())
.and_then(|st| st.symbols().get(offset - 1).cloned())
}

/// Returns a Vec<u8> containing the serialized data consisting of all fragments in the path
/// for this context.
pub fn input(&self, child_encoding: IonEncoding) -> InnerResult<(Vec<u8>, IonEncoding)> {
Expand All @@ -120,6 +144,7 @@ impl<'a> Context<'a> {
let (data, data_encoding) = self.input(encoding)?;
let data_slice = IonSlice::new(data);


if self.fragments.is_empty() {
let empty: Vec<Element> = vec!();
return Ok(empty.into());
Expand Down
107 changes: 77 additions & 30 deletions tests/conformance_dsl/continuation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

use super::*;
use super::context::Context;
use super::model::ModelValue;
use super::model::{compare_values, ModelValue};

use ion_rs::{Element, Sequence};

Expand All @@ -13,10 +13,10 @@ pub(crate) enum Continuation {
// expectations

// Verify that reading the current document produces the expected data provided.
Produces(Vec<Element>),
Produces(Produces),
// Verify that reading the current document produces the expected data, provided through data
// model elements.
Denotes(Vec<ModelValue>),
Denotes(Denotes),
// Verify that reading the current document produces an error.
Signals(String),
// extensions
Expand All @@ -39,18 +39,7 @@ impl Continuation {
pub fn evaluate(&self, ctx: &Context) -> InnerResult<()> {
match self {
// Produces is terminal, so we can evaluate.
Continuation::Produces(expected_elems) => {
let elems = ctx.read_all(ctx.encoding())?;
if expected_elems.len() != elems.len() {
Err(ConformanceErrorKind::MismatchedProduce)
} else {
let zip = expected_elems.iter().zip(elems.iter());
match zip.fold(true, |acc, (x, y)| acc && (*x == *y)) {
true => Ok(()),
false => Err(ConformanceErrorKind::MismatchedProduce),
}
}
}
Continuation::Produces(produces) => produces.evaluate(ctx),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, this was helpful.

Continuation::Not(inner) => match inner.evaluate(ctx) {
Err(_) => Ok(()),
Ok(_) => Err(ConformanceErrorKind::UnknownError),
Expand All @@ -62,18 +51,7 @@ impl Continuation {
Ok(())
}
Continuation::Then(then) => then.evaluate(ctx),
Continuation::Denotes(expected_vals) => {
let elems = ctx.read_all(ctx.encoding())?;
if expected_vals.len() != elems.len() {
Err(ConformanceErrorKind::MismatchedDenotes)
} else {
let zip = expected_vals.iter().zip(elems.iter());
match zip.fold(true, |acc, (x, y)| acc && (*x == *y)) {
true => Ok(()),
false => Err(ConformanceErrorKind::MismatchedDenotes),
}
}
}
Continuation::Denotes(denotes) => denotes.evaluate(ctx),
Continuation::Extensions(exts) => {
for ext in exts {
ext.evaluate(ctx)?;
Expand All @@ -97,19 +75,20 @@ impl Continuation {
}
}
}

}

impl Default for Continuation {
fn default() -> Self {
Continuation::Produces(vec!())
Continuation::Produces(Produces { elems: vec!() })
}
}

/// Parses a clause known to be a continuation into a proper Continuation instance.
pub fn parse_continuation(clause: Clause) -> InnerResult<Continuation> {
let continuation = match clause.tpe {
ClauseType::Produces => {
Continuation::Produces(clause.body.clone())
Continuation::Produces(Produces { elems: clause.body.clone() })
}
ClauseType::And => {
if !clause.body.is_empty() {
Expand Down Expand Up @@ -159,7 +138,7 @@ pub fn parse_continuation(clause: Clause) -> InnerResult<Continuation> {
return Err(ConformanceErrorKind::ExpectedModelValue);
}
}
Continuation::Denotes(values)
Continuation::Denotes(Denotes { model: values })
}
ClauseType::Each => {
let mut parsing_branches = true;
Expand Down Expand Up @@ -230,6 +209,74 @@ pub(crate) struct EachBranch {
fragment: Fragment,
}

/// Represents a Produces clause, used for defining the expected ion values, using ion literals.
#[derive(Clone, Debug, Default)]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could add docs and a reference link to these like you have on the other types?

pub(crate) struct Produces {
pub elems: Vec<Element>,
}

impl Produces {
/// Creates a reader using the provided context, and compares the read values from the input
/// document with the elements specified in the associated Produces clause for equality.
Comment on lines +219 to +220
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very helpful, thanks.

pub fn evaluate(&self, ctx: &Context) -> InnerResult<()> {
use ion_rs::{Decoder, AnyEncoding};
let (input, _encoding) = ctx.input(ctx.encoding())?;
let mut reader = ion_rs::Reader::new(AnyEncoding.with_catalog(ctx.build_catalog()), input)?;

let mut is_equal = true;
let mut elem_iter = self.elems.iter();

while is_equal {
let (actual_value, expected_elem) = (reader.next()?, elem_iter.next());
match (actual_value, expected_elem) {
(None, None) => break,
(Some(actual_value), Some(expected_elem)) => {
is_equal &= super::fragment::ProxyElement(expected_elem, ctx) == actual_value
}
_ => is_equal = false,
}
}

if is_equal {
Ok(())
} else {
Err(ConformanceErrorKind::MismatchedProduce)
}
}
}

/// Represents a Denotes clause, used for defining the expected ion values using the Denotes Value Model.
#[derive(Clone, Debug, Default)]
pub(crate) struct Denotes {
pub model: Vec<ModelValue>,
}

impl Denotes {
pub fn evaluate(&self, ctx: &Context) -> InnerResult<()> {
use ion_rs::{Decoder, AnyEncoding};
let (input, _encoding) = ctx.input(ctx.encoding())?;
let mut reader = ion_rs::Reader::new(AnyEncoding.with_catalog(ctx.build_catalog()), input)?;
let mut elem_iter = self.model.iter();

let mut is_equal = true;
while is_equal {
let (read_value, expected_element) = (reader.next()?, elem_iter.next());
is_equal = match (read_value, expected_element) {
(Some(actual), Some(expected)) =>
is_equal && compare_values(ctx, expected, &actual)?,
(None, None) => break,
_ => false,
}
}

if is_equal {
Ok(())
} else {
Err(ConformanceErrorKind::MismatchedDenotes)
}
}
}

/// Represents a Then clause, it's optional name, the list of fragments, and continuation. A 'Then'
/// acts as almost a sub-document.
#[derive(Clone, Debug, Default)]
Expand Down
2 changes: 1 addition & 1 deletion tests/conformance_dsl/document.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ pub(crate) struct Document {
impl Document {
/// Execute the test by evaluating the document's continuation.
pub fn run(&self) -> Result<()> {
let ctx = Context::new(IonVersion::Unspecified, self.encoding(), &self.fragments);
let ctx = Context::new(IonVersion::Unspecified, self.encoding(), &self.fragments)?;
self.continuation.evaluate(&ctx)?;
Ok(())
}
Expand Down
Loading
Loading