Improve test coverage of "adding rows to table collections" (#143):

* Add generic metadata fixture.

Bugs fixed: 

* Add missing Iterator implementations for IndividualTableIterator
  and IndividualTableRefIterator.
* TableCollection::add_migration_* now return Result<MigrationId, TskitError>.
* Fix bug where MigrationTable::node and MigrationTable::source were
  accessing the wrong C arrays.

Author: molpopgen

src/individual_table.rs

@@ -67,6 +67,26 @@ pub type IndividualTableRefIterator<'a> =
     crate::table_iterator::TableIterator<&'a IndividualTable<'a>>;
 pub type IndividualTableIterator<'a> = crate::table_iterator::TableIterator<IndividualTable<'a>>;
 
+impl<'a> Iterator for IndividualTableRefIterator<'a> {
+    type Item = IndividualTableRow;
+
+    fn next(&mut self) -> Option<Self::Item> {
+        let rv = make_individual_table_row(self.table, self.pos);
+        self.pos += 1;
+        rv
+    }
+}
+
+impl<'a> Iterator for IndividualTableIterator<'a> {
+    type Item = IndividualTableRow;
+
+    fn next(&mut self) -> Option<Self::Item> {
+        let rv = make_individual_table_row(&self.table, self.pos);
+        self.pos += 1;
+        rv
+    }
+}
+
 impl<'a> IndividualTable<'a> {
     pub(crate) fn new_from_table(individuals: &'a ll_bindings::tsk_individual_table_t) -> Self {
         IndividualTable {

src/migration_table.rs

@@ -112,7 +112,7 @@ impl<'a> MigrationTable<'a> {
     ///
     /// * [`TskitError::IndexError`] if `row` is out of range.
     pub fn node<M: Into<MigrationId> + Copy>(&'a self, row: M) -> Result<NodeId, TskitError> {
-        unsafe_tsk_column_access!(row.into().0, 0, self.num_rows(), self.table_.source, NodeId)
+        unsafe_tsk_column_access!(row.into().0, 0, self.num_rows(), self.table_.node, NodeId)
     }
 
     /// Return the source population for a given row.
@@ -128,7 +128,7 @@ impl<'a> MigrationTable<'a> {
             row.into().0,
             0,
             self.num_rows(),
-            self.table_.node,
+            self.table_.source,
             PopulationId
         )
     }

src/provenance.rs

@@ -275,3 +275,48 @@ impl<'a> ProvenanceTable<'a> {
         crate::table_iterator::make_table_iterator::<&ProvenanceTable<'a>>(&self)
     }
 }
+
+#[cfg(test)]
+mod test_trigger_provenance_errors {
+    use super::*;
+    use crate::test_fixtures::make_empty_table_collection;
+    use Provenance;
+
+    #[test]
+    #[should_panic]
+    fn test_empty_record_string() {
+        //TODO: decide if we like this behavior:
+        //See GitHub issue 150 -- this behavior should change.
+        let mut tables = make_empty_table_collection(1.0);
+        let row_id = tables.add_provenance(&String::from("")).unwrap();
+        let _ = tables.provenances().row(row_id).unwrap();
+    }
+}
+
+#[cfg(test)]
+mod test_provenance_tables {
+    use super::*;
+    use crate::test_fixtures::make_empty_table_collection;
+    use Provenance;
+
+    #[test]
+    fn test_add_rows() {
+        let records = vec!["banana".to_string(), "split".to_string()];
+        let mut tables = make_empty_table_collection(1.);
+        for (i, r) in records.iter().enumerate() {
+            let row_id = tables.add_provenance(&r).unwrap();
+            assert!(row_id == ProvenanceId(i as crate::tsk_id_t));
+            assert_eq!(tables.provenances().record(row_id).unwrap(), *r);
+        }
+        assert_eq!(tables.provenances().num_rows() as usize, records.len());
+        for (i, row) in tables.provenances_iter().enumerate() {
+            assert_eq!(records[i], row.record);
+        }
+        for (i, row) in tables.provenances().iter().enumerate() {
+            assert_eq!(records[i], row.record);
+        }
+
+        assert!(tables.provenances().row(0).unwrap() == tables.provenances().row(0).unwrap());
+        assert!(tables.provenances().row(0).unwrap() != tables.provenances().row(1).unwrap());
+    }
+}