From 643dfe3ea8f0c65bb87f0b7d996ec413642594cb Mon Sep 17 00:00:00 2001 From: Ichthyostega Date: Tue, 6 May 2014 00:24:45 +0200 Subject: [PATCH] fix long standing error in testsuite runner ...uncovered by switching to c++11 When invoking an individual test, we used to erase the 0-th cmdline argument, which happens to be allways the name of the test being invoked. Yet none of our tests actually complied to that contract. Rather, all tests taking arguments access them by 1-based argument index. Previously, the argument values just happened to be still in memory at the original location after erasing the 0st element. "Fixed" that by changing the contract. Now, the 0th argument remains in place, but when there are no additional arguments, the whole cmdline is cleared. This is messy, but the test runer needs to be rewritten entirely, the whole API is clumsy and dangerous. Ticket #289 --- src/lib/test/suite.cpp | 16 ++++++++++------ tests/library/helloworldtest.cpp | 3 ++- tests/library/iter-adapter-stl-test.cpp | 2 +- tests/library/iter-adapter-test.cpp | 2 +- tests/library/iter-source-test.cpp | 2 +- tests/library/itertools-test.cpp | 2 +- tests/library/util-foreach-test.cpp | 2 +- 7 files changed, 17 insertions(+), 12 deletions(-) diff --git a/src/lib/test/suite.cpp b/src/lib/test/suite.cpp index f1d569884..37c2b0bca 100644 --- a/src/lib/test/suite.cpp +++ b/src/lib/test/suite.cpp @@ -159,7 +159,7 @@ namespace test { -#define VALID(test,testID) \ +#define IS_VALID(test,testID) \ ASSERT ((test), "NULL testcase launcher for test '%s' found in testsuite '%s'", groupID_.c_str(),testID.c_str()); @@ -200,7 +200,7 @@ namespace test { { PTestMap tests = testcases.getGroup(groupID_); if (!tests) - throw lumiera::error::Invalid ("test group not found"); ///////// TODO: pass error description + throw lumiera::error::Invalid ("No tests found for test group \""+groupID_+"\""); if (0 < cmdline.size()) { @@ -211,8 +211,12 @@ namespace test { // first cmdline argument denotes a valid testcase registered in // this group: invoke just this test with the remaining cmdline Launcher* test = (*tests)[testID]; - cmdline.erase (cmdline.begin()); - VALID (test,testID); + IS_VALID (test,testID); + + // Special contract: in case the cmdline holds no actual arguments + // beyond the test name, then it's cleared entirely. + if (1 == cmdline.size()) cmdline.clear(); // TODO this invalidates also testID -- really need to redesign the API ////TICKET #289 + exitCode_ |= invokeTestCase (*(*test)(), cmdline); // TODO confusing statement, improve definition of test collection datatype Ticket #289 return true; } @@ -226,7 +230,7 @@ namespace test { { std::cout << "\n ----------"<< i->first<< "----------\n"; Launcher* test = (i->second); - VALID (test, i->first); + IS_VALID (test, i->first); exitCode_ |= invokeTestCase (*(*test)(), cmdline); // actually no cmdline arguments } return true; @@ -251,7 +255,7 @@ namespace test { cout << "\n\n"; cout << "TEST \""<second); - VALID (test, i->first); + IS_VALID (test, i->first); try { (*test)()->run(noCmdline); // run it to insert test generated output diff --git a/tests/library/helloworldtest.cpp b/tests/library/helloworldtest.cpp index d94b90fdb..753881b80 100644 --- a/tests/library/helloworldtest.cpp +++ b/tests/library/helloworldtest.cpp @@ -40,7 +40,8 @@ namespace test { */ class HelloWorld_test : public Test { - virtual void run(Arg arg) + virtual void + run (Arg arg) { int num= isnil(arg)? 1 : lexical_cast (arg[1]); diff --git a/tests/library/iter-adapter-stl-test.cpp b/tests/library/iter-adapter-stl-test.cpp index 021fe0108..d73ef5e34 100644 --- a/tests/library/iter-adapter-stl-test.cpp +++ b/tests/library/iter-adapter-stl-test.cpp @@ -89,7 +89,7 @@ namespace test{ virtual void run (Arg arg) { - if (0 < arg.size()) NUM_ELMS = lexical_cast (arg[0]); + if (0 < arg.size()) NUM_ELMS = lexical_cast (arg[1]); checkDistinctValIter(); diff --git a/tests/library/iter-adapter-test.cpp b/tests/library/iter-adapter-test.cpp index 599633e75..fc5324b7d 100644 --- a/tests/library/iter-adapter-test.cpp +++ b/tests/library/iter-adapter-test.cpp @@ -184,7 +184,7 @@ namespace test{ virtual void run (Arg arg) { - if (0 < arg.size()) NUM_ELMS = lexical_cast (arg[0]); + if (0 < arg.size()) NUM_ELMS = lexical_cast (arg[1]); useSimpleWrappedContainer (); diff --git a/tests/library/iter-source-test.cpp b/tests/library/iter-source-test.cpp index 8b994b921..f91b2d81f 100644 --- a/tests/library/iter-source-test.cpp +++ b/tests/library/iter-source-test.cpp @@ -165,7 +165,7 @@ namespace test{ virtual void run (Arg arg) { - if (0 < arg.size()) NUM_ELMS = lexical_cast (arg[0]); + if (0 < arg.size()) NUM_ELMS = lexical_cast (arg[1]); verify_simpleIters(); verify_transformIter(); diff --git a/tests/library/itertools-test.cpp b/tests/library/itertools-test.cpp index 93c767659..02c66270f 100644 --- a/tests/library/itertools-test.cpp +++ b/tests/library/itertools-test.cpp @@ -98,7 +98,7 @@ namespace test{ virtual void run (Arg arg) { - if (0 < arg.size()) NUM_ELMS = lexical_cast (arg[0]); + if (0 < arg.size()) NUM_ELMS = lexical_cast (arg[1]); TestSource source(NUM_ELMS); diff --git a/tests/library/util-foreach-test.cpp b/tests/library/util-foreach-test.cpp index eaa93f1a6..601ea4615 100644 --- a/tests/library/util-foreach-test.cpp +++ b/tests/library/util-foreach-test.cpp @@ -128,7 +128,7 @@ namespace test { void run (Arg arg) { - if (0 < arg.size()) NUM_ELMS = lexical_cast (arg[0]); + if (0 < arg.size()) NUM_ELMS = lexical_cast (arg[1]); VecI container = buildTestNumberz (NUM_ELMS); RangeI iterator(container.begin(), container.end());