Merge pull request #5558 from ethereum/testeth

winsvega · web-flow · commit f5db298d76aa · 2019-04-24T15:33:59.000+03:00
testeth check for unused folders in test fillers
diff --git a/test/tools/jsontests/BlockChainTests.cpp b/test/tools/jsontests/BlockChainTests.cpp
@@ -1008,16 +1008,19 @@ class bcTestFixture {
     bcTestFixture()
     {
         test::BlockchainTestSuite suite;
-        string const& casename = boost::unit_test::framework::current_test_case().p_name;
+        string const casename = boost::unit_test::framework::current_test_case().p_name;
+        boost::filesystem::path suiteFillerPath = suite.getFullPathFiller(casename).parent_path();
 
         //skip wallet test as it takes too much time (250 blocks) run it with --all flag
         if (casename == "bcWalletTest" && !test::Options::get().all)
         {
             cnote << "Skipping " << casename << " because --all option is not specified.\n";
+            test::TestOutputHelper::get().markTestFolderAsFinished(suiteFillerPath, casename);
             return;
         }
 
         suite.runAllTestsInFolder(casename);
+        test::TestOutputHelper::get().markTestFolderAsFinished(suiteFillerPath, casename);
     }
 };
 
@@ -1026,8 +1029,10 @@ class bcTransitionFixture {
     bcTransitionFixture()
     {
         test::TransitionTestsSuite suite;
-        string const& casename = boost::unit_test::framework::current_test_case().p_name;
+        string const casename = boost::unit_test::framework::current_test_case().p_name;
+        boost::filesystem::path suiteFillerPath = suite.getFullPathFiller(casename).parent_path();
         suite.runAllTestsInFolder(casename);
+        test::TestOutputHelper::get().markTestFolderAsFinished(suiteFillerPath, casename);
     }
 };
 
@@ -1037,14 +1042,18 @@ class bcGeneralTestsFixture
     bcGeneralTestsFixture()
     {
         test::BCGeneralStateTestsSuite suite;
-        string const& casename = boost::unit_test::framework::current_test_case().p_name;
+        string const casename = boost::unit_test::framework::current_test_case().p_name;
+        boost::filesystem::path suiteFillerPath = suite.getFullPathFiller(casename).parent_path();
+
         //skip this test suite if not run with --all flag (cases are already tested in state tests)
         if (!test::Options::get().all)
         {
             cnote << "Skipping hive test " << casename << ". Use --all to run it.\n";
+            test::TestOutputHelper::get().markTestFolderAsFinished(suiteFillerPath, casename);
             return;
         }
         suite.runAllTestsInFolder(casename);
+        test::TestOutputHelper::get().markTestFolderAsFinished(suiteFillerPath, casename);
     }
 };
 
diff --git a/test/tools/jsontests/StateTests.cpp b/test/tools/jsontests/StateTests.cpp
@@ -142,17 +142,24 @@ class GeneralTestFixture
 public:
 	GeneralTestFixture()
 	{
-		test::StateTestSuite suite;
-		string casename = boost::unit_test::framework::current_test_case().p_name;
-		if (casename == "stQuadraticComplexityTest" && !test::Options::get().all)
-		{
-			std::cout << "Skipping " << casename << " because --all option is not specified.\n";
-			return;
-		}
-		suite.runAllTestsInFolder(casename);
-	}
+        test::StateTestSuite suite;
+        string casename = boost::unit_test::framework::current_test_case().p_name;
+        boost::filesystem::path suiteFillerPath = suite.getFullPathFiller(casename).parent_path();
+
+        // Check specific test cases
+        static vector<string> timeConsumingTestSuites{string{"stQuadraticComplexityTest"}};
+        if (test::inArray(timeConsumingTestSuites, casename) && !test::Options::get().all)
+        {
+            std::cout << "Skipping " << casename << " because --all option is not specified.\n";
+            test::TestOutputHelper::get().markTestFolderAsFinished(suiteFillerPath, casename);
+            return;
+        }
+        suite.runAllTestsInFolder(casename);
+        test::TestOutputHelper::get().markTestFolderAsFinished(suiteFillerPath, casename);
+    }
 };
 
+
 BOOST_FIXTURE_TEST_SUITE(GeneralStateTests, GeneralTestFixture)
 
 //Frontier Tests
diff --git a/test/tools/jsontests/TransactionTests.cpp b/test/tools/jsontests/TransactionTests.cpp
@@ -219,9 +219,11 @@ class TransactionTestFixture
 public:
     TransactionTestFixture()
     {
-        string const& casename = boost::unit_test::framework::current_test_case().p_name;
         test::TransactionTestSuite suite;
+        string const casename = boost::unit_test::framework::current_test_case().p_name;
+        boost::filesystem::path suiteFillerPath = suite.getFullPathFiller(casename).parent_path();
         suite.runAllTestsInFolder(casename);
+        test::TestOutputHelper::get().markTestFolderAsFinished(suiteFillerPath, casename);
     }
 };
 
diff --git a/test/tools/jsontests/vm.cpp b/test/tools/jsontests/vm.cpp
@@ -488,13 +488,16 @@ class VmTestFixture
     VmTestFixture()
     {
         test::VmTestSuite suite;
-        string const& casename = boost::unit_test::framework::current_test_case().p_name;
+        string const casename = boost::unit_test::framework::current_test_case().p_name;
+        boost::filesystem::path suiteFillerPath = suite.getFullPathFiller(casename).parent_path();
         if (casename == "vmPerformance" && !Options::get().all)
         {
             std::cout << "Skipping " << casename << " because --all option is not specified.\n";
+            test::TestOutputHelper::get().markTestFolderAsFinished(suiteFillerPath, casename);
             return;
         }
         suite.runAllTestsInFolder(casename);
+        test::TestOutputHelper::get().markTestFolderAsFinished(suiteFillerPath, casename);
     }
 };
 
diff --git a/test/tools/libtesteth/TestOutputHelper.cpp b/test/tools/libtesteth/TestOutputHelper.cpp
@@ -18,12 +18,13 @@
  * Fixture class for boost output when running testeth
  */
 
-#include <boost/test/unit_test.hpp>
-#include <boost/io/ios_state.hpp>
 #include <libethashseal/Ethash.h>
 #include <libethcore/BasicAuthority.h>
-#include <test/tools/libtesteth/TestOutputHelper.h>
 #include <test/tools/libtesteth/Options.h>
+#include <test/tools/libtesteth/TestOutputHelper.h>
+#include <boost/filesystem.hpp>
+#include <boost/io/ios_state.hpp>
+#include <boost/test/unit_test.hpp>
 #include <numeric>
 
 using namespace std;
@@ -84,6 +85,7 @@ void TestOutputHelper::finishTest()
 
 void TestOutputHelper::printTestExecStats()
 {
+    checkUnfinishedTestFolders();
     if (Options::get().exectimelog)
     {
         boost::io::ios_flags_saver saver(cout);
@@ -100,3 +102,86 @@ void TestOutputHelper::printTestExecStats()
         saver.restore();
     }
 }
+
+// check if a boost path contain no test files
+bool pathHasTests(boost::filesystem::path const& _path)
+{
+    using fsIterator = boost::filesystem::directory_iterator;
+    for (fsIterator it(_path); it != fsIterator(); ++it)
+    {
+        // if the extention of a test file
+        if (boost::filesystem::is_regular_file(it->path()) &&
+            (it->path().extension() == ".json" || it->path().extension() == ".yml"))
+        {
+            // if the filename ends with Filler/Copier type
+            std::string const name = it->path().stem().filename().string();
+            std::string const suffix =
+                (name.length() > 7) ? name.substr(name.length() - 6) : string();
+            if (suffix == "Filler" || suffix == "Copier")
+                return true;
+        }
+    }
+    return false;
+}
+
+void TestOutputHelper::checkUnfinishedTestFolders()
+{
+    // -t SuiteName/caseName   parse caseName as filter
+    // rCurrentTestSuite is empty if run without -t argument
+    string filter;
+    size_t pos = Options::get().rCurrentTestSuite.find('/');
+    if (pos != string::npos)
+        filter = Options::get().rCurrentTestSuite.substr(pos + 1);
+
+    if (!filter.empty())
+    {
+        if (m_finishedTestFoldersMap.size() != 1)
+        {
+            std::cerr << "ERROR: Expected a single test to be passed: " << filter << "\n";
+            return;
+        }
+        std::map<boost::filesystem::path, FolderNameSet>::const_iterator it =
+            m_finishedTestFoldersMap.begin();
+        if (!pathHasTests(it->first / filter))
+            std::cerr << "WARNING: Test folder " << it->first / filter
+                      << " appears to have no tests!"
+                      << "\n";
+    }
+    else
+    {
+        for (auto const& allTestsIt : m_finishedTestFoldersMap)
+        {
+            boost::filesystem::path path = allTestsIt.first;
+            set<string> allFolders;
+            using fsIterator = boost::filesystem::directory_iterator;
+            for (fsIterator it(path); it != fsIterator(); ++it)
+            {
+                if (boost::filesystem::is_directory(*it))
+                {
+                    allFolders.insert(it->path().filename().string());
+                    if (!pathHasTests(it->path()))
+                        std::cerr << "WARNING: Test folder " << it->path()
+                                  << " appears to have no tests!"
+                                  << "\n";
+                }
+            }
+
+            std::vector<string> diff;
+            FolderNameSet finishedFolders = allTestsIt.second;
+            std::set_difference(allFolders.begin(), allFolders.end(), finishedFolders.begin(),
+                finishedFolders.end(), std::back_inserter(diff));
+            for (auto const& it : diff)
+            {
+                std::cerr << "WARNING: Test folder " << path / it << " appears to be unused!"
+                          << "\n";
+            }
+        }
+    }
+}
+
+void TestOutputHelper::markTestFolderAsFinished(
+    boost::filesystem::path const& _suitePath, string const& _folderName)
+{
+    // Mark test folder _folderName as finished for the test suite path _suitePath
+    m_finishedTestFoldersMap[_suitePath].emplace(_folderName);
+}
diff --git a/test/tools/libtesteth/TestOutputHelper.h b/test/tools/libtesteth/TestOutputHelper.h
@@ -52,16 +52,24 @@ class TestOutputHelper
 	boost::filesystem::path const& testFile() { return m_currentTestFileName; }
 	void printTestExecStats();
 
+    // Mark the _folderName as executed for a given _suitePath (to filler files)
+    void markTestFolderAsFinished(
+        boost::filesystem::path const& _suitePath, std::string const& _folderName);
+
 private:
 	TestOutputHelper() {}
-	Timer m_timer;
+    void checkUnfinishedTestFolders();  // Checkup that all test folders are active during the test
+                                        // run
+    Timer m_timer;
 	size_t m_currTest;
 	size_t m_maxTests;
 	std::string m_currentTestName;
 	std::string m_currentTestCaseName;
 	boost::filesystem::path m_currentTestFileName;
 	typedef std::pair<double, std::string> execTimeName;
 	std::vector<execTimeName> m_execTimeResults;
+    typedef std::set<std::string> FolderNameSet;
+    std::map<boost::filesystem::path, FolderNameSet> m_finishedTestFoldersMap;
 };
 
 class TestOutputHelperFixture

Original file line number	Diff line number	Diff line change
`@@ -1008,16 +1008,19 @@ class bcTestFixture {`
`1008`	`1008`	`bcTestFixture()`
`1009`	`1009`	`{`
`1010`	`1010`	`test::BlockchainTestSuite suite;`
`1011`		`- string const& casename = boost::unit_test::framework::current_test_case().p_name;`
	`1011`	`+ string const casename = boost::unit_test::framework::current_test_case().p_name;`
	`1012`	`+ boost::filesystem::path suiteFillerPath = suite.getFullPathFiller(casename).parent_path();`
`1012`	`1013`
`1013`	`1014`	`//skip wallet test as it takes too much time (250 blocks) run it with --all flag`
`1014`	`1015`	`if (casename == "bcWalletTest" && !test::Options::get().all)`
`1015`	`1016`	`{`
`1016`	`1017`	`cnote << "Skipping " << casename << " because --all option is not specified.\n";`
	`1018`	`+ test::TestOutputHelper::get().markTestFolderAsFinished(suiteFillerPath, casename);`
`1017`	`1019`	`return;`
`1018`	`1020`	`}`
`1019`	`1021`
`1020`	`1022`	`suite.runAllTestsInFolder(casename);`
	`1023`	`+ test::TestOutputHelper::get().markTestFolderAsFinished(suiteFillerPath, casename);`
`1021`	`1024`	`}`
`1022`	`1025`	`};`
`1023`	`1026`
`@@ -1026,8 +1029,10 @@ class bcTransitionFixture {`
`1026`	`1029`	`bcTransitionFixture()`
`1027`	`1030`	`{`
`1028`	`1031`	`test::TransitionTestsSuite suite;`
`1029`		`- string const& casename = boost::unit_test::framework::current_test_case().p_name;`
	`1032`	`+ string const casename = boost::unit_test::framework::current_test_case().p_name;`
	`1033`	`+ boost::filesystem::path suiteFillerPath = suite.getFullPathFiller(casename).parent_path();`
`1030`	`1034`	`suite.runAllTestsInFolder(casename);`
	`1035`	`+ test::TestOutputHelper::get().markTestFolderAsFinished(suiteFillerPath, casename);`
`1031`	`1036`	`}`
`1032`	`1037`	`};`
`1033`	`1038`
`@@ -1037,14 +1042,18 @@ class bcGeneralTestsFixture`
`1037`	`1042`	`bcGeneralTestsFixture()`
`1038`	`1043`	`{`
`1039`	`1044`	`test::BCGeneralStateTestsSuite suite;`
`1040`		`- string const& casename = boost::unit_test::framework::current_test_case().p_name;`
	`1045`	`+ string const casename = boost::unit_test::framework::current_test_case().p_name;`
	`1046`	`+ boost::filesystem::path suiteFillerPath = suite.getFullPathFiller(casename).parent_path();`
	`1047`	`+`
`1041`	`1048`	`//skip this test suite if not run with --all flag (cases are already tested in state tests)`
`1042`	`1049`	`if (!test::Options::get().all)`
`1043`	`1050`	`{`
`1044`	`1051`	`cnote << "Skipping hive test " << casename << ". Use --all to run it.\n";`
	`1052`	`+ test::TestOutputHelper::get().markTestFolderAsFinished(suiteFillerPath, casename);`
`1045`	`1053`	`return;`
`1046`	`1054`	`}`
`1047`	`1055`	`suite.runAllTestsInFolder(casename);`
	`1056`	`+ test::TestOutputHelper::get().markTestFolderAsFinished(suiteFillerPath, casename);`
`1048`	`1057`	`}`
`1049`	`1058`	`};`
`1050`	`1059`
Original file line number	Diff line number	Diff line change
`@@ -219,9 +219,11 @@ class TransactionTestFixture`
`219`	`219`	`public:`
`220`	`220`	`TransactionTestFixture()`
`221`	`221`	`{`
`222`		`- string const& casename = boost::unit_test::framework::current_test_case().p_name;`
`223`	`222`	`test::TransactionTestSuite suite;`
	`223`	`+ string const casename = boost::unit_test::framework::current_test_case().p_name;`
	`224`	`+ boost::filesystem::path suiteFillerPath = suite.getFullPathFiller(casename).parent_path();`
`224`	`225`	`suite.runAllTestsInFolder(casename);`
	`226`	`+ test::TestOutputHelper::get().markTestFolderAsFinished(suiteFillerPath, casename);`
`225`	`227`	`}`
`226`	`228`	`};`
`227`	`229`
Original file line number	Diff line number	Diff line change
`@@ -488,13 +488,16 @@ class VmTestFixture`
`488`	`488`	`VmTestFixture()`
`489`	`489`	`{`
`490`	`490`	`test::VmTestSuite suite;`
`491`		`- string const& casename = boost::unit_test::framework::current_test_case().p_name;`
	`491`	`+ string const casename = boost::unit_test::framework::current_test_case().p_name;`
	`492`	`+ boost::filesystem::path suiteFillerPath = suite.getFullPathFiller(casename).parent_path();`
`492`	`493`	`if (casename == "vmPerformance" && !Options::get().all)`
`493`	`494`	`{`
`494`	`495`	`std::cout << "Skipping " << casename << " because --all option is not specified.\n";`
	`496`	`+ test::TestOutputHelper::get().markTestFolderAsFinished(suiteFillerPath, casename);`
`495`	`497`	`return;`
`496`	`498`	`}`
`497`	`499`	`suite.runAllTestsInFolder(casename);`
	`500`	`+ test::TestOutputHelper::get().markTestFolderAsFinished(suiteFillerPath, casename);`
`498`	`501`	`}`
`499`	`502`	`};`
`500`	`503`