From c406d16fbce77844b795acda5caba2a97d8d7db2 Mon Sep 17 00:00:00 2001
From: Ichthyostega <prg@ichthyostega.de>
Date: Thu, 4 Feb 2010 18:48:47 +0100
Subject: [PATCH] threadwrapper-test should have been better written this
 way...

---
 tests/lib/subsystem-runner-test.cpp | 23 ++++-----
 tests/lib/thread-wrapper-test.cpp   | 80 ++++++++++++++++++-----------
 2 files changed, 60 insertions(+), 43 deletions(-)

diff --git a/tests/lib/subsystem-runner-test.cpp b/tests/lib/subsystem-runner-test.cpp
index 0787c40e1..150eb56a5 100644
--- a/tests/lib/subsystem-runner-test.cpp
+++ b/tests/lib/subsystem-runner-test.cpp
@@ -43,7 +43,7 @@ using util::isnil;
 using util::cStr;
 using test::Test;
 using lib::Sync;
-using lib::NonrecursiveLock_Waitable;
+using lib::RecursiveLock_Waitable;
 using backend::Thread;
 
 
@@ -86,7 +86,7 @@ namespace test  {
        */
       class MockSys
         : public lumiera::Subsys
-        , public Sync<NonrecursiveLock_Waitable>
+        , public Sync<RecursiveLock_Waitable>      ////////////TODO: Sync could be eliminated entirely here
         {
           Literal id_;
           const string spec_;
@@ -97,6 +97,7 @@ namespace test  {
           volatile bool termRequest_;
           int running_duration_;
           
+          
           bool
           shouldStart (lumiera::Option&)
             {
@@ -117,7 +118,12 @@ namespace test  {
               
               if ("true"==startSpec) //----simulate successful subsystem start
                 {
-                  Thread (id_, bind (&MockSys::run, this, termination)).sync();            /////TODO: the thread description should be rather "Test: "+string(*this), but this requires to implement the class Literal as planned
+                  REQUIRE (!started_);
+                   
+                  Thread (id_, bind (&MockSys::run, this, termination))
+                        .sync();     // run-status hanshake
+                  
+                  ASSERT (started_);
                 }
               else
               if ("fail"==startSpec) //----not starting, incorrectly reporting success
@@ -160,6 +166,7 @@ namespace test  {
               string runSpec (extractID ("run",spec_));
               ASSERT (!isnil (runSpec));
               
+              // run-status handshake
               started_ = true;
               isUp_    = ("true"==runSpec || "throw"==runSpec);
               didRun_  = ("false"!=runSpec); // includes "fail" and "throw"
@@ -219,7 +226,7 @@ namespace test  {
           operator string ()  const { return "MockSys(\""+id_+"\")"; }
           
           friend inline ostream&
-          operator<< (ostream& os, MockSys const& mosi) { return os << string(mosi); }
+          operator<< (ostream& os, MockSys const& subsys) { return os << string(subsys); }
           
           bool didRun ()  const { return didRun_; }
         };
@@ -398,14 +405,6 @@ namespace test  {
             ASSERT (!unit3.didRun());
             // can't say for sure if unit4 actually did run
           }
-
-      public:
-        SubsystemRunner_test()
-        { }
-        
-        ~SubsystemRunner_test()
-        { }
-
       };
     
     
diff --git a/tests/lib/thread-wrapper-test.cpp b/tests/lib/thread-wrapper-test.cpp
index 6b686b2dd..9049fc45c 100644
--- a/tests/lib/thread-wrapper-test.cpp
+++ b/tests/lib/thread-wrapper-test.cpp
@@ -23,59 +23,81 @@
 
 #include "lib/test/run.hpp"
 
-#include "lib/symbol.hpp"
 #include "backend/thread-wrapper.hpp"
 #include "lib/sync.hpp"
+#include "lib/symbol.hpp"
 
 #include <tr1/functional>
 
 using std::tr1::bind;
 using test::Test;
-using lib::Sync;
-using lib::NonrecursiveLock_NoWait;
+
 
 namespace backend {
   namespace test  {
   
     namespace { // private test classes and data...
       
-      volatile ulong sum;
-      ulong checksum;
-      
-      Sync<NonrecursiveLock_NoWait> lockme;
-      
       const uint NUM_THREADS      = 20;
       const uint MAX_RAND_SUMMAND = 100;
       
-      uint
-      createVal()  ///< generating test values, remembering the sum
-      {
-        uint val(rand() % MAX_RAND_SUMMAND);
-        checksum += val;
-        return val;
-      }
+      
+      class Checker
+        : public lib::Sync<>
+        {
+          volatile ulong hot_sum_;
+          ulong control_sum_;
+          
+        public:
+          Checker() : hot_sum_(0), control_sum_(0) { }
+          
+          bool
+          verify()    ///< verify test values got handled and accounted
+            {
+              return 0 < hot_sum_
+                  && control_sum_ == hot_sum_;
+            }
+          
+          uint
+          createVal() ///< generating test values, remembering the control sum
+            {
+              uint val(rand() % MAX_RAND_SUMMAND);
+              control_sum_ += val;
+              return val;
+            }
+          
+          void
+          addValues (uint a, uint b)   ///< to be called concurrently
+            {
+              Lock guard(this);
+              
+              hot_sum_ *= 2;
+              usleep (200);             // force preemption
+              hot_sum_ += 2 * (a+b);
+              usleep (200);
+              hot_sum_ /= 2;
+            }
+        };
+      
+      
+      Checker checksum; ///< global variable used by multiple threads
+      
+      
       
       
       struct TestThread
-        : Sync<NonrecursiveLock_NoWait>  // using dedicated locking for this test TODO: needs classlock
-        , Thread
+        : Thread
         {
           TestThread()
             : Thread("test Thread creation",
-                     bind (&TestThread::theOperation, this, createVal(), createVal()))
+                     bind (&TestThread::theOperation, this, checksum.createVal(), checksum.createVal()))
             { }                         // note the binding (functor object) is passed as anonymous temporary
           
           
           void
           theOperation (uint a, uint b) ///< the actual operation running in a separate thread
             {
-              //Lock(this);     << broken we need a classlock, using sync-classlock is left as excercise for the reader
-              Sync<NonrecursiveLock_NoWait>::Lock gotit(&lockme);
-              sum *= 2;
-              usleep (200);             // force preemption
-              sum += (2*(a+b));
-              usleep (200);
-              sum /= 2;
+              checksum.addValues (a,b);
             }
         };
       
@@ -104,13 +126,11 @@ namespace backend {
         virtual void
         run (Arg)
           {
-            sum = checksum = 0;
             TestThread instances[NUM_THREADS]    SIDEEFFECT;
             
             usleep (200000);  // pause 200ms for the threads to terminate.....
             
-            ASSERT (0 < sum);
-            ASSERT (sum==checksum);
+            ASSERT (checksum.verify());
           }
       };
     
@@ -121,6 +141,4 @@ namespace backend {
     
     
     
-  } // namespace test
-
-} // namespace backend
+}} // namespace backend::test