This is the mail archive of the mauve-patches@sourceware.org mailing list for the Mauve project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: RFC: Harness with no busy-waiting


This is committed with a very small change, the actual patch it attached
to this message.

--Tony

On Tue, 2006-06-27 at 15:32 -0400, Anthony Balkissoon wrote:
> > Okay, here's a second attempt, no Sockets, no tricks.  Still RFC.
> > Comments?
> 
> By the way, this patch includes a known regression, any System.err.print
> statements inside of tests will not be echoed.  This will be fixed soon,
> if the overall patch is approved.
> 
> --Tony
Index: Harness.java
===================================================================
RCS file: /cvs/mauve/mauve/Harness.java,v
retrieving revision 1.15
diff -u -r1.15 Harness.java
--- Harness.java	24 Jun 2006 17:33:22 -0000	1.15
+++ Harness.java	27 Jun 2006 20:52:51 -0000
@@ -79,9 +79,6 @@
   // The location of the eclipse-ecj.jar file
   private static String ecjJarLocation = null;
   
-  // How long the bootclasspath finder program can run before termination
-  private static long bcp_timeout = 60000;
-  
   // How long a test may run before it is considered hung
   private static long runner_timeout = 60000;
 
@@ -138,12 +135,6 @@
   // A watcher to determine if runnerProcess is hung
   private static TimeoutWatcher runner_watcher = null;
   
-  // A flag indicating whether or not runnerProcess is hung
-  private static boolean testIsHung = false;
-  
-  // A lock used for synchronizing access to testIsHung
-  private static Object runner_lock = new Object();
-  
   // The arguments used when this Harness was invoked, we use this to create an
   // appropriate RunnerProcess
   private static String[] harnessArgs = null;
@@ -447,7 +438,7 @@
         // " -bootclasspath " followed by the detected path.
         if (temp != null)              
           return " -bootclasspath " + temp;
-        return temp;
+        return "";
       }
     
     // This section is for bootclasspath's specified with
@@ -484,52 +475,22 @@
         new BufferedReader
         (new InputStreamReader(p.getInputStream()));
       String bcpOutput = null;
-      // Create a timer to watch this new process.
-      TimeoutWatcher tw = new TimeoutWatcher(bcp_timeout);
-      tw.start();
       while (true)
         {
-          // If for some reason the process hangs, return null, indicating we
-          // cannot auto-detect the bootclasspath.
-          if (testIsHung)
+          // This readLine() is a blocking call.  This will hang if the 
+          // bootclasspath finder hangs.
+          bcpOutput = br.readLine();
+          if (bcpOutput.equals("BCP_FINDER:can't_find_bcp_"))
             {
-              synchronized (runner_lock)
-              {
-                testIsHung = false;
-              }
-              br.close();
-              p.destroy();
+              // This means the auto-detection failed.
               return null;
             }
-          if (br.ready())
+          else if (bcpOutput.startsWith("BCP_FINDER:"))
             {
-              bcpOutput = br.readLine();
-              if (bcpOutput == null)
-                {
-                  // This means the auto-detection failed.
-                  tw.stop();
-                  return null;
-                }
-              if (bcpOutput.startsWith("BCP_FINDER:"))
-                {
-                  tw.stop();
-                  return bcpOutput.substring(11);
-                }
-              else
-                System.out.println(bcpOutput);
+              return bcpOutput.substring(11);
             }
-	  else
-	    {
-	      // FIXME - Quickworkaround for busy-waiting, needs proper FIX.
-	      try
-	        {
-		  Thread.sleep(100);
-		}
-	      catch (InterruptedException ie)
-	        {
-		  // ignore
-		}
-	    }
+          else
+            System.out.println(bcpOutput);
         }
     }
     catch (IOException ioe)
@@ -634,10 +595,10 @@
     try
       {
         runTest("_dump_data_");
+        runnerProcess.destroy();
         runner_in.close();
         runner_in_err.close();
-        runner_out.close();        
-        runnerProcess.destroy();
+        runner_out.close();                
       } 
     catch (IOException e) 
       {
@@ -682,6 +643,8 @@
     // because it may be a while before we run the next test (due to 
     // preprocessing and compilation) and we don't want the runner_watcher
     // to time out.
+    if (runner_watcher != null)
+      runner_watcher.stop();
     runner_watcher = new TimeoutWatcher(runner_timeout);    
     runTest("_confirm_startup_");
     runner_watcher.stop();
@@ -774,9 +737,8 @@
   {
     String tn = stripPrefix(testName.replace(File.separatorChar, '.'));
     String outputFromTest;
-    StringBuffer sb = new StringBuffer();
     boolean invalidTest = false;
-    int temp = -1;
+    int temp = -99;
 
     // Start the timeout watcher
     if (runner_watcher.isAlive())
@@ -789,113 +751,72 @@
     
     while (true)
       {
-        // This while loop polls for output from the test process and 
-        // passes it to System.out unless it is the signal that the 
-        // test finished properly.  Also checks to see if the watcher
-        // thread has declared the test hung and if so ends the process.
-        if (testIsHung)
-          {
-            System.err.print(sb.toString());
-            if (testName.equals("_confirm_startup_"))
-              {
-                System.err.println("ERROR: Cannot create test runner process.  Exit");
-                System.exit(1);
-              }
-            synchronized (runner_lock)
-            {
-              testIsHung = false;
-            }
-            try
-              {
-                runner_in.close();
-                runner_in_err.close();
-                runner_out.close();
-                runnerProcess.destroy();
-              }
-            catch (IOException e)
-              {
-                System.err.println("Could not close the interprocess pipes.");
-                System.exit(- 1);
-              }
-            initProcess(harnessArgs);
-            break;
-          }
+        // Continue getting output from the RunnerProcess until it
+        // signals the test completed or was invalid, or until the
+        // TimeoutWatcher stops the RunnerProcess forcefully.
         try
         {
-          if (runner_in_err.ready())
-            sb.append(runner_in_err.readLine() + "\n");
-          if (runner_in.ready())
+          outputFromTest = runner_in.readLine();
+          if (outputFromTest == null)
             {
-              outputFromTest = runner_in.readLine();              
-              if (outputFromTest.startsWith("RunnerProcess:"))
-                {
-                  invalidTest = false;
-                  // This means the RunnerProcess has sent us back some
-                  // information.  This could be telling us that a check() call
-                  // was made and we should reset the timer, or that the 
-                  // test passed, or failed, or that it wasn't a test.
-                  if (outputFromTest.endsWith("restart-timer"))
-                    runner_watcher.reset();
-                  else if (outputFromTest.endsWith("pass"))
-                    {
-                      temp = 0;
-                      break;
-                    }
-                  else if (outputFromTest.indexOf("fail-") != -1)
-                    {
-                      total_check_fails += 
-                        Integer.parseInt(outputFromTest.substring(19));
-                      temp = 1;
-                      break;
-                    }
-                  else if (outputFromTest.endsWith("not-a-test"))
-                    {
-                      // Temporarily decrease the total number of tests,
-                      // because it will be incremented later even 
-                      // though the test was not a real test.
-                      invalidTest = true;
-                      total_tests--;
-                      temp = 0;
-                      break;
-                    }                  
-                } 
-              else if (outputFromTest.equals("_startup_okay_") || 
-                  outputFromTest.equals("_data_dump_okay_"))
-                return;
-              else
-                // This means it was just output from the test, like a 
-                // System.out.println within the test itself, we should
-                // pass these on to stdout.
-                System.out.println(outputFromTest);
+              // This means the test hung.
+              initProcess(harnessArgs);
+              temp = - 1;              
+              break;
             }
-          else
+          else if (outputFromTest.startsWith("RunnerProcess:"))
             {
-	      if (sb.length() != 0)
-	        {
-                  System.err.print(sb.toString());
-                  sb = new StringBuffer();
-		}
-
-              // FIXME - Quickworkaround for busy-waiting, needs proper FIX.
-              try
+              invalidTest = false;
+              // This means the RunnerProcess has sent us back some
+              // information. This could be telling us that a check() call
+              // was made and we should reset the timer, or that the
+              // test passed, or failed, or that it wasn't a test.
+              if (outputFromTest.endsWith("restart-timer"))
+                runner_watcher.reset();
+              else if (outputFromTest.endsWith("pass"))
+                {
+                  temp = 0;
+                  break;
+                }
+              else if (outputFromTest.indexOf("fail-loading") != -1)
+                {
+                  temp = 1;
+                  System.out.println(outputFromTest.substring(27));
+                }
+              else if (outputFromTest.indexOf("fail-") != - 1)
                 {
-                  Thread.sleep(100);
+                  total_check_fails += Integer.parseInt(outputFromTest.substring(19));
+                  temp = 1;
+                  break;
                 }
-              catch (InterruptedException ie)
+              else if (outputFromTest.endsWith("not-a-test"))
                 {
-                  // ignore
+                  // Temporarily decrease the total number of tests,
+                  // because it will be incremented later even
+                  // though the test was not a real test.
+                  invalidTest = true;
+                  total_tests--;
+                  temp = 0;
+                  break;
                 }
             }
+          else if (outputFromTest.equals("_startup_okay_")
+              || outputFromTest.equals("_data_dump_okay_"))
+            return;
+          else
+            // This means it was just output from the test, like a
+            // System.out.println within the test itself, we should
+            // pass these on to stdout.
+            System.out.println(outputFromTest);
         }
         catch (IOException e)
         {
         }
       }
-    System.err.print(sb.toString());
     if (temp == -1)
       {        
-        // This means the watcher thread had to stop the process 
-        // from running.  So this is a fail.
+        // This means the watcher thread had to stop the process
+        // from running. So this is a fail.
         if (verbose)
           System.out.println("  FAIL: timed out. \nTEST FAILED: timeout " + 
                              tn);
@@ -1224,12 +1145,19 @@
         }
       if (shouldContinue)
         {
-          // The test is hung, set testIsHung to true so the process will be 
-          // destroyed and restarted.      
-          synchronized (runner_lock)
+          // The test is hung, destroy and restart the RunnerProcess.      
+          try
           {
-            testIsHung = true;
+            runnerProcess.destroy();
+            runner_in.close();
+            runner_in_err.close();
+            runner_out.close();
           }
+          catch (IOException e)
+          {
+            System.err.println("Could not close the interprocess pipes.");
+            System.exit(- 1);
+          }          
         }
     }
   }
@@ -1279,7 +1207,7 @@
               System.err.println("WARNING: Cannot auto-detect the " +
                       "bootclasspath for your VM, please file a bug report" +
                       " specifying which VM you are testing.");
-              return;
+              temp = "can't_find_bcp_";              
             }
         }
       System.out.println(result + temp);
Index: RunnerProcess.java
===================================================================
RCS file: /cvs/mauve/mauve/RunnerProcess.java,v
retrieving revision 1.9
diff -u -r1.9 RunnerProcess.java
--- RunnerProcess.java	27 Jun 2006 15:14:06 -0000	1.9
+++ RunnerProcess.java	27 Jun 2006 20:52:51 -0000
@@ -51,6 +51,12 @@
   // A description of files that are not tests
   private static final String NOT_A_TEST_DESCRIPTION = "not-a-test";
   
+  // A description of files that fail to load
+  private static final String FAIL_TO_LOAD_DESCRIPTION = "failed-to-load";
+  
+  // A description of a test that throws an uncaught exception
+  private static final String UNCAUGHT_EXCEPTION_DESCRIPTION = "uncaught-exception";
+  
   // Total number of harness.check calls since the last checkpoint
   private int count = 0;
   
@@ -195,8 +201,10 @@
         try
         {
           testname = in.readLine();
+          if (testname == null)
+            System.exit(0);
           if (testname.equals("_dump_data_"))
-            {
+            {              
               if (useEMMA)
                 dumpCoverageData();
               else
@@ -238,6 +246,7 @@
     System.runFinalization();
 
     currentResult = new TestResult(name.substring(12));
+    description = name;
 
     checkPoint(null);
 
@@ -256,6 +265,7 @@
       }
     catch (Throwable ex)
       {
+        description = FAIL_TO_LOAD_DESCRIPTION;
         // Maybe the file was marked not-a-test, check that before we report
         // it as an error
         try
@@ -282,28 +292,35 @@
         {          
         }
         
-        String d = "FAIL: " + stripPrefix(name)
-                   + "uncaught exception when loading";
-        currentResult.addException(ex, "failed loading class ",
-                                   "couldn't load: " + name);
-        if (verbose || exceptions)
-          d += ": " + ex.toString();
-
-        if (exceptions)
-          ex.printStackTrace(System.out);
+        String r = getStackTraceString(ex, "          ");
+        currentResult.addException(ex, "failed loading class ", r);
+        
         debug(ex);
         if (ex instanceof InstantiationException
             || ex instanceof IllegalAccessException)
           debug("Hint: is the code we just loaded a public non-abstract "
                 + "class with a public nullary constructor???");
-        ++failures;
-        ++total;
+
+        
+        if (!verbose)
+          System.out.println ("FAIL: " + stripPrefix(name) + ": exception when loading:");
+        else
+          {
+            System.out.println ("TEST: "+stripPrefix(name));
+            System.out.println("  FAIL: exception when loading");
+          }
+        if (exceptions)
+          System.out.println(getStackTraceString(ex, "   "));
+        
+        if (verbose)
+          System.out.println("TEST FAILED: exception when loading "
+                             + stripPrefix(name));
+        return;
       }
 
     // If the harness started okay, now we run the test.
     if (t != null)
       {
-        description = name;
         try
           {
             if (verbose)
@@ -313,18 +330,24 @@
           }
         catch (Throwable ex)
           {
-            if (failures == 0 && !verbose)
-              System.out.println ("FAIL: " + stripPrefix(name) + ":");
-            removeSecurityManager();
+            
             String d = exceptionDetails(ex, name, exceptions);
             String r = getStackTraceString(ex, "          ");
-            currentResult.addException(ex, d, r);
+
+            if (failures == 0 && !verbose)
+                System.out.println ("FAIL: " + stripPrefix(name) + ":");
             System.out.println(d);
+            removeSecurityManager();
+            currentResult.addException(ex, d, r);
+            
             if (exceptions)
               System.out.println(getStackTraceString(ex, "   "));
             debug(ex);
-            ++failures;
-            ++total;
+            
+            if (verbose)
+              System.out.println("TEST FAILED: uncaught exception "
+                                 + stripPrefix(description));
+            description = UNCAUGHT_EXCEPTION_DESCRIPTION;
           }
       }
     if (report != null)
@@ -365,6 +388,17 @@
         System.out.println("RunnerProcess:not-a-test");
         return;
       }
+    else if (harness.description.equals(FAIL_TO_LOAD_DESCRIPTION))
+      {
+        System.out.println("RunnerProcess:fail-0");
+        return;
+      }
+    else if (harness.description.equals(UNCAUGHT_EXCEPTION_DESCRIPTION))
+      {
+        System.out.println("RunnerProcess:fail-0");
+        return;
+      }
+    
     // Print out a summary.
     int temp = harness.done();
     // Print the report if necessary.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]