TestUtils: add copy/move operation test functions

Testing move and copy operations are something we do all the time and
it's not always trivial. It includes a lot of boiler plate code. These
two template functions take away the boiler plate and test the following
aspects of the copy and move operations:

* copy and move construction
* copy and move assignment
* check that self assignment doesn't destroy the object

The copy version returns all three copies made to test the functionality.

The move version returns the final moved to object. Note that the move
function will move from the parameter passed in, so that parameter
becomes unusable.

fixes #206

gamgee/fastq.h

@@ -27,19 +27,34 @@ class Fastq {
       m_name {name}, m_comment {comment}, m_sequence{sequence}, m_quals{quals} 
   {}
 
+  Fastq(const Fastq&) = default;
+  Fastq& operator=(const Fastq&) = default;
+  Fastq(Fastq&&) = default;
+  Fastq& operator=(Fastq&&) = default;
+
 
   /**
     * @brief inequality comparison of all fields in the record
     *
+    * @return true if any field differs (string comparison)
+    */
+  bool operator!=(const Fastq& other) const {
+      return !(*this == other); 
+  }
+
+  /**
+    * @brief equality comparison of all fields in the record
+    *
     * @return true only if every field is the same (string comparison)
     */
-  bool operator!=(const Fastq& other) {
+  bool operator==(const Fastq& other) const {
       return m_name     == other.m_name     &&
              m_comment  == other.m_comment  &&
              m_sequence == other.m_sequence &&
              m_quals    == other.m_quals;
   }
 
+
   std::string name() const                       { return m_name;         }
   std::string comment() const                    { return m_comment;      }
   std::string sequence() const                   { return m_sequence;     }

gamgee/indexed_sam_reader.h

@@ -70,17 +70,19 @@ class IndexedSamReader {
      * @copydoc IndexedSamReader(IndexedSamReader&&)
      */
     IndexedSamReader& operator=(IndexedSamReader&& other) {
-      m_sam_file_ptr = std::move(other.m_sam_file_ptr);
-      m_sam_index_ptr = std::move(other.m_sam_index_ptr);
-      m_sam_header_ptr = std::move(other.m_sam_header_ptr);
-      m_interval_list = std::move(other.m_interval_list);
-      other.m_sam_index_ptr = nullptr;
-      other.m_sam_file_ptr = nullptr;
+      if (this != &other) {
+        m_sam_file_ptr = std::move(other.m_sam_file_ptr);
+        m_sam_index_ptr = std::move(other.m_sam_index_ptr);
+        m_sam_header_ptr = std::move(other.m_sam_header_ptr);
+        m_interval_list = std::move(other.m_interval_list);
+        other.m_sam_index_ptr = nullptr;
+        other.m_sam_file_ptr = nullptr;
+      }
       return *this;
     }
 
     /**
-     * @brief no copy construction/assignment allowed for iterators and readers
+     * @brief no copy construction/assignment allowed for input iterators and readers
      */
     IndexedSamReader(IndexedSamReader& other) = delete;
 
@@ -94,7 +96,7 @@ class IndexedSamReader {
      */
     ~IndexedSamReader() {
       if (m_sam_index_ptr)
-        hts_idx_destroy(m_sam_index_ptr); // TODO: Copied from samtools. Is there a {bam,sam}_idx_destroy alias? Didn't find under "BAM/CRAM indexing" in sam.h
+        hts_idx_destroy(m_sam_index_ptr); 
       if (m_sam_file_ptr)
         sam_close(m_sam_file_ptr);
     }

test/fastq_reader_test.cpp

@@ -1,7 +1,8 @@
-#include "../gamgee/fastq_reader.h"
-
 #include <boost/test/unit_test.hpp>
 
+#include "fastq_reader.h"
+#include "test_utils.h"
+
 using namespace std;
 using namespace gamgee;
 
@@ -45,3 +46,22 @@ BOOST_AUTO_TEST_CASE( read_fastq_vector_too_large )
 {
   BOOST_CHECK_THROW(vector_too_large("testdata/complete_same_seq.fa"), std::runtime_error);
 }
+
+BOOST_AUTO_TEST_CASE( fastq_copy_and_move_constructor ) {
+  auto it = FastqReader{"testdata/complete_same_seq.fa"}.begin();
+  auto c0 = *it;
+  auto copies = check_copy_constructor(c0);
+  auto c1 = get<0>(copies);
+  auto c2 = get<1>(copies);
+  auto c3 = get<2>(copies);
+  BOOST_CHECK(c0 == c1);
+  BOOST_CHECK(c0 == c2);
+  BOOST_CHECK(c0 == c3);
+  c1.set_name("modified");
+  BOOST_CHECK(c1 != c0);
+  BOOST_CHECK(c1 != c2);
+  auto m0 = *it;
+  auto m1 = check_move_constructor(m0);
+  auto m2 = *it;
+  BOOST_CHECK(m1 == m2);
+}

test/indexed_sam_reader_test.cpp

@@ -1,6 +1,8 @@
+#include <boost/test/unit_test.hpp>
+
 #include "indexed_sam_reader.h"
+#include "test_utils.h"
 
-#include <boost/test/unit_test.hpp>
 
 using namespace std;
 using namespace gamgee;
@@ -77,12 +79,18 @@ BOOST_AUTO_TEST_CASE( indexed_single_readers_empty )
 }
 
 BOOST_AUTO_TEST_CASE( indexed_single_readers_move_constructor_and_assignment ) {
+  auto r0 = IndexedSingleSamReader{"testdata/test_simple.bam", vector<string>{"."}};
+  auto m1 = check_move_constructor(r0);
+  auto m2 = IndexedSingleSamReader{"testdata/test_simple.bam", vector<string>{"."}};
+  BOOST_CHECK_EQUAL((*(m1.begin())).alignment_start(), (*(m2.begin())).alignment_start());
+}
+
+BOOST_AUTO_TEST_CASE( indexed_single_readers_begin_always_restarts ) {
   auto reader1 = IndexedSingleSamReader{"testdata/test_simple.bam", vector<string>{"."}};
   auto it1 = reader1.begin();
-  auto reader2 = std::move(reader1); // check move constructor
-  reader1 = IndexedSingleSamReader{"testdata/test_simple.bam", vector<string>{"."}}; // check move assignment
-  auto it2 = reader2.begin();
-  auto it3 = reader1.begin();
-  BOOST_CHECK_EQUAL((*it1).alignment_start(), (*it2).alignment_start());    // unlike the SingleSamReader, these should be the same because they are pointing at the exact same restarted iterator
-  BOOST_CHECK_EQUAL((*it1).alignment_start(), (*it3).alignment_start()); // these should be the same! both pointing at the first record
+  ++it1;
+  auto it2 = reader1.begin();
+  BOOST_CHECK_NE((*it1).alignment_start(), (*it2).alignment_start());
+  ++it2;
+  BOOST_CHECK_EQUAL((*it1).alignment_start(), (*it2).alignment_start());
 }

test/interval_test.cpp

@@ -1,7 +1,8 @@
-#include "interval.h"
-
 #include <boost/test/unit_test.hpp>
 
+#include "interval.h"
+#include "test_utils.h"
+
 #include <iostream>
 #include <vector>
 #include <string>
@@ -119,3 +120,19 @@ BOOST_AUTO_TEST_CASE( interval_equality )
   i.set_chr("TAST");
   BOOST_CHECK(!(i == j));
 }
+
+BOOST_AUTO_TEST_CASE( interval_copy_and_move_constructors ) {
+  auto i0 = Interval {"A", 1'000, 2'000};
+  auto copies = check_copy_constructor(i0);
+  auto c2 = get<1>(copies);
+  BOOST_CHECK(i0 ==  get<0>(copies));
+  BOOST_CHECK(i0 == c2);
+  BOOST_CHECK(i0 == get<2>(copies));
+  c2.set_start(1'500);
+  BOOST_CHECK(i0 != c2);
+  BOOST_CHECK(c2 != get<0>(copies));
+  BOOST_CHECK(c2 != get<1>(copies));
+  BOOST_CHECK(c2 != get<2>(copies)); 
+  auto m1 = check_move_constructor(get<0>(copies));
+  BOOST_CHECK(i0 == m1);
+}

test/sam_header_test.cpp

@@ -1,7 +1,8 @@
-#include "sam_reader.h"
-
 #include <boost/test/unit_test.hpp>
 
+#include "sam_reader.h"
+#include "test_utils.h"
+
 using namespace std;
 using namespace gamgee;
 
@@ -18,11 +19,8 @@ BOOST_AUTO_TEST_CASE( sam_header ) {
 /** @todo Need a way to modify the header in between these copies/moves to make sure these are working properly! */
 BOOST_AUTO_TEST_CASE( sam_header_constructors ) {
   auto reader = SingleSamReader{"testdata/test_simple.bam"};
-  auto header1 = reader.header();
-  auto header2 = header1;                  // copy constructor
-  auto header3 = std::move(header1);       // move constructor
-  header1 = header2;                       // copy assignment
-  const auto header4 = std::move(header1); // transfer header1's memory somewhere else so we can reuse it for move assignment
-  header1 = std::move(header3);            // move assignment
-  header1 = header1;                       // self assignment
+  auto h0 = reader.header();
+  auto copies = check_copy_constructor(h0);
+  auto moves = check_copy_constructor(h0);
+  // need builder to be able to modify the header and check. At least this test will blow up if something is not functional.
 }

test/sam_test.cpp

@@ -4,6 +4,9 @@
 #include "sam_reader.h"
 #include "sam_builder.h"
 #include "missing.h"
+
+#include "test_utils.h"
+
 #include <vector>
 
 using namespace std;
@@ -177,25 +180,26 @@ BOOST_AUTO_TEST_CASE( sam_in_place_cigar_modification ) {
   BOOST_CHECK_EQUAL(read_cigar[0], Cigar::make_cigar_element(30, CigarOperator::I));
 }
 
-BOOST_AUTO_TEST_CASE( sam_cigar_copy_and_move_constructors ) {
-  auto read = *(SingleSamReader{"testdata/test_simple.bam"}.begin());
-  auto cigar = read.cigar();
-  auto cigar_copy = cigar;
-  cigar_copy[0] = Cigar::make_cigar_element(3, CigarOperator::M); 
-  BOOST_CHECK(cigar_copy != cigar);                               // check that modifying the copy doesn't affect the original
-  auto cigar_move = std::move(cigar);                             // move construct a new cigar
-  cigar_move[0] = Cigar::make_cigar_element(1, CigarOperator::D);
-  BOOST_CHECK(cigar_move != cigar_copy);                          // check that modifying the moved one doesn't affect the copy
-  cigar = cigar_copy;                                             // check the copy assignment now that cigar has been moved to move_cigar
-  BOOST_CHECK(cigar == cigar_copy);                               // check that the cigar is now the same as the cigar_copy
-  BOOST_CHECK(cigar != cigar_move);                               // check that the cigar is not the same as the moved one
-  cigar[0] = Cigar::make_cigar_element(2, CigarOperator::N);
-  BOOST_CHECK(cigar != cigar_copy);                               // check that modifying the copied version doesn't affect the original
-  auto cigar_tmp = std::move(cigar);                              // take ownership of cigar's memory so we can play around with cigar again
-  cigar = std::move(cigar_move);                                  // we should be back to the original again!
-  BOOST_CHECK(cigar == read.cigar());
-  cigar = cigar;                                                  // check self assignment
-  BOOST_CHECK(cigar == read.cigar());
+BOOST_AUTO_TEST_CASE( sam_cigar_templated_copy_and_move_constructors ) {
+  auto it = SingleSamReader{"testdata/test_simple.bam"}.begin();
+  const auto c0 = (*it).cigar();
+  const auto copies = check_copy_constructor(c0);
+  auto c1 = get<0>(copies);
+  auto c2 = get<1>(copies);
+  auto c3 = get<2>(copies);
+  BOOST_CHECK(c0 == c1); 
+  BOOST_CHECK(c0 == c2);
+  BOOST_CHECK(c0 == c3);
+  c1[0] = Cigar::make_cigar_element(30, CigarOperator::I);
+  BOOST_CHECK(c0 != c1);
+  auto m0 = (*it).cigar();
+  auto m1 = check_move_constructor(m0);
+  auto m2 = (*it).cigar();
+  BOOST_CHECK(m1 == m2);
+  m1[0] = Cigar::make_cigar_element(20, CigarOperator::I);
+  BOOST_CHECK(m1 == m2);  // the underlying object is the same and it still exists 
+  BOOST_CHECK(m0 == m1);  // the underlying object is the same and it still exists (hasn't been destroyed, so they must still match)
+  BOOST_CHECK(m1 != c1);  // check that modifying the moved doesn't affect the copied
 }
 
 BOOST_AUTO_TEST_CASE( invalid_cigar_access ) {
@@ -217,25 +221,26 @@ BOOST_AUTO_TEST_CASE( comparing_different_cigars ) {
   BOOST_CHECK(read1.cigar() != read2.cigar());
 }
 
-BOOST_AUTO_TEST_CASE( sam_base_quals_copy_and_move_constructors ) {
-  auto read = *(SingleSamReader{"testdata/test_simple.bam"}.begin());
-  auto bq = read.base_quals();
-  auto bq_copy = bq;
-  bq_copy[0] = 99;
-  BOOST_CHECK(bq_copy != bq);           // check that modifying the copy doesn't affect the original
-  auto bq_move = std::move(bq);         // move construct a new bq
-  bq_move[0] = 90;
-  BOOST_CHECK(bq_move != bq_copy);      // check that modifying the moved one doesn't affect the copy
-  bq = bq_copy;                         // check the copy assignment now that bq has been moved to move_bq
-  BOOST_CHECK(bq == bq_copy);           // check that the bq is now the same as the bq_copy
-  BOOST_CHECK(bq != bq_move);           // check that the bq is not the same as the moved one
-  bq[0] = 93;
-  BOOST_CHECK(bq != bq_copy);           // check that modifying the copied version doesn't affect the original
-  auto bq_tmp = std::move(bq);          // take ownership of bq's memory so we can play around with bq again
-  bq = std::move(bq_move);              // we should be back to the original again!
-  BOOST_CHECK(bq == read.base_quals());
-  bq = bq;                              // check self assignment
-  BOOST_CHECK(bq == read.base_quals());
+BOOST_AUTO_TEST_CASE( sam_base_quals_templated_copy_and_move_constructors ) {
+  auto it = SingleSamReader{"testdata/test_simple.bam"}.begin();
+  auto c0 = (*it).base_quals();
+  auto copies = check_copy_constructor(c0);
+  auto c1 = get<0>(copies);
+  auto c2 = get<1>(copies);
+  auto c3 = get<2>(copies);
+  BOOST_CHECK(c0 == c1); 
+  BOOST_CHECK(c0 == c2);
+  BOOST_CHECK(c0 == c3);
+  c1[0] = 99;
+  BOOST_CHECK(c0 != c1);
+  auto m0 = (*it).base_quals();
+  auto m1 = check_move_constructor(m0);
+  auto m2 = (*it).base_quals();
+  BOOST_CHECK(m1 == m2);
+  m1[0] = 90;
+  BOOST_CHECK(m1 == m2);  // the underlying object is the same and it still exists 
+  BOOST_CHECK(m0 == m1);  // the underlying object is the same and it still exists (hasn't been destroyed, so they must still match)
+  BOOST_CHECK(m1 != c2);  // check that modifying the moved doesn't affect the copied
 }
 
 BOOST_AUTO_TEST_CASE( comparing_different_base_quals ) {
@@ -247,25 +252,26 @@ BOOST_AUTO_TEST_CASE( comparing_different_base_quals ) {
   BOOST_CHECK(read1.base_quals() != read2.base_quals());
 }
 
-BOOST_AUTO_TEST_CASE( sam_read_bases_copy_and_move_constructors ) {
-  auto read = *(SingleSamReader{"testdata/test_simple.bam"}.begin());
-  auto bases = read.bases();
-  auto bases_copy = bases;
-  bases_copy.set_base(0, Base::C);
-  BOOST_CHECK(bases_copy != bases);           // check that modifying the copy doesn't affect the original
-  auto bases_move = std::move(bases);         // move construct a new bases
-  bases_move.set_base(0, Base::N);
-  BOOST_CHECK(bases_move != bases_copy);      // check that modifying the moved one doesn't affect the copy
-  bases = bases_copy;                         // check the copy assignment now that bases has been moved to move_bases
-  BOOST_CHECK(bases == bases_copy);           // check that the bases is now the same as the bases_copy
-  BOOST_CHECK(bases != bases_move);           // check that the bases is not the same as the moved one
-  bases.set_base(0, Base::T);
-  BOOST_CHECK(bases != bases_copy);           // check that modifying the copied version doesn't affect the original
-  auto bases_tmp = std::move(bases);          // take ownership of bases's memory so we can play around with bases again
-  bases = std::move(bases_move);              // we should be back to the original again!
-  BOOST_CHECK(bases == read.bases());
-  bases = bases;                              // check self assignment
-  BOOST_CHECK(bases == read.bases());
+BOOST_AUTO_TEST_CASE( sam_read_bases_templated_copy_and_move_constructors ) {
+  auto it = SingleSamReader{"testdata/test_simple.bam"}.begin();
+  auto c0 = (*it).bases();
+  auto copies = check_copy_constructor(c0);
+  auto c1 = get<0>(copies);
+  auto c2 = get<1>(copies);
+  auto c3 = get<2>(copies);
+  BOOST_CHECK(c0 == c1); 
+  BOOST_CHECK(c0 == c2);
+  BOOST_CHECK(c0 == c3);
+  c1.set_base(0, Base::C);
+  BOOST_CHECK(c0 != c1);
+  auto m0 = (*it).bases();
+  auto m1 = check_move_constructor(m0);
+  auto m2 = (*it).bases();
+  BOOST_CHECK(m1 == m2);
+  m1.set_base(0, Base::N);
+  BOOST_CHECK(m1 == m2);  // the underlying object is the same and it still exists 
+  BOOST_CHECK(m0 == m1);  // the underlying object is the same and it still exists (hasn't been destroyed, so they must still match)
+  BOOST_CHECK(m1 != c2);  // check that modifying the moved doesn't affect the copied
 }
 
 BOOST_AUTO_TEST_CASE( comparing_different_read_bases ) {
@@ -361,19 +367,26 @@ BOOST_AUTO_TEST_CASE( sam_read_tags ) {
   BOOST_CHECK(missing(not_a_string_tag));            // this should yield "not a char" which is equal to a missing value
 }
 
-BOOST_AUTO_TEST_CASE( sam_copy_constructor ) {
-  const auto read1 = *(SingleSamReader{"testdata/test_simple.bam"}.begin());
-  auto read2 = read1; // copy read1
-  read2.set_alignment_start(5000);
-  BOOST_CHECK(read1.alignment_start() != read2.alignment_start());
-  read2 = read1; // copy assignment test 
-  BOOST_CHECK_EQUAL(read1.alignment_start(), read2.alignment_start());
-  read2.set_alignment_start(1);
-  read2 = read2; // check self assignment
-  BOOST_CHECK_EQUAL(read2.alignment_start(), 1);
-  auto read3 = read1; // check that variable length data field modifications don't affect the original record
-  read3.base_quals()[0] = 90;
-  BOOST_CHECK(read1.base_quals()[0] != read3.base_quals()[0]);
+BOOST_AUTO_TEST_CASE( sam_templated_copy_and_move_constructors ) {
+  auto it = SingleSamReader{"testdata/test_simple.bam"}.begin();
+  auto c0 = *it;
+  auto copies = check_copy_constructor(c0);
+  auto c1 = get<0>(copies);
+  auto c2 = get<1>(copies);
+  auto c3 = get<2>(copies);
+  BOOST_CHECK_EQUAL(c0.alignment_start(), c1.alignment_start()); 
+  BOOST_CHECK_EQUAL(c0.alignment_start(), c2.alignment_start());
+  BOOST_CHECK_EQUAL(c0.alignment_start(), c3.alignment_start());
+  c1.set_alignment_start(1);
+  BOOST_CHECK_NE(c0.alignment_start(), c1.alignment_start());
+  auto m0 = *it;
+  auto m1 = check_move_constructor(m0);
+  BOOST_CHECK(m0.empty());
+  auto m2 = *it;
+  BOOST_CHECK_EQUAL(m1.alignment_start(), m2.alignment_start());
+  m1.set_alignment_start(5000);
+  BOOST_CHECK_NE(m1.alignment_start(), m2.alignment_start());  // the underlying object is the same and it still exists 
+  BOOST_CHECK_NE(m1.alignment_start(), c2.alignment_start());  // check that modifying the moved doesn't affect the copied
 }
 
 void check_read_alignment_starts_and_stops(const Sam& read, const uint32_t astart, const uint32_t astop, const uint32_t ustart, const uint32_t ustop) {