csv: fix record equality, redux

This commit fixes a bug in the fix for #138, where it was possible for
two records of different lengths to compare as equal. We fix that and also
re-arrange the regression tests to be more consistent with how other
tests are structured.

Thanks to @niklasf for reporting this:
efc4a51#r31692187

src/byte_record.rs

@@ -109,6 +109,9 @@ pub struct ByteRecord(Box<ByteRecordInner>);
 
 impl PartialEq for ByteRecord {
  fn eq(&self, other: &ByteRecord) -> bool {
+ if self.len() != other.len() {
+ return false;
+ }
  self.iter().zip(other.iter()).all(|e| e.0 == e.1)
  }
 }
@@ -1144,4 +1147,25 @@ mod tests {
  assert_eq!(it.next_back(), None);
  assert_eq!(it.next(), None);
  }
+
+ // Check that record equality respects field boundaries.
+ //
+ // Regression test for #138.
+ #[test]
+ fn eq_field_boundaries() {
+ let test1 = ByteRecord::from(vec!["12","34"]);
+ let test2 = ByteRecord::from(vec!["123","4"]);
+
+ assert_ne!(test1, test2);
+ }
+
+ // Check that record equality respects number of fields.
+ //
+ // Regression test for #138.
+ #[test]
+ fn eq_record_len() {
+ let test1 = ByteRecord::from(vec!["12","34", "56"]);
+ let test2 = ByteRecord::from(vec!["12","34"]);
+ assert_ne!(test1, test2);
+ }
 }

src/string_record.rs

@@ -74,7 +74,7 @@ pub struct StringRecord(ByteRecord);
 
 impl PartialEq for StringRecord {
  fn eq(&self, other: &StringRecord) -> bool {
- self.iter().zip(other.iter()).all(|e| e.0 == e.1)
+ byte_record::eq(&self.0, &other.0)
  }
 }
 
@@ -809,4 +809,25 @@ mod tests {
  rec.trim();
  assert_eq!(rec.get(0), Some(""));
  }
+
+ // Check that record equality respects field boundaries.
+ //
+ // Regression test for #138.
+ #[test]
+ fn eq_field_boundaries() {
+ let test1 = StringRecord::from(vec!["12","34"]);
+ let test2 = StringRecord::from(vec!["123","4"]);
+
+ assert_ne!(test1, test2);
+ }
+
+ // Check that record equality respects number of fields.
+ //
+ // Regression test for #138.
+ #[test]
+ fn eq_record_len() {
+ let test1 = StringRecord::from(vec!["12","34", "56"]);
+ let test2 = StringRecord::from(vec!["12","34"]);
+ assert_ne!(test1, test2);
+ }
 }

tests/tests.rs

@@ -2,9 +2,6 @@
 
 extern crate csv;
 
-use csv::ByteRecord;
-use csv::StringRecord;
-
 use std::env;
 use std::io::{self, Write};
 use std::path::PathBuf;
@@ -338,24 +335,6 @@ fn tutorial_perf_core_01() {
  assert_eq!(out.stdout(), "11\n");
 }
 
-///Check for correctness of ```impl PartialEq for ByteRecord``` (Issue #138)
-#[test]
-fn byte_record_eq_field_boundaries() {
- let test1 = ByteRecord::from(vec!["12","34"]);
- let test2 = ByteRecord::from(vec!["123","4"]);
-
- assert_ne!(test1, test2);
-}
-
-///Check for correctness of ```impl PartialEq for StringRecord``` (Issue #138)
-#[test]
-fn string_record_eq_field_boundaries() {
- let test1 = StringRecord::from(vec!["12","34"]);
- let test2 = StringRecord::from(vec!["123","4"]);
-
- assert_ne!(test1, test2);
-}
-
 // Helper functions follow.
 
 /// Return the target/debug directory path.