From b7c8745d4413d7628ddcf6f0c64d4d708ae7051b Mon Sep 17 00:00:00 2001 From: Mauricio Carneiro Date: Thu, 21 Aug 2014 18:25:00 -0400 Subject: [PATCH] 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 | 12 ++- gamgee/sam.h | 2 +- test/fastq_reader_test.cpp | 25 ++++- test/indexed_sam_reader_test.cpp | 35 +++++-- test/interval_test.cpp | 21 ++++- test/sam_header_test.cpp | 16 ++-- test/sam_test.cpp | 153 +++++++++++++++++-------------- test/test_utils.h | 26 ++++++ test/variant_header_test.cpp | 15 +++ test/variant_reader_test.cpp | 8 ++ 10 files changed, 221 insertions(+), 92 deletions(-) create mode 100644 test/test_utils.h diff --git a/gamgee/fastq.h b/gamgee/fastq.h index 230cf6bd4..0caf70a72 100644 --- a/gamgee/fastq.h +++ b/gamgee/fastq.h @@ -31,15 +31,25 @@ class Fastq { /** * @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; } diff --git a/gamgee/sam.h b/gamgee/sam.h index 0eea33854..64501c311 100644 --- a/gamgee/sam.h +++ b/gamgee/sam.h @@ -239,7 +239,7 @@ class Sam { // modify non-variable length fields (things outside of the data member) // TODO: provide setter for TLEN (core.isize)? void set_chromosome(const uint32_t chr) { m_body->core.tid = int32_t(chr); } ///< @brief simple setter for the chromosome index. Index is 0-based. - void set_alignment_start(const uint32_t start) { m_body->core.pos = int32_t(start-1); } ///< @brief simple setter for the alignment start. @warning You should use (1-based and inclusive) alignment but internally this is stored 0-based to simplify BAM conversion. + void set_alignment_start(const uint32_t start) { printf("body: %p\n",m_body.get()); m_body->core.pos = int32_t(start-1); } ///< @brief simple setter for the alignment start. @warning You should use (1-based and inclusive) alignment but internally this is stored 0-based to simplify BAM conversion. void set_mate_chromosome(const uint32_t mchr) { m_body->core.mtid = int32_t(mchr); } ///< @brief simple setter for the mate's chromosome index. Index is 0-based. void set_mate_alignment_start(const uint32_t mstart) { m_body->core.mpos = int32_t(mstart - 1); } ///< @brief simple setter for the mate's alignment start. @warning You should use (1-based and inclusive) alignment but internally this is stored 0-based to simplify BAM conversion. diff --git a/test/fastq_reader_test.cpp b/test/fastq_reader_test.cpp index 557950a46..f86868d4b 100644 --- a/test/fastq_reader_test.cpp +++ b/test/fastq_reader_test.cpp @@ -1,7 +1,8 @@ -#include "../gamgee/fastq_reader.h" - #include +#include "fastq_reader.h" +#include "test_utils.h" + using namespace std; using namespace gamgee; @@ -45,3 +46,23 @@ 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); + BOOST_CHECK(m1 != m0); +} diff --git a/test/indexed_sam_reader_test.cpp b/test/indexed_sam_reader_test.cpp index 72b17a280..928a90dd9 100644 --- a/test/indexed_sam_reader_test.cpp +++ b/test/indexed_sam_reader_test.cpp @@ -1,6 +1,8 @@ +#include + #include "indexed_sam_reader.h" +#include "test_utils.h" -#include using namespace std; using namespace gamgee; @@ -79,10 +81,29 @@ BOOST_AUTO_TEST_CASE( indexed_single_readers_empty ) BOOST_AUTO_TEST_CASE( indexed_single_readers_move_constructor_and_assignment ) { auto reader1 = IndexedSingleSamReader{"testdata/test_simple.bam", vector{"."}}; auto it1 = reader1.begin(); - auto reader2 = std::move(reader1); // check move constructor - reader1 = IndexedSingleSamReader{"testdata/test_simple.bam", vector{"."}}; // 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 + auto c0 = *it1; + 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(4'000'000); + BOOST_CHECK_NE(c1.alignment_start(), c0.alignment_start()); + BOOST_CHECK_NE(c1.alignment_start(), c2.alignment_start()); + auto m0 = *it1; + auto m1 = check_move_constructor(m0); + auto m2 = *it1; + BOOST_CHECK_EQUAL(m1.alignment_start(), m2.alignment_start()); +} + +BOOST_AUTO_TEST_CASE( indexed_single_readers_begin_always_restarts ) { + auto reader1 = IndexedSingleSamReader{"testdata/test_simple.bam", vector{"."}}; + auto it1 = reader1.begin(); + ++it1; + auto it2 = reader1.begin(); + BOOST_CHECK_NE((*it1).alignment_start(), (*it2).alignment_start()); + ++it2; + BOOST_CHECK_EQUAL((*it1).alignment_start(), (*it2).alignment_start()); } diff --git a/test/interval_test.cpp b/test/interval_test.cpp index 19a54f3cc..0233d5ccb 100644 --- a/test/interval_test.cpp +++ b/test/interval_test.cpp @@ -1,7 +1,8 @@ -#include "interval.h" - #include +#include "interval.h" +#include "test_utils.h" + #include #include #include @@ -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)); // checking that c2 is a copy of get<2>(copies) + auto m1 = check_move_constructor(get<1>(copies)); + BOOST_CHECK(i0 == m1); +} diff --git a/test/sam_header_test.cpp b/test/sam_header_test.cpp index d380d42ea..209f6a525 100644 --- a/test/sam_header_test.cpp +++ b/test/sam_header_test.cpp @@ -1,7 +1,8 @@ -#include "sam_reader.h" - #include +#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. } diff --git a/test/sam_test.cpp b/test/sam_test.cpp index 2a71dd98c..279ab1434 100644 --- a/test/sam_test.cpp +++ b/test/sam_test.cpp @@ -4,6 +4,9 @@ #include "sam_reader.h" #include "sam_builder.h" #include "missing.h" + +#include "test_utils.h" + #include 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) { diff --git a/test/test_utils.h b/test/test_utils.h new file mode 100644 index 000000000..f4633f2ac --- /dev/null +++ b/test/test_utils.h @@ -0,0 +1,26 @@ +#ifndef gamgee_test_utils__guard +#define gamgee_test_utils__guard + +#include + +template +std::tuple check_copy_constructor (T& original) { + auto obj2 = original; // copy construction + auto obj3 = original; // copy construction + obj2 = obj2; // check self copy-assignment + obj3 = obj2; // copy assignment + return std::make_tuple(original, obj2, obj3); +} + +template +T check_move_constructor (T& original) { + auto obj2 = std::move(original); // move construction + auto obj3 = std::move(original); // move construction (of an already moved object...) + obj2 = std::move(obj2); // check self move-assignment + obj3 = std::move(obj2); // move assignment + return obj3; +} + + + +#endif diff --git a/test/variant_header_test.cpp b/test/variant_header_test.cpp index c6eea416b..df0d6335a 100644 --- a/test/variant_header_test.cpp +++ b/test/variant_header_test.cpp @@ -1,4 +1,5 @@ #include +#include "test_utils.h" #include "variant_header_builder.h" #include "missing.h" @@ -52,3 +53,17 @@ BOOST_AUTO_TEST_CASE( variant_header_builder_simple_building ) { BOOST_CHECK_EQUAL(vh.field_index("PASS"), 0); } +BOOST_AUTO_TEST_CASE( variant_header_move_and_copy_constructor ) { + auto builder = VariantHeaderBuilder{}; + builder.add_sample("S1"); + auto h0 = builder.build(); + auto copies = check_copy_constructor(h0); + auto c2 = get<2>(copies); + BOOST_CHECK_EQUAL_COLLECTIONS(h0.samples().begin(), h0.samples().end(), get<0>(copies).samples().begin(), get<0>(copies).samples().end()); + BOOST_CHECK_EQUAL_COLLECTIONS(h0.samples().begin(), h0.samples().end(), get<1>(copies).samples().begin(), get<1>(copies).samples().end()); + BOOST_CHECK_EQUAL_COLLECTIONS(h0.samples().begin(), h0.samples().end(), c2.samples().begin(), c2.samples().end()); + auto m1 = check_move_constructor(get<1>(copies)); + BOOST_CHECK_EQUAL_COLLECTIONS(h0.samples().begin(), h0.samples().end(), m1.samples().begin(), m1.samples().end()); + // can't modify a variant header... so this is it for the test. +} + diff --git a/test/variant_reader_test.cpp b/test/variant_reader_test.cpp index db30394fe..cb72db6b3 100644 --- a/test/variant_reader_test.cpp +++ b/test/variant_reader_test.cpp @@ -5,6 +5,7 @@ #include "indexed_variant_reader.h" #include "indexed_variant_iterator.h" #include "missing.h" +#include "test_utils.h" #include @@ -808,3 +809,10 @@ BOOST_AUTO_TEST_CASE( indexed_variant_iterator_move_test ) { BOOST_CHECK_EQUAL(rec1.alignment_start(), rec3.alignment_start()); } } + +BOOST_AUTO_TEST_CASE( varaint_reader_move_constructor ) { + auto r0 = SingleVariantReader{"testdata/test_variants.bcf"}; + auto r1 = SingleVariantReader{"testdata/test_variants.bcf"}; + auto m1 = check_move_constructor(r1); + BOOST_CHECK_EQUAL(r0.begin().operator*().alignment_start(), m1.begin().operator*().alignment_start()); +}