Skip to content

Commit

Permalink
tests: fix BOM issue
Browse files Browse the repository at this point in the history
Basically, the problem here is that the quickcheck tests will sometimes
generate CSV data that begins with a UTF-8 BOM and the CSV reader
automatically strips it.

There's a better way to solve this by making tests more robust, but they
were written without this consideration. As a hack, we just make sure
that we don't generate CSV data that begins with a BOM.

This isn't quite 100% correct since we could still get CSV data
beginning with a UTF-8 BOM through shrinking, but that only occurs when
a legitimate test failure happens. So this could potentially make
failure modes worse, but we abide for now.

Fixes #227
  • Loading branch information
BurntSushi committed Sep 24, 2020
1 parent 1e20138 commit a1165e0
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 4 deletions.
8 changes: 5 additions & 3 deletions tests/test_cat.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use std::process;

use {Csv, CsvData, qcheck};
use quickcheck::TestResult;
use workdir::Workdir;

fn no_headers(cmd: &mut process::Command) {
Expand Down Expand Up @@ -72,7 +73,7 @@ fn cat_rows_headers() {

#[test]
fn prop_cat_cols() {
fn p(rows1: CsvData, rows2: CsvData) -> bool {
fn p(rows1: CsvData, rows2: CsvData) -> TestResult {
let got: Vec<Vec<String>> = run_cat(
"cat_cols", "columns", rows1.clone(), rows2.clone(), no_headers);

Expand All @@ -83,9 +84,10 @@ fn prop_cat_cols() {
r1.extend(r2.into_iter());
expected.push(r1);
}
rassert_eq!(got, expected)
assert_eq!(got, expected);
TestResult::passed()
}
qcheck(p as fn(CsvData, CsvData) -> bool);
qcheck(p as fn(CsvData, CsvData) -> TestResult);
}

#[test]
Expand Down
14 changes: 13 additions & 1 deletion tests/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,10 @@ impl ops::Deref for CsvRecord {
fn deref<'a>(&'a self) -> &'a [String] { &*self.0 }
}

impl ops::DerefMut for CsvRecord {
fn deref_mut<'a>(&'a mut self) -> &'a mut [String] { &mut *self.0 }
}

impl fmt::Debug for CsvRecord {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
let bytes: Vec<_> = self.iter()
Expand Down Expand Up @@ -139,13 +143,21 @@ impl Arbitrary for CsvData {
fn arbitrary<G: Gen>(g: &mut G) -> CsvData {
let record_len = { let s = g.size(); g.gen_range(1, s) };
let num_records: usize = g.gen_range(0, 100);
CsvData{
let mut d = CsvData{
data: (0..num_records).map(|_| {
CsvRecord((0..record_len)
.map(|_| Arbitrary::arbitrary(g))
.collect())
}).collect(),
};
// If the CSV data starts with a BOM, strip it, because it wreaks havoc
// with tests that weren't designed to handle it.
if !d.data.is_empty() && !d.data[0].is_empty() {
if let Some(stripped) = d.data[0][0].strip_prefix("\u{FEFF}") {
d.data[0][0] = stripped.to_string();
}
}
d
}

fn shrink(&self) -> Box<Iterator<Item=CsvData>+'static> {
Expand Down

0 comments on commit a1165e0

Please sign in to comment.