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 ion-tests and start cataloging conformance issues. #911

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
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
2 changes: 1 addition & 1 deletion ion-tests
204 changes: 173 additions & 31 deletions tests/conformance.rs
Original file line number Diff line number Diff line change
@@ -1,52 +1,194 @@
#![cfg(feature = "experimental-ion-1-1")]

use std::collections::HashMap;

use ion_rs::serde::to_string;
use serde::Serialize;

use crate::conformance_dsl::prelude::*;

#[cfg(feature = "experimental-reader-writer")]
mod conformance_dsl;

#[cfg(feature = "experimental-reader-writer")]
pub fn main() {
use crate::conformance_dsl::prelude::*;
#[derive(PartialEq, Serialize)]
enum TestResult {
Ok,
Failed,
Panic,
}

let test_paths = std::env::args().skip(1).collect::<Vec<String>>();
let mut errors: Vec<(String, String, conformance_dsl::ConformanceError)> = vec!();
impl ToString for TestResult {
fn to_string(&self) -> String {
match self {
Self::Ok => "Ok".to_string(),
Self::Failed => "Failed".to_string(),
Self::Panic => "Panic".to_string(),
}
}
}

println!("Testing {} conformance collections.\n", test_paths.len());
#[derive(Serialize)]
struct TestReport {
name: String,
result: TestResult,
error: Option<String>,
}

let mut failures = 0;
/// Implementable trait for providing different output formats for the test results report.
trait TestReporter {
/// Set the current collection. This is generally a single source file containing one or more
/// test documents. This function will be called before any test results are provided.
fn current_collection(&mut self, source: &str, collection: &TestCollection);
/// Add a result for the current collection.
fn add_test_result(&mut self, result: TestReport);
/// Called when all test results have been added for all test collections.
fn finalize(&mut self);
}

for test_path in test_paths {
println!("\nRunning tests: {} ========================", test_path);
let collection = TestCollection::load(&test_path).expect("unable to load test file");
let name_len = collection.iter().fold(0, |acc, d| std::cmp::max(acc, d.name.as_ref().map_or(0, |n| n.len())));
/// A test reporter that renders the results of all tests to the terminal in a human readable way,
/// mimicking the output of cargo test.
#[derive(Default)]
struct PlainReporter {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe PlainTextReporter?

name_width: usize,
current_source: String,
failed_tests: Vec<(String, TestReport)>,
}

for doc in collection.iter() {
match doc.name.as_ref() {
Some(n) => print!(" {:<width$}", n, width = name_len),
None => print!(" {:<width$}", "<unnamed>", width = name_len),
}
impl TestReporter for PlainReporter {
fn current_collection(&mut self, source: &str, collection: &TestCollection) {
println!("\nRunning tests: {}", source);
self.current_source = source.to_owned();
self.name_width = collection.iter().fold(0, |acc, d| {
std::cmp::max(acc, d.name.as_ref().map_or(0, |n| n.len()))
});
Comment on lines +61 to +63
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice 👍

}

print!(" ... ");
match doc.run() {
Err(e) => {
println!("[FAILED]");
failures += 1;
errors.push((test_path.to_owned(), doc.name.as_deref().unwrap_or("<unnamed>").to_owned(), e.clone()));
}
Ok(_) => println!("[OK]"),
fn add_test_result(&mut self, report: TestReport) {
println!(
" {:<width$} ... [{}]",
report.name,
report.result.to_string(),
width = self.name_width
);
if report.result != TestResult::Ok {
self.failed_tests
.push((self.current_source.clone(), report));
}
}

fn finalize(&mut self) {
if self.failed_tests.len() > 0 {
println!(
"============================================================================"
);
for (source, result) in self.failed_tests.iter() {
println!("Source: {}", source);
println!("Test : {}", result.name);
let error = if result.result == TestResult::Panic {
format!("PANIC: {}", result.error.clone().unwrap())
} else {
result.error.clone().unwrap()
};
println!("Error : {}", error);
println!("----------------------------------------------");
}
}
}
}

/// A test reporter that renders the results of all tests into an ion document containing the
/// pass/fail results as well as the failure reason for each test within each test source.
#[derive(Default)]
struct IonReporter {
current_source: String,
results: HashMap<String, Vec<TestReport>>,
}

impl TestReporter for IonReporter {
fn current_collection(&mut self, source: &str, _collection: &TestCollection) {
self.current_source = source.to_string();
self.results.insert(self.current_source.clone(), vec![]);
}

for (test_path, test_name, err) in errors {
println!("-------------------------");
println!("File: {}", test_path);
println!("Test: {}", test_name);
println!("Error: {:?}", err);
fn add_test_result(&mut self, report: TestReport) {
self.results
.get_mut(&self.current_source)
.unwrap()
.push(report);
}

if failures > 0 {
panic!("Conformance test(s) failed");
fn finalize(&mut self) {
println!("{}", to_string(&self.results).unwrap());
}
}

#[cfg(feature = "experimental-reader-writer")]
pub fn main() {
let options = std::env::args()
.skip(1)
.take_while(|a| a.starts_with("-"))
.collect::<Vec<String>>();
let test_paths = std::env::args()
.skip(1 + options.len())
.collect::<Vec<String>>();
let emit_ion = Some("-i") == options.get(0).map(|v| v.as_str());

let mut reporter: Box<dyn TestReporter> = if emit_ion {
Copy link
Contributor

Choose a reason for hiding this comment

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

This may not need to be Boxed? It seems like &mut dyn might work.

Box::new(IonReporter::default())
} else {
Box::new(PlainReporter::default())
};

for test_path in test_paths {
let collection = TestCollection::load(&test_path).expect("unable to load test file");

reporter.current_collection(&test_path, &collection);
for doc in collection.iter() {
let panic_buffer = std::sync::Arc::new(std::sync::Mutex::new(String::new()));
let old_hook = std::panic::take_hook();
// Limit the span that we've hijack'd the panic hook, so that if something goes wrong
// with the unit test outside of our conformance eval, we don't conflate the two.
std::panic::set_hook({
Copy link
Contributor

Choose a reason for hiding this comment

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

It's been so long since I tried to catch a panic that I didn't realize you could set a handler for the current thread. o_O

let mut panic_buffer = panic_buffer.clone();
Box::new(move |info| {
let mut panic_buffer = panic_buffer.lock().unwrap();
// If we have a nice string-ish payload we can just take it.. otherwise we'll
// capture the debug fmt of the info itself.
if let Some(s) = info.payload().downcast_ref::<&str>() {
panic_buffer.push_str(s);
} else {
panic_buffer.push_str(&format!("{:?}", info));
}
})
});
let panic_result = std::panic::catch_unwind(|| doc.run());
std::panic::set_hook(old_hook);

let test_result = if panic_result.is_ok() {
let name = doc.name.as_deref().unwrap_or("<unnamed>").to_owned();
match panic_result.unwrap() {
Err(e) => TestReport {
name,
result: TestResult::Failed,
error: Some(format!("{:?}", e)),
},
Ok(_) => TestReport {
name,
result: TestResult::Ok,
error: None,
},
}
} else {
TestReport {
name: doc.name.as_deref().unwrap_or("<unnamed>").to_owned(),
result: TestResult::Panic,
error: Some((*panic_buffer.lock().unwrap()).to_string()),
}
};
reporter.add_test_result(test_result);
}
}
reporter.finalize();
}

#[cfg(not(feature = "experimental-reader-writer"))]
Expand Down
93 changes: 83 additions & 10 deletions tests/conformance_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,17 +79,90 @@ mod implementation {
mod ion_tests {
use super::*;

#[test_resources("ion-tests/conformance/core/*")]
#[test_resources("ion-tests/conformance/data_model/annotations.ion")]
#[test_resources("ion-tests/conformance/data_model/boolean.ion")]
#[test_resources("ion-tests/conformance/data_model/integer.ion")]
#[test_resources("ion-tests/conformance/data_model/null.ion")]
// No support for half-precision floats yet.
// #[test_resources("ion-tests/conformance/data_model/float.ion")]
type SkipList = &'static [&'static str];
static GLOBAL_CONFORMANCE_SKIPLIST: SkipList = &[
// half-float not implemented
"ion-tests/conformance/data_model/float.ion",
// e-expression transcription
"ion-tests/conformance/eexp/arg_inlining.ion",
"ion-tests/conformance/eexp/element_inlining.ion",
"ion-tests/conformance/eexp/basic_system_macros.ion",
"ion-tests/conformance/ion_encoding/mactab.ion",
"ion-tests/conformance/ion_encoding/module/macro/cardinality/invoke_cardinality_ee.ion",
"ion-tests/conformance/ion_encoding/module/macro/template/literal_form.ion",
"ion-tests/conformance/ion_encoding/module/macro/template/quasiliteral.ion",
"ion-tests/conformance/ion_encoding/module/macro/template/variable_reference.ion",
"ion-tests/conformance/ion_encoding/module/macro/template/if.ion",
"ion-tests/conformance/ion_encoding/module/macro/trivial/literal_value.ion",
"ion-tests/conformance/ion_encoding/module/macro/trivial/invoke_ee.ion",
// Error: Mismatched denotes
"ion-tests/conformance/eexp/binary/tagless_types.ion",
// Error: Unexpected EOF
"ion-tests/conformance/eexp/binary/argument_encoding.ion",
// Error: Mismatched Produce
"ion-tests/conformance/ion_encoding/symtab.ion",
"ion-tests/conformance/ion_encoding/load_symtab.ion",
"ion-tests/conformance/ion_encoding/trivial_forms.ion",
"ion-tests/conformance/ion_encoding/module/trivial.ion",
"ion-tests/conformance/ion_encoding/module/macro_table.ion",
"ion-tests/conformance/system_macros/add_macros.ion",
// Error: found operation name with non-symbol type: sexp
"ion-tests/conformance/ion_encoding/module/load_symtab.ion",
"ion-tests/conformance/ion_encoding/module/symtab.ion",
// Error: Too few arguments.
"ion-tests/conformance/ion_encoding/module/macro/cardinality/invoke_cardinality_tl.ion",
// Error: "Invalid macro name:"
"ion-tests/conformance/ion_encoding/module/macro/trivial/signature.ion",
// Error: ExpectedSignal: No such macro: NoSuchMacro
"ion-tests/conformance/ion_encoding/module/macro/trivial/invoke_tl.ion",
// Error: ExpectedSIgnal: invalid argument
"ion-tests/conformance/system_macros/add_symbols.ion",
"ion-tests/conformance/system_macros/set_macros.ion",
"ion-tests/conformance/system_macros/set_symbols.ion",
// Error: Decoding Error: macro none signature has 0 parameters(s), e-expression had an
// extra argument.
"ion-tests/conformance/system_macros/default.ion",
// System macro delta not yet implemented
"ion-tests/conformance/system_macros/delta.ion",
// System macro make_decimal not yet implemented
"ion-tests/conformance/system_macros/make_decimal.ion",
// System macro repeat not yet implemented
"ion-tests/conformance/system_macros/repeat.ion",
// System macro parse_ion not yet implemented
"ion-tests/conformance/system_macros/parse_ion.ion",
// System macro sum not yet implemented
"ion-tests/conformance/system_macros/sum.ion",
// System macro make_timestamp not yet implemented
"ion-tests/conformance/system_macros/make_timestamp.ion",
// Expected Signal: invalid macro definition
"ion-tests/conformance/tdl/expression_groups.ion",
// Expected Signal Errors and e-expression transcription
"ion-tests/conformance/tdl/if_none.ion",
"ion-tests/conformance/tdl/if_some.ion",
"ion-tests/conformance/tdl/if_multi.ion",
"ion-tests/conformance/tdl/if_single.ion",
];

#[test_resources("ion-tests/conformance/core/*.ion")]
#[test_resources("ion-tests/conformance/data_model/*.ion")]
#[test_resources("ion-tests/conformance/eexp/*.ion")]
#[test_resources("ion-tests/conformance/eexp/binary/*.ion")]
#[test_resources("ion-tests/conformance/ion_encoding/*.ion")]
#[test_resources("ion-tests/conformance/ion_encoding/module/*.ion")]
#[test_resources("ion-tests/conformance/ion_encoding/module/macro/cardinality/*.ion")]
#[test_resources("ion-tests/conformance/ion_encoding/module/macro/template/*.ion")]
#[test_resources("ion-tests/conformance/ion_encoding/module/macro/trivial/*.ion")]
#[test_resources("ion-tests/conformance/system_macros/*.ion")]
#[test_resources("ion-tests/conformance/tdl/*")]
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this macro support ** syntax for nesting at any depth? (Or maybe you're enumerating the subdirectories to leave something out?)

fn conformance(file_name: &str) {
println!("Testing: {}", file_name);
let collection = TestCollection::load(file_name).expect("unable to load test file");
// Test for skip list. Convert windows '\\' separators into '/' to match skiplist.
if !GLOBAL_CONFORMANCE_SKIPLIST.iter().any(|f| *f == file_name.replace("\\", "/")) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This works; here's another place we solved the same issue differently:

// A copy of the `ELEMENT_GLOBAL_SKIP_LIST` in which each file name has been canonicalized for the
// current host machine. This makes it possible to compare names in the list with names of files
// on the host without worrying about differences in (for example) path separators.
static CANONICAL_FILE_NAMES: LazyLock<Vec<String>> = LazyLock::new(|| {
ELEMENT_GLOBAL_SKIP_LIST
.iter()
.filter_map(|filename| {
// Canonicalize the skip list file names so they're in the host OS' preferred format.
// This involves looking up the actual file; if canonicalization fails, the file could
// not be found/read which could mean the skip list is outdated.
fs::canonicalize(filename).ok()
})
.map(|filename| filename.to_string_lossy().into())
.collect()
});

println!("TESTING: {}", file_name);
let collection = TestCollection::load(file_name).expect("unable to load test file");

collection.run().expect("failed to run collection");
collection.run().expect("failed to run collection");
} else {
println!("SKIPPING: {}", file_name);
}
}
}
Loading