I think this was causing the random test failures --- the change in rev.14466...

I think this was causing the random test failures --- the change in rev.14466 was making the test slaves not to connect initially.

git-svn-id: https://hudson.dev.java.net/svn/hudson/trunk/hudson/main@14525 71c3de6d-444a-0410-be80-ed276b4c234a

core/src/main/java/hudson/model/Computer.java

@@ -161,7 +161,9 @@ public abstract class Computer extends AbstractModelObject implements AccessCont
      *      the new one will be started. If false, and a connect activity is already in progress, this method
      *      will do nothing and just return the pending connection operation.
      * @return
-     *      A {@link Future} representing pending completion of the task.
+     *      A {@link Future} representing pending completion of the task. The 'completion' includes
+     *      both a successful completion and a non-successful completion (such distinction typically doesn't
+     *      make much sense because as soon as {@link Computer} is connected it can be disconnected by some other threads.)
      */
     public abstract Future<?> connect(boolean forceReconnect);
 

core/src/main/java/hudson/model/Hudson.java

@@ -821,7 +821,7 @@ public final class Hudson extends AbstractModelObject implements ItemGroup<TopLe
                 RetentionStrategy retentionStrategy = c.getRetentionStrategy();
                 if (retentionStrategy != null) {
                     // if there is a retention strategy, it is responsible for deciding to start the computer
-                    retentionStrategy.check(c);
+                    retentionStrategy.start(c);
                 } else {
                     // we should never get here, but just in case, we'll fall back to the legacy behaviour
                     c.connect(true);

core/src/main/java/hudson/slaves/ComputerLauncher.java

@@ -47,6 +47,10 @@ public abstract class ComputerLauncher implements Describable<ComputerLauncher>,
      * The operation could also fail, in which case there's no need to make any callback notification,
      * (except to notify the user of the failure through {@link StreamTaskListener}.)
      *
+     * <p>
+     * This method must operate synchronously. Asynchrony is provided by {@link Computer#connect(boolean)} and
+     * its correct operation depends on this. 
+     *
      * @param listener
      *      The progress of the launch, as well as any error, should be sent to this listener.
      *

core/src/main/java/hudson/slaves/RetentionStrategy.java

@@ -42,6 +42,21 @@ public abstract class RetentionStrategy<T extends Computer> implements Describab
         return true;
     }
 
+    /**
+     * Called when a new {@link Computer} object is introduced (such as when Hudson started, or when
+     * a new slave is added.)
+     *
+     * <p>
+     * The default implementation of this method delegates to {@link #check(Computer)},
+     * but this allows {@link RetentionStrategy} to distinguish the first time invocation from the rest.
+     *
+     * @since 1.275
+     */
+    public void start(T c) {
+        check(c);
+    }
+
+
     /**
      * All registered {@link RetentionStrategy} implementations.
      */
@@ -55,6 +70,11 @@ public abstract class RetentionStrategy<T extends Computer> implements Describab
             return 1;
         }
 
+        @Override
+        public void start(Computer c) {
+            c.connect(false);
+        }
+
         public Descriptor<RetentionStrategy<?>> getDescriptor() {
             throw new UnsupportedOperationException();
         }

test/src/test/java/hudson/slaves/NodeProvisionerTest.java

@@ -171,11 +171,11 @@ public class NodeProvisionerTest extends HudsonTestCase {
         return builds;
     }
 
-    private void verifySuccessfulCompletion(List<Future<FreeStyleBuild>> builds) throws InterruptedException, ExecutionException {
+    private void verifySuccessfulCompletion(List<Future<FreeStyleBuild>> builds) throws Exception {
         System.out.println("Waiting for a completion");
         for (Future<FreeStyleBuild> f : builds) {
             FreeStyleBuild b = f.get();// if it's taking too long, abort.
-            assertEquals(Result.SUCCESS,b.getResult());
+            assertBuildStatus(Result.SUCCESS,b);
         }
     }
 }