From 4e63175d6ad658a570c47489dac7c91dde90f9d7 Mon Sep 17 00:00:00 2001 From: Teemu Murtola Date: Fri, 7 Aug 2015 16:04:15 +0300 Subject: [PATCH] Add --ref-data update-changed Add an option to update only those reference data entries that fail the test. This makes it somewhat easier to maintain tests with floating-point reference data (as long as those floating-point values themselves do not change), since it is possible to update other parts of the test data without a double-precision build. User-/developer-facing documentation will be added separately together with more comprehensive documentation of the reference data framework later. Change-Id: Id03346358b91c5859e8ad5614f4ff4c4691e8594 --- src/testutils/refdata-checkers.h | 11 + src/testutils/refdata-impl.h | 89 +++++++- src/testutils/refdata-xml.cpp | 4 + src/testutils/refdata.cpp | 402 ++++++++++++++++------------------ src/testutils/refdata.h | 15 +- src/testutils/tests/refdata_tests.cpp | 137 +++++++++--- 6 files changed, 401 insertions(+), 257 deletions(-) diff --git a/src/testutils/refdata-checkers.h b/src/testutils/refdata-checkers.h index a7a457ecc2..f878f7e377 100644 --- a/src/testutils/refdata-checkers.h +++ b/src/testutils/refdata-checkers.h @@ -73,6 +73,17 @@ class IReferenceDataEntryChecker virtual ~IReferenceDataEntryChecker() {} }; +class NullChecker : public IReferenceDataEntryChecker +{ + public: + virtual void fillEntry(ReferenceDataEntry *) const {} + virtual ::testing::AssertionResult + checkEntry(const ReferenceDataEntry &, const std::string &) const + { + return ::testing::AssertionSuccess(); + } +}; + class ExactStringChecker : public IReferenceDataEntryChecker { public: diff --git a/src/testutils/refdata-impl.h b/src/testutils/refdata-impl.h index b63111fef9..1c87d5dfbb 100644 --- a/src/testutils/refdata-impl.h +++ b/src/testutils/refdata-impl.h @@ -66,7 +66,8 @@ class ReferenceDataEntry } ReferenceDataEntry(const char *type, const char *id) - : type_(type), id_(id != NULL ? id : ""), isTextBlock_(false) + : type_(type), id_(id != NULL ? id : ""), isTextBlock_(false), + correspondingOutputEntry_(NULL) { } @@ -76,6 +77,69 @@ class ReferenceDataEntry bool isTextBlock() const { return isTextBlock_; } const std::string &value() const { return value_; } const ChildList &children() const { return children_; } + ReferenceDataEntry *correspondingOutputEntry() const + { + return correspondingOutputEntry_; + } + + bool idMatches(const char *id) const + { + return (id == NULL && id_.empty()) || (id != NULL && id_ == id); + } + + ChildIterator findChild(const char *id, const ChildIterator &prev) const + { + if (children_.empty()) + { + return children_.end(); + } + ChildIterator child = prev; + bool wrappingSearch = true; + if (child != children_.end()) + { + if (id == NULL && (*child)->id().empty()) + { + wrappingSearch = false; + ++child; + if (child == children_.end()) + { + return children_.end(); + } + } + } + else + { + child = children_.begin(); + wrappingSearch = false; + } + do + { + if ((*child)->idMatches(id)) + { + return child; + } + ++child; + if (wrappingSearch && child == children_.end()) + { + child = children_.begin(); + } + } + while (child != children_.end() && child != prev); + return children_.end(); + } + bool isValidChild(const ChildIterator &prev) const + { + return prev != children_.end(); + } + + EntryPointer cloneToOutputEntry() + { + EntryPointer entry(new ReferenceDataEntry(type_.c_str(), id_.c_str())); + setCorrespondingOutputEntry(entry.get()); + entry->setValue(value()); + entry->isTextBlock_ = isTextBlock_; + return move(entry); + } void setValue(const std::string &value) { @@ -90,6 +154,18 @@ class ReferenceDataEntry value_ = value; isTextBlock_ = true; } + void makeCompound(const char *type) + { + type_.assign(type); + value_.clear(); + isTextBlock_ = false; + } + void setCorrespondingOutputEntry(ReferenceDataEntry *entry) + { + GMX_RELEASE_ASSERT(correspondingOutputEntry_ == NULL, + "Output entry already exists"); + correspondingOutputEntry_ = entry; + } ChildIterator addChild(EntryPointer child) { GMX_RELEASE_ASSERT(isCompound() || value_.empty(), @@ -98,11 +174,12 @@ class ReferenceDataEntry } private: - std::string type_; - std::string id_; - std::string value_; - bool isTextBlock_; - ChildList children_; + std::string type_; + std::string id_; + std::string value_; + bool isTextBlock_; + ChildList children_; + ReferenceDataEntry *correspondingOutputEntry_; }; } // namespace test diff --git a/src/testutils/refdata-xml.cpp b/src/testutils/refdata-xml.cpp index ae5a838fd1..c009dc5e93 100644 --- a/src/testutils/refdata-xml.cpp +++ b/src/testutils/refdata-xml.cpp @@ -206,6 +206,10 @@ void readEntry(xmlNodePtr element, ReferenceDataEntry *entry) { readChildEntries(element, entry); } + else if (hasCDataContent(element)) + { + entry->setTextBlockValue(getValueFromLeafElement(element)); + } else { entry->setValue(getValueFromLeafElement(element)); diff --git a/src/testutils/refdata.cpp b/src/testutils/refdata.cpp index 0f82dffdc5..7122c656b9 100644 --- a/src/testutils/refdata.cpp +++ b/src/testutils/refdata.cpp @@ -88,21 +88,36 @@ class TestReferenceDataImpl TestReferenceDataImpl(ReferenceDataMode mode, bool bSelfTestMode); //! Performs final reference data processing when test ends. - void onTestEnd(); + void onTestEnd(bool testPassed); //! Full path of the reference data file. std::string fullFilename_; /*! \brief - * Root entry for the reference data. + * Root entry for comparing the reference data. * - * If null after construction, the reference data is not present. + * Null after construction iff in compare mode and reference data was + * not loaded successfully. + * In all write modes, copies are present for nodes added to + * \a outputRootEntry_, and ReferenceDataEntry::correspondingOutputEntry() + * points to the copy in the output tree. */ - ReferenceDataEntry::EntryPointer rootEntry_; + ReferenceDataEntry::EntryPointer compareRootEntry_; /*! \brief - * Whether the reference data is being written (true) or compared - * (false). + * Root entry for writing new reference data. + * + * Null if only comparing against existing data. Otherwise, starts + * always as empty. + * When creating new reference data, this is maintained as a copy of + * \a compareRootEntry_. + * When updating existing data, entries are added either by copying + * from \a compareRootEntry_ (if they exist and comparison passes), or + * by creating new ones. */ - bool bWrite_; + ReferenceDataEntry::EntryPointer outputRootEntry_; + /*! \brief + * Whether updating existing reference data. + */ + bool updateMismatchingEntries_; //! `true` if self-testing (enables extra failure messages). bool bSelfTestMode_; /*! \brief @@ -157,7 +172,9 @@ TestReferenceDataImplPointer initReferenceDataInstanceForSelfTest(ReferenceDataM { if (g_referenceData) { - g_referenceData->onTestEnd(); + GMX_RELEASE_ASSERT(g_referenceData.unique(), + "Test cannot create multiple TestReferenceData instances"); + g_referenceData->onTestEnd(true); g_referenceData.reset(); } g_referenceData.reset(new internal::TestReferenceDataImpl(mode, true)); @@ -167,13 +184,13 @@ TestReferenceDataImplPointer initReferenceDataInstanceForSelfTest(ReferenceDataM class ReferenceDataTestEventListener : public ::testing::EmptyTestEventListener { public: - virtual void OnTestEnd(const ::testing::TestInfo &) + virtual void OnTestEnd(const ::testing::TestInfo &test_info) { if (g_referenceData) { GMX_RELEASE_ASSERT(g_referenceData.unique(), "Test leaked TestRefeferenceData objects"); - g_referenceData->onTestEnd(); + g_referenceData->onTestEnd(test_info.result()->Passed()); g_referenceData.reset(); } } @@ -190,7 +207,8 @@ class ReferenceDataTestEventListener : public ::testing::EmptyTestEventListener void initReferenceData(IOptionsContainer *options) { // Needs to correspond to the enum order in refdata.h. - const char *const refDataEnum[] = { "check", "create", "update" }; + const char *const refDataEnum[] = + { "check", "create", "update-changed", "update-all" }; options->addOption( StringOption("ref-data").enumValue(refDataEnum) .defaultEnumIndex(0) @@ -209,7 +227,7 @@ namespace internal TestReferenceDataImpl::TestReferenceDataImpl( ReferenceDataMode mode, bool bSelfTestMode) - : bWrite_(false), bSelfTestMode_(bSelfTestMode), bInUse_(false) + : updateMismatchingEntries_(false), bSelfTestMode_(bSelfTestMode), bInUse_(false) { const std::string dirname = bSelfTestMode @@ -218,32 +236,48 @@ TestReferenceDataImpl::TestReferenceDataImpl( const std::string filename = TestFileManager::getTestSpecificFileName(".xml"); fullFilename_ = Path::join(dirname, "refdata", filename); - bWrite_ = true; - if (mode != erefdataUpdateAll) - { - if (File::exists(fullFilename_)) - { - bWrite_ = false; - } - else if (mode == erefdataCompare) - { - bWrite_ = false; - return; - } - } - if (bWrite_) + switch (mode) { - rootEntry_ = ReferenceDataEntry::createRoot(); - } - else - { - rootEntry_ = readReferenceDataFile(fullFilename_); + case erefdataCompare: + if (File::exists(fullFilename_)) + { + compareRootEntry_ = readReferenceDataFile(fullFilename_); + } + break; + case erefdataCreateMissing: + if (File::exists(fullFilename_)) + { + compareRootEntry_ = readReferenceDataFile(fullFilename_); + } + else + { + compareRootEntry_ = ReferenceDataEntry::createRoot(); + outputRootEntry_ = ReferenceDataEntry::createRoot(); + } + break; + case erefdataUpdateChanged: + if (File::exists(fullFilename_)) + { + compareRootEntry_ = readReferenceDataFile(fullFilename_); + } + else + { + compareRootEntry_ = ReferenceDataEntry::createRoot(); + } + outputRootEntry_ = ReferenceDataEntry::createRoot(); + updateMismatchingEntries_ = true; + break; + case erefdataUpdateAll: + compareRootEntry_ = ReferenceDataEntry::createRoot(); + outputRootEntry_ = ReferenceDataEntry::createRoot(); + break; } } -void TestReferenceDataImpl::onTestEnd() +void TestReferenceDataImpl::onTestEnd(bool testPassed) { - if (bWrite_ && bInUse_ && rootEntry_) + // TODO: Only write the file with update-changed if there were actual changes. + if (testPassed && bInUse_ && outputRootEntry_) { std::string dirname = Path::getParentPath(fullFilename_); if (!Directory::exists(dirname)) @@ -253,7 +287,7 @@ void TestReferenceDataImpl::onTestEnd() GMX_THROW(TestException("Creation of reference data directory failed: " + dirname)); } } - writeReferenceDataFile(fullFilename_, *rootEntry_); + writeReferenceDataFile(fullFilename_, *outputRootEntry_); } } @@ -294,59 +328,55 @@ class TestReferenceChecker::Impl static const char * const cSequenceLengthName; //! Creates a checker that does nothing. - explicit Impl(bool bWrite); + Impl(); //! Creates a checker with a given root entry. - Impl(const std::string &path, ReferenceDataEntry *rootEntry, bool bWrite, + Impl(const std::string &path, ReferenceDataEntry *compareRootEntry, + ReferenceDataEntry *outputRootEntry, bool updateMismatchingEntries, bool bSelfTestMode, const FloatingPointTolerance &defaultTolerance); //! Returns the path of this checker with \p id appended. std::string appendPath(const char *id) const; - //! Returns whether an iterator returned by findEntry() is valid. - bool isValidEntry(const ReferenceDataEntry::ChildIterator &iter) const + //! Creates an entry with given parameters and fills it with \p checker. + ReferenceDataEntry::EntryPointer + createEntry(const char *type, const char *id, + const IReferenceDataEntryChecker &checker) const + { + ReferenceDataEntry::EntryPointer entry(new ReferenceDataEntry(type, id)); + checker.fillEntry(entry.get()); + return move(entry); + } + //! Checks an entry for correct type and using \p checker. + ::testing::AssertionResult + checkEntry(const ReferenceDataEntry &entry, const std::string &fullId, + const char *type, const IReferenceDataEntryChecker &checker) const { - if (rootEntry_ == NULL) + if (entry.type() != type) { - return false; + return ::testing::AssertionFailure() + << "Mismatching reference data item type" << std::endl + << " In item: " << fullId << std::endl + << " Actual: " << type << std::endl + << "Reference: " << entry.type(); } - return iter != rootEntry_->children().end(); + return checker.checkEntry(entry, fullId); } - - /*! \brief - * Finds a reference data entry. - * - * \param[in] type Type of entry to find (can be NULL, in which case - * any type is matched). - * \param[in] id Unique identifier of the entry (can be NULL, in - * which case the next entry without an id is matched). - * \returns Matching entry, or an invalid iterator (see - * isValidEntry()) if no matching entry found. - * - * Searches for an entry in the reference data that matches the given - * \p name and \p id. Searching starts from the entry that follows the - * previously matched entry (relevant for performance, and if there are - * nodes without ids). Note that the match pointer is not updated by - * this method. - */ - ReferenceDataEntry::ChildIterator - findEntry(const char *type, const char *id) const; + //! Finds an entry by id and updates the last found entry pointer. + ReferenceDataEntry *findEntry(const char *id); /*! \brief * Finds/creates a reference data entry to match against. * - * \param[in] type Type of entry to find. + * \param[in] type Type of entry to create. * \param[in] id Unique identifier of the entry (can be NULL, in * which case the next entry without an id is matched). - * \param[out] bFound Whether the entry was found (false if the entry - * was created in write mode). - * \returns Matching entry, or NULL if no matching entry found - * (NULL is never returned in write mode). - * - * Finds an entry using findEntry() and updates the match pointer if a - * match is found. If a match is not found, the method returns NULL in - * read mode and creates a new entry in write mode. + * \param[out] checker Checker to use for filling out created entries. + * \returns Matching entry, or NULL if no matching entry found + * (NULL is never returned in write mode; new entries are created + * instead). */ - ReferenceDataEntry *findOrCreateEntry(const char *type, const char *id, - bool *bFound); + ReferenceDataEntry * + findOrCreateEntry(const char *type, const char *id, + const IReferenceDataEntryChecker &checker); /*! \brief * Helper method for checking a reference data value. * @@ -376,7 +406,7 @@ class TestReferenceChecker::Impl * reference data could not be found, such that only one error is * issued for the missing compound, instead of every individual value. */ - bool shouldIgnore() const { return rootEntry_ == NULL; } + bool shouldIgnore() const { return compareRootEntry_ == NULL; } //! Default floating-point comparison tolerance. FloatingPointTolerance defaultTolerance_; @@ -389,30 +419,39 @@ class TestReferenceChecker::Impl */ std::string path_; /*! \brief - * Current node under which reference data is searched. + * Current entry under which reference data is searched for comparison. * - * Points to either the TestReferenceDataImpl::rootEntry_, or to + * Points to either the TestReferenceDataImpl::compareRootEntry_, or to * a compound entry in the tree rooted at that entry. * * Can be NULL, in which case this checker does nothing (doesn't even * report errors, see shouldIgnore()). */ - ReferenceDataEntry *rootEntry_; + ReferenceDataEntry *compareRootEntry_; /*! \brief - * Iterator to a child of \a rootEntry_ that was last found. + * Current entry under which entries for writing are created. * - * If isValidEntry() returns false, no entry has been found yet. + * Points to either the TestReferenceDataImpl::outputRootEntry_, or to + * a compound entry in the tree rooted at that entry. NULL if only + * comparing, or if shouldIgnore() returns `false`. + */ + ReferenceDataEntry *outputRootEntry_; + /*! \brief + * Iterator to a child of \a compareRootEntry_ that was last found. + * + * If `compareRootEntry_->isValidChild()` returns false, no entry has + * been found yet. * After every check, is updated to point to the entry that was used * for the check. * Subsequent checks start the search for the matching node on this * node. */ - ReferenceDataEntry::ChildIterator prevFoundNode_; + ReferenceDataEntry::ChildIterator lastFoundEntry_; /*! \brief * Whether the reference data is being written (true) or compared * (false). */ - bool bWrite_; + bool updateMismatchingEntries_; //! `true` if self-testing (enables extra failure messages). bool bSelfTestMode_; /*! \brief @@ -435,20 +474,24 @@ const char *const TestReferenceChecker::Impl::cSequenceType = "Sequence"; const char *const TestReferenceChecker::Impl::cSequenceLengthName = "Length"; -TestReferenceChecker::Impl::Impl(bool bWrite) +TestReferenceChecker::Impl::Impl() : defaultTolerance_(defaultRealTolerance()), - rootEntry_(NULL), bWrite_(bWrite), - bSelfTestMode_(false), seqIndex_(0) + compareRootEntry_(NULL), outputRootEntry_(NULL), + updateMismatchingEntries_(false), bSelfTestMode_(false), seqIndex_(0) { } -TestReferenceChecker::Impl::Impl(const std::string &path, ReferenceDataEntry *rootEntry, - bool bWrite, bool bSelfTestMode, +TestReferenceChecker::Impl::Impl(const std::string &path, + ReferenceDataEntry *compareRootEntry, + ReferenceDataEntry *outputRootEntry, + bool updateMismatchingEntries, bool bSelfTestMode, const FloatingPointTolerance &defaultTolerance) : defaultTolerance_(defaultTolerance), path_(path + "/"), - rootEntry_(rootEntry), prevFoundNode_(rootEntry->children().end()), - bWrite_(bWrite), bSelfTestMode_(bSelfTestMode), seqIndex_(0) + compareRootEntry_(compareRootEntry), outputRootEntry_(outputRootEntry), + lastFoundEntry_(compareRootEntry->children().end()), + updateMismatchingEntries_(updateMismatchingEntries), + bSelfTestMode_(bSelfTestMode), seqIndex_(0) { } @@ -461,95 +504,30 @@ TestReferenceChecker::Impl::appendPath(const char *id) const } -ReferenceDataEntry::ChildIterator -TestReferenceChecker::Impl::findEntry(const char *type, const char *id) const +ReferenceDataEntry *TestReferenceChecker::Impl::findEntry(const char *id) { - const ReferenceDataEntry::ChildList &children = rootEntry_->children(); - if (children.empty()) - { - return children.end(); - } - ReferenceDataEntry::ChildIterator node = prevFoundNode_; - bool bWrap = true; - if (node != children.end()) - { - if (id == NULL) - { - if ((*node)->id().empty()) - { - if (type == NULL || (*node)->type() == type) - { - bWrap = false; - ++node; - if (node == children.end()) - { - return children.end(); - } - } - } - } - } - else + ReferenceDataEntry::ChildIterator entry = compareRootEntry_->findChild(id, lastFoundEntry_); + seqIndex_ = (id == NULL) ? seqIndex_+1 : 0; + if (compareRootEntry_->isValidChild(entry)) { - node = children.begin(); - bWrap = false; + lastFoundEntry_ = entry; + return entry->get(); } - do - { - if (type == NULL || (*node)->type() == type) - { - if (id == NULL && (*node)->id().empty()) - { - return node; - } - if (!(*node)->id().empty()) - { - if (id != NULL && (*node)->id() == id) - { - return node; - } - } - } - ++node; - if (bWrap && node == children.end()) - { - node = children.begin(); - } - } - while (node != children.end() && node != prevFoundNode_); - return children.end(); + return NULL; } - ReferenceDataEntry * -TestReferenceChecker::Impl::findOrCreateEntry(const char *type, - const char *id, - bool *bFound) +TestReferenceChecker::Impl::findOrCreateEntry( + const char *type, const char *id, + const IReferenceDataEntryChecker &checker) { - *bFound = false; - if (rootEntry_ == NULL) - { - return NULL; - } - ReferenceDataEntry::ChildIterator node = findEntry(type, id); - if (isValidEntry(node)) + ReferenceDataEntry *entry = findEntry(id); + if (entry == NULL && outputRootEntry_ != NULL) { - *bFound = true; - prevFoundNode_ = node; + lastFoundEntry_ = compareRootEntry_->addChild(createEntry(type, id, checker)); + entry = lastFoundEntry_->get(); } - else - { - if (bWrite_) - { - ReferenceDataEntry::EntryPointer newEntry( - new ReferenceDataEntry(type, id)); - node = rootEntry_->addChild(move(newEntry)); - prevFoundNode_ = node; - } - } - seqIndex_ = (id == NULL) ? seqIndex_+1 : 0; - - return isValidEntry(node) ? node->get() : NULL; + return entry; } ::testing::AssertionResult @@ -561,31 +539,36 @@ TestReferenceChecker::Impl::processItem(const char *type, const char *id, return ::testing::AssertionSuccess(); } std::string fullId = appendPath(id); - bool bFound; - ReferenceDataEntry *entry = findOrCreateEntry(type, id, &bFound); + ReferenceDataEntry *entry = findOrCreateEntry(type, id, checker); if (entry == NULL) { return ::testing::AssertionFailure() << "Reference data item " << fullId << " not found"; } - if (bWrite_ && !bFound) - { - checker.fillEntry(entry); - return ::testing::AssertionSuccess(); - } - else + ::testing::AssertionResult result(checkEntry(*entry, fullId, type, checker)); + if (outputRootEntry_ != NULL && entry->correspondingOutputEntry() == NULL) { - ::testing::AssertionResult result(checker.checkEntry(*entry, fullId)); - if (bSelfTestMode_ && !result) + if (!updateMismatchingEntries_ || result) + { + outputRootEntry_->addChild(entry->cloneToOutputEntry()); + } + else { - ReferenceDataEntry expected(type, id); - checker.fillEntry(&expected); - result << std::endl - << "String value: " << expected.value() << std::endl - << " Ref. string: " << entry->value(); + ReferenceDataEntry::EntryPointer outputEntry(createEntry(type, id, checker)); + entry->setCorrespondingOutputEntry(outputEntry.get()); + outputRootEntry_->addChild(move(outputEntry)); + return ::testing::AssertionSuccess(); } - return result; } + if (bSelfTestMode_ && !result) + { + ReferenceDataEntry expected(type, id); + checker.fillEntry(&expected); + result << std::endl + << "String value: " << expected.value() << std::endl + << " Ref. string: " << entry->value(); + } + return result; } @@ -610,27 +593,22 @@ TestReferenceData::~TestReferenceData() } -bool TestReferenceData::isWriteMode() const -{ - return impl_->bWrite_; -} - - TestReferenceChecker TestReferenceData::rootChecker() { - if (!isWriteMode() && !impl_->bInUse_ && !impl_->rootEntry_) + if (!impl_->bInUse_ && !impl_->compareRootEntry_) { ADD_FAILURE() << "Reference data file not found: " << impl_->fullFilename_; } impl_->bInUse_ = true; - if (!impl_->rootEntry_) + if (!impl_->compareRootEntry_) { - return TestReferenceChecker(new TestReferenceChecker::Impl(isWriteMode())); + return TestReferenceChecker(new TestReferenceChecker::Impl()); } return TestReferenceChecker( - new TestReferenceChecker::Impl("", impl_->rootEntry_.get(), - isWriteMode(), impl_->bSelfTestMode_, + new TestReferenceChecker::Impl("", impl_->compareRootEntry_.get(), + impl_->outputRootEntry_.get(), + impl_->updateMismatchingEntries_, impl_->bSelfTestMode_, defaultRealTolerance())); } @@ -664,12 +642,6 @@ TestReferenceChecker::~TestReferenceChecker() } -bool TestReferenceChecker::isWriteMode() const -{ - return impl_->bWrite_; -} - - void TestReferenceChecker::setDefaultTolerance( const FloatingPointTolerance &tolerance) { @@ -679,12 +651,14 @@ void TestReferenceChecker::setDefaultTolerance( bool TestReferenceChecker::checkPresent(bool bPresent, const char *id) { - if (isWriteMode() || impl_->shouldIgnore()) + if (impl_->shouldIgnore() || impl_->outputRootEntry_ != NULL) { return bPresent; } - ReferenceDataEntry::ChildIterator node = impl_->findEntry(NULL, id); - bool bFound = impl_->isValidEntry(node); + ReferenceDataEntry::ChildIterator entry + = impl_->compareRootEntry_->findChild(id, impl_->lastFoundEntry_); + const bool bFound + = impl_->compareRootEntry_->isValidChild(entry); if (bFound != bPresent) { ADD_FAILURE() << "Mismatch while checking reference data item '" @@ -694,7 +668,7 @@ bool TestReferenceChecker::checkPresent(bool bPresent, const char *id) } if (bFound && bPresent) { - impl_->prevFoundNode_ = node; + impl_->lastFoundEntry_ = entry; return true; } return false; @@ -705,19 +679,27 @@ TestReferenceChecker TestReferenceChecker::checkCompound(const char *type, const { if (impl_->shouldIgnore()) { - return TestReferenceChecker(new Impl(isWriteMode())); + return TestReferenceChecker(new Impl()); } std::string fullId = impl_->appendPath(id); - bool bFound; - ReferenceDataEntry *newNode = impl_->findOrCreateEntry(type, id, &bFound); - if (newNode == NULL) + ReferenceDataEntry *entry = impl_->findOrCreateEntry(type, id, NullChecker()); + if (entry == NULL) { ADD_FAILURE() << "Reference data item " << fullId << " not found"; - return TestReferenceChecker(new Impl(isWriteMode())); + return TestReferenceChecker(new Impl()); + } + if (impl_->updateMismatchingEntries_) + { + entry->makeCompound(type); + } + if (impl_->outputRootEntry_ != NULL && entry->correspondingOutputEntry() == NULL) + { + impl_->outputRootEntry_->addChild(entry->cloneToOutputEntry()); } return TestReferenceChecker( - new Impl(fullId, newNode, isWriteMode(), - impl_->bSelfTestMode_, impl_->defaultTolerance_)); + new Impl(fullId, entry, entry->correspondingOutputEntry(), + impl_->updateMismatchingEntries_, impl_->bSelfTestMode_, + impl_->defaultTolerance_)); } diff --git a/src/testutils/refdata.h b/src/testutils/refdata.h index a571d7a8e1..1acfd9fffc 100644 --- a/src/testutils/refdata.h +++ b/src/testutils/refdata.h @@ -86,6 +86,13 @@ enum ReferenceDataMode */ erefdataCreateMissing, /*! \brief + * Update reference data that does not pass comparison. + * + * Tests utilizing reference data should always pass in this mode unless + * there is an I/O error. + */ + erefdataUpdateChanged, + /*! \brief * Update reference data, overwriting old data. * * Tests utilizing reference data should always pass in this mode unless @@ -188,9 +195,6 @@ class TestReferenceData */ ~TestReferenceData(); - //! Returns true if reference data is currently being written. - bool isWriteMode() const; - /*! \brief * Returns a root-level checker object for comparisons. * @@ -237,9 +241,6 @@ class TestReferenceChecker //! Assigns a test reference checker. TestReferenceChecker &operator=(const TestReferenceChecker &other); - //! Returns true if reference data is currently being written. - bool isWriteMode() const; - /*! \brief * Sets the tolerance for floating-point comparisons. * @@ -263,7 +264,7 @@ class TestReferenceChecker * present, otherwise checks that the data item is absent. * If the check fails, a non-fatal Google Test assertion is generated. * - * If isWriteMode() returns true, the check always succeeds and the + * If reference data is being written, the check always succeeds and the * return value is \p bPresent. * * The main use of this method is to assign meaning for missing diff --git a/src/testutils/tests/refdata_tests.cpp b/src/testutils/tests/refdata_tests.cpp index 1331d9eda6..133a6d3f0e 100644 --- a/src/testutils/tests/refdata_tests.cpp +++ b/src/testutils/tests/refdata_tests.cpp @@ -51,15 +51,15 @@ namespace { +using gmx::test::TestReferenceData; +using gmx::test::TestReferenceChecker; + TEST(ReferenceDataTest, HandlesSimpleData) { - using gmx::test::TestReferenceData; - using gmx::test::TestReferenceChecker; - { TestReferenceData data(gmx::test::erefdataUpdateAll); TestReferenceChecker checker(data.rootChecker()); - checker.checkBoolean(true, "int"); + checker.checkBoolean(true, "bool"); checker.checkInteger(1, "int"); checker.checkInt64(1ULL<<42, "int64"); checker.checkUInt64(1ULL<<42, "uint64"); @@ -69,7 +69,7 @@ TEST(ReferenceDataTest, HandlesSimpleData) { TestReferenceData data(gmx::test::erefdataCompare); TestReferenceChecker checker(data.rootChecker()); - checker.checkBoolean(true, "int"); + checker.checkBoolean(true, "bool"); checker.checkInteger(1, "int"); checker.checkInt64(1ULL<<42, "int64"); checker.checkUInt64(1ULL<<42, "uint64"); @@ -80,8 +80,6 @@ TEST(ReferenceDataTest, HandlesSimpleData) TEST(ReferenceDataTest, HandlesFloatingPointData) { - using gmx::test::TestReferenceData; - using gmx::test::TestReferenceChecker; const float floatValue = 4.0f/3.0f; const double doubleValue = 4.0/3.0; @@ -104,9 +102,6 @@ TEST(ReferenceDataTest, HandlesFloatingPointData) TEST(ReferenceDataTest, HandlesPresenceChecks) { - using gmx::test::TestReferenceData; - using gmx::test::TestReferenceChecker; - { TestReferenceData data(gmx::test::erefdataUpdateAll); TestReferenceChecker checker(data.rootChecker()); @@ -132,9 +127,6 @@ TEST(ReferenceDataTest, HandlesPresenceChecks) TEST(ReferenceDataTest, HandlesStringBlockData) { - using gmx::test::TestReferenceData; - using gmx::test::TestReferenceChecker; - { TestReferenceData data(gmx::test::erefdataUpdateAll); TestReferenceChecker checker(data.rootChecker()); @@ -153,8 +145,6 @@ TEST(ReferenceDataTest, HandlesStringBlockData) TEST(ReferenceDataTest, HandlesVectorData) { - using gmx::test::TestReferenceData; - using gmx::test::TestReferenceChecker; int veci[3] = { -1, 3, 5 }; float vecf[3] = { -2.3f, 1.43f, 2.5f }; double vecd[3] = { -2.3, 1.43, 2.5 }; @@ -178,8 +168,6 @@ TEST(ReferenceDataTest, HandlesVectorData) TEST(ReferenceDataTest, HandlesSequenceData) { - using gmx::test::TestReferenceData; - using gmx::test::TestReferenceChecker; const int seq[5] = { -1, 3, 5, 2, 4 }; { @@ -197,8 +185,6 @@ TEST(ReferenceDataTest, HandlesSequenceData) TEST(ReferenceDataTest, HandlesIncorrectData) { - using gmx::test::TestReferenceData; - using gmx::test::TestReferenceChecker; int seq[5] = { -1, 3, 5, 2, 4 }; { @@ -224,8 +210,6 @@ TEST(ReferenceDataTest, HandlesIncorrectData) TEST(ReferenceDataTest, HandlesMissingData) { - using gmx::test::TestReferenceData; - using gmx::test::TestReferenceChecker; const int seq[5] = { -1, 3, 5, 2, 4 }; { @@ -245,8 +229,6 @@ TEST(ReferenceDataTest, HandlesMissingData) TEST(ReferenceDataTest, HandlesMissingReferenceDataFile) { - using gmx::test::TestReferenceData; - using gmx::test::TestReferenceChecker; const int seq[5] = { -1, 3, 5, 2, 4 }; EXPECT_NONFATAL_FAILURE({ @@ -262,9 +244,6 @@ TEST(ReferenceDataTest, HandlesMissingReferenceDataFile) TEST(ReferenceDataTest, HandlesSpecialCharactersInStrings) { - using gmx::test::TestReferenceData; - using gmx::test::TestReferenceChecker; - { TestReferenceData data(gmx::test::erefdataUpdateAll); TestReferenceChecker checker(data.rootChecker()); @@ -283,8 +262,6 @@ TEST(ReferenceDataTest, HandlesSpecialCharactersInStrings) TEST(ReferenceDataTest, HandlesSequenceItemIndices) { - using gmx::test::TestReferenceData; - using gmx::test::TestReferenceChecker; int seq[5] = { -1, 3, 5, 2, 4 }; { @@ -308,9 +285,6 @@ TEST(ReferenceDataTest, HandlesSequenceItemIndices) TEST(ReferenceDataTest, HandlesMultipleChecksAgainstSameData) { - using gmx::test::TestReferenceData; - using gmx::test::TestReferenceChecker; - { TestReferenceData data(gmx::test::erefdataUpdateAll); TestReferenceChecker checker(data.rootChecker()); @@ -332,9 +306,6 @@ TEST(ReferenceDataTest, HandlesMultipleChecksAgainstSameData) TEST(ReferenceDataTest, HandlesMultipleNullIds) { - using gmx::test::TestReferenceData; - using gmx::test::TestReferenceChecker; - { TestReferenceData data(gmx::test::erefdataUpdateAll); TestReferenceChecker checker(data.rootChecker()); @@ -350,4 +321,102 @@ TEST(ReferenceDataTest, HandlesMultipleNullIds) } } + +TEST(ReferenceDataTest, HandlesUpdateChangedWithoutChanges) +{ + { + TestReferenceData data(gmx::test::erefdataUpdateAll); + TestReferenceChecker checker(data.rootChecker()); + checker.checkInteger(1, "int"); + checker.checkString("Test", "string"); + TestReferenceChecker compound(checker.checkCompound("Compound", "Compound")); + compound.checkInteger(2, "int"); + } + { + TestReferenceData data(gmx::test::erefdataUpdateChanged); + TestReferenceChecker checker(data.rootChecker()); + checker.checkInteger(1, "int"); + checker.checkString("Test", "string"); + TestReferenceChecker compound(checker.checkCompound("Compound", "Compound")); + compound.checkInteger(2, "int"); + } + { + TestReferenceData data(gmx::test::erefdataCompare); + TestReferenceChecker checker(data.rootChecker()); + checker.checkInteger(1, "int"); + checker.checkString("Test", "string"); + TestReferenceChecker compound(checker.checkCompound("Compound", "Compound")); + compound.checkInteger(2, "int"); + } +} + +TEST(ReferenceDataTest, HandlesUpdateChangedWithValueChanges) +{ + { + TestReferenceData data(gmx::test::erefdataUpdateAll); + TestReferenceChecker checker(data.rootChecker()); + checker.checkInteger(1, "int"); + checker.checkString("Test", "string"); + } + { + TestReferenceData data(gmx::test::erefdataUpdateChanged); + TestReferenceChecker checker(data.rootChecker()); + checker.checkInteger(2, "int"); + checker.checkString("Test", "string"); + } + { + TestReferenceData data(gmx::test::erefdataCompare); + TestReferenceChecker checker(data.rootChecker()); + checker.checkInteger(2, "int"); + checker.checkString("Test", "string"); + } +} + +TEST(ReferenceDataTest, HandlesUpdateChangedWithTypeChanges) +{ + { + TestReferenceData data(gmx::test::erefdataUpdateAll); + TestReferenceChecker checker(data.rootChecker()); + checker.checkInteger(1, "foo"); + checker.checkString("Test", "string"); + } + { + TestReferenceData data(gmx::test::erefdataUpdateChanged); + TestReferenceChecker checker(data.rootChecker()); + checker.checkString("foo", "foo"); + checker.checkString("Test", "string"); + } + { + TestReferenceData data(gmx::test::erefdataCompare); + TestReferenceChecker checker(data.rootChecker()); + checker.checkString("foo", "foo"); + checker.checkString("Test", "string"); + } +} + +TEST(ReferenceDataTest, HandlesUpdateChangedWithCompoundChanges) +{ + { + TestReferenceData data(gmx::test::erefdataUpdateAll); + TestReferenceChecker checker(data.rootChecker()); + checker.checkInteger(1, "1"); + TestReferenceChecker compound(checker.checkCompound("Compound", "2")); + compound.checkInteger(2, "int"); + } + { + TestReferenceData data(gmx::test::erefdataUpdateChanged); + TestReferenceChecker checker(data.rootChecker()); + TestReferenceChecker compound(checker.checkCompound("Compound", "1")); + compound.checkInteger(2, "int"); + checker.checkString("Test", "2"); + } + { + TestReferenceData data(gmx::test::erefdataCompare); + TestReferenceChecker checker(data.rootChecker()); + TestReferenceChecker compound(checker.checkCompound("Compound", "1")); + compound.checkInteger(2, "int"); + checker.checkString("Test", "2"); + } +} + } // namespace -- 2.11.4.GIT