Cppunit and data test enhancements (#4616)

* enh(CppUnit): Source code enhancements.

* enh(DataTest): Code enhancements (mostly to use override) to prevent wrong test calls when renaming.

Author: matejk

CppUnit/include/CppUnit/CppUnitException.h

@@ -33,11 +33,11 @@ class CppUnit_API CppUnitException: public std::exception
 	                 long data2lineNumber,
 	                 const std::string& fileName);
 	CppUnitException(const CppUnitException& other);
-	virtual ~CppUnitException() noexcept;
+	~CppUnitException() noexcept override;
 
 	CppUnitException& operator = (const CppUnitException& other);
 
-	const char* what() const noexcept;
+	const char* what() const noexcept override;
 
 	long lineNumber() const;
 	long data1LineNumber() const;

CppUnit/include/CppUnit/Orthodox.h

@@ -55,7 +55,7 @@ class Orthodox: public TestCase
 
 protected:
     ClassUnderTest call(ClassUnderTest object);
-    void runTest ();
+	void runTest () override;
 };
 
 

CppUnit/include/CppUnit/RepeatedTest.h

@@ -34,7 +34,7 @@ class CppUnit_API RepeatedTest: public TestDecorator
 
 	int countTestCases();
 	std::string toString();
-	void run(TestResult* result, const Test::Callback& callback = nullptr);
+	void run(TestResult* result, const Test::Callback& callback = nullptr) override;
 
 private:
 	const int _timesRepeat;

CppUnit/include/CppUnit/TestCase.h

@@ -13,7 +13,6 @@
 #include "CppUnit/TestResult.h"
 #include "CppUnit/CppUnitException.h"
 #include <string>
-#include <utility>
 #include <vector>
 #include <typeinfo>
 

CppUnit/src/TestCase.cpp

@@ -4,7 +4,7 @@
 
 
 #include <stdexcept>
-#include <math.h>
+#include <cmath>
 #include "CppUnit/TestCase.h"
 #include "CppUnit/TestResult.h"
 #include "CppUnit/estring.h"
@@ -117,7 +117,7 @@ void TestCase::run(TestResult *result, const Test::Callback& callback)
 	}
 	catch (CppUnitException& e)
 	{
-		CppUnitException* copy = new CppUnitException(e);
+		auto* copy = new CppUnitException(e);
 		result->addFailure(this, copy);
 	}
 	catch (std::exception& e)
@@ -128,7 +128,7 @@ void TestCase::run(TestResult *result, const Test::Callback& callback)
 	}
 	catch (...)
 	{
-		CppUnitException *e = new CppUnitException ("unknown exception");
+		auto* e = new CppUnitException ("unknown exception");
 		result->addError (this, e);
 	}
 	tearDown ();

CppUnit/src/TestDecorator.cpp

@@ -15,9 +15,7 @@ TestDecorator::TestDecorator(Test* test)
 }
 
 
-TestDecorator::~TestDecorator()
-{
-}
+TestDecorator::~TestDecorator() = default;
 
 
 int TestDecorator::countTestCases() const

CppUnit/src/TestRunner.cpp

@@ -8,7 +8,6 @@
 #include "CppUnit/TestSuite.h"
 #include "CppUnit/TextTestResult.h"
 #include <iostream>
-#include <fstream>
 
 
 namespace CppUnit {
@@ -28,8 +27,8 @@ TestRunner::TestRunner(std::ostream& ostr):
 
 TestRunner::~TestRunner()
 {
-	for (Mappings::iterator it = _mappings.begin(); it != _mappings.end(); ++it)
-		delete it->second;
+	for (auto & _mapping : _mappings)
+		delete _mapping.second;
 }
 
 
@@ -80,9 +79,9 @@ bool TestRunner::run(const std::vector<std::string>& args, const Test::Callback&
 		}
 		else if (arg == "-print")
 		{
-			for (Mappings::iterator it = _mappings.begin(); it != _mappings.end(); ++it)
+			for (auto& _mapping : _mappings)
 			{
-				print(it->first, it->second, 0);
+				print(_mapping.first, _mapping.second, 0);
 			}
 			printed = true;
 			continue;
@@ -104,8 +103,8 @@ bool TestRunner::run(const std::vector<std::string>& args, const Test::Callback&
 				return false;
 			}
 
-			Test* testToRun = 0;
-			for (Mappings::iterator it = _mappings.begin(); !testToRun && it != _mappings.end(); ++it)
+			Test* testToRun = nullptr;
+			for (auto it = _mappings.begin(); !testToRun && it != _mappings.end(); ++it)
 			{
 				testToRun = find(testCase, it->second, it->first);
 			}
@@ -124,18 +123,18 @@ bool TestRunner::run(const std::vector<std::string>& args, const Test::Callback&
 	if (all)
 	{
 		tests.clear();
-		for (Mappings::iterator it = _mappings.begin(); it != _mappings.end(); ++it)
+		for (auto& _mapping : _mappings)
 		{
-			collectAllTestCases(it->second, tests);
+			collectAllTestCases(_mapping.second, tests);
 		}
 	}
 
 	TextTestResult result(_ostr, ignore);
-	for (std::vector<Test*>::const_iterator it = tests.begin(); it != tests.end(); ++it)
+	for (auto testToRun : tests)
 	{
-		Test* testToRun = *it;
 		if(testToRun->getType() == Test::Long && !longRunning)
 			continue;
+
 		if (setup.size() > 0)
 			testToRun->addSetup(setup);
 
@@ -163,7 +162,7 @@ bool TestRunner::run(const std::vector<std::string>& args, const Test::Callback&
 
 void TestRunner::addTest(const std::string& name, Test* test)
 {
-	_mappings.push_back(Mapping(name, test));
+	_mappings.emplace_back(name, test);
 }
 
 
@@ -176,9 +175,9 @@ void TestRunner::print(const std::string& name, Test* pTest, int indent)
 	if (pSuite)
 	{
 		const std::vector<Test*>& tests = pSuite->tests();
-		for (std::vector<Test*>::const_iterator it = tests.begin(); it != tests.end(); ++it)
+		for (auto* test : tests)
 		{
-			print((*it)->toString(), *it, indent + 1);
+			print(test->toString(), test, indent + 1);
 		}
 	}
 }
@@ -192,17 +191,17 @@ Test* TestRunner::find(const std::string& name, Test* pTest, const std::string&
 	}
 	else
 	{
-		TestSuite* pSuite = dynamic_cast<TestSuite*>(pTest);
+		auto* pSuite = dynamic_cast<TestSuite*>(pTest);
 		if (pSuite)
 		{
 			const std::vector<Test*>& tests = pSuite->tests();
-			for (std::vector<Test*>::const_iterator it = tests.begin(); it != tests.end(); ++it)
+			for (auto* test : tests)
 			{
-				Test* result = find(name, *it, (*it)->toString());
+				Test* result = find(name, test, test->toString());
 				if (result) return result;
 			}
 		}
-		return 0;
+		return nullptr;
 	}
 }
 
@@ -212,14 +211,14 @@ int TestRunner::collectAllTestCases(Test* pTest, std::vector<Test*>& testcases)
 	int added = 0;
 	if (pTest->getType() == Test::Suite)
 	{
-		TestSuite* pSuite = dynamic_cast<TestSuite*>(pTest);
+		auto* pSuite = dynamic_cast<TestSuite*>(pTest);
 
 		if (pSuite)
 		{
 			const std::vector<Test*>& tests = pSuite->tests();
-			for (std::vector<Test*>::const_iterator it = tests.begin(); it != tests.end(); ++it)
+			for (auto* test : tests)
 			{
-				added += collectAllTestCases(*it, testcases);
+				added += collectAllTestCases(test, testcases);
 			}
 		}
 	}

CppUnit/src/TestSuite.cpp

@@ -13,22 +13,22 @@ namespace CppUnit {
 // Deletes all tests in the suite.
 void TestSuite::deleteContents()
 {
-	for (std::vector<Test*>::iterator it = _tests.begin(); it != _tests.end(); ++it)
-		delete *it;
+	for (auto* _test : _tests)
+		delete _test;
 }
 
 
 // Runs the tests and collects their result in a TestResult.
 void TestSuite::run(TestResult *result, const Test::Callback& callback)
 {
-	for (std::vector<Test*>::iterator it = _tests.begin(); it != _tests.end(); ++it)
+	for (auto* test : _tests)
 	{
 		if (result->shouldStop())
 			break;
 
-		Test *test = *it;
 		if (!setup().empty())
 			test->addSetup(setup());
+
 		test->run(result, callback);
 	}
 }
@@ -39,8 +39,8 @@ int TestSuite::countTestCases() const
 {
 	int count = 0;
 
-	for (std::vector<Test*>::const_iterator it = _tests.begin(); it != _tests.end(); ++it)
-		count += (*it)->countTestCases();
+	for (auto* _test : _tests)
+		count += _test->countTestCases();
 
 	return count;
 }

Data/DataTest/CMakeLists.txt

@@ -3,7 +3,7 @@ file(GLOB SRCS_G ${CMAKE_CURRENT_SOURCE_DIR}/src/*.cpp)
 POCO_SOURCES_AUTO(DATA_TEST_LIB_SRCS ${SRCS_G})
 
 # Headers
-file(GLOB HDRS_G ${CMAKE_CURRENT_SOURCE_DIR}/include/*.h)
+file(GLOB HDRS_G ${CMAKE_CURRENT_SOURCE_DIR}/include/Poco/Data/Test/*.h)
 POCO_HEADERS_AUTO(DATA_TEST_LIB_SRCS ${HDRS_G})
 
 # Version Resource

Data/DataTest/include/Poco/Data/Test/SQLExecutor.h

@@ -14,15 +14,13 @@
 #define DataTest_SQLExecutor_INCLUDED
 
 
+#include "CppUnit/TestCase.h"
 #include "Poco/Data/Test/DataTest.h"
 #include "Poco/Data/Session.h"
 #include "Poco/Data/BulkExtraction.h"
 #include "Poco/Data/BulkBinding.h"
-#include "Poco/NumberFormatter.h"
-#include "Poco/String.h"
 #include "Poco/Exception.h"
 #include <iostream>
-#include <string_view>
 
 
 namespace Poco {
@@ -55,7 +53,7 @@ class DataTest_API SQLExecutor: public CppUnit::TestCase
 	};
 
 	SQLExecutor(const std::string& name, Poco::Data::Session* pSession, Poco::Data::Session* pEncSession = nullptr, bool numberedPlaceHolders = false);
-	~SQLExecutor();
+	~SQLExecutor() override;
 
 	template <typename C>
 	void connection(C& c, const std::string& connectString)