jenkins-client-217 - Revert to sync pooling. Fix for unit tests

Author: wtrocki

jenkins-client/src/main/java/com/offbytwo/jenkins/helper/BuildConsoleStreamListener.java

@@ -12,12 +12,6 @@ public interface BuildConsoleStreamListener {
      */
     void onData(String newLogChunk);
 
-    /**
-     * Called by api when error happens
-     * @param error
-     */
-    void onError(Exception error);
-
     /**
      * Called when streaming console output is finished
      */

jenkins-client/src/main/java/com/offbytwo/jenkins/model/BuildWithDetails.java

@@ -402,47 +402,35 @@ public String getConsoleOutputHtml() throws IOException {
      * @param poolingTimeout pooling timeout (seconds) used to break pooling in case build stuck
      *
      */
-    public void streamConsoleOutput(final BuildConsoleStreamListener listener, final int poolingInterval, final int poolingTimeout) {
+    public void streamConsoleOutput(final BuildConsoleStreamListener listener, final int poolingInterval, final int poolingTimeout) throws InterruptedException, IOException {
         // Calculate start and timeout
         final long startTime = System.currentTimeMillis();
         final long timeoutTime = startTime + (poolingTimeout * 1000);
 
-        // Pool for logs in separate thread
-        new Thread(new Runnable() {
-            public void run() {
-                int bufferOffset = 0;
-                while (true) {
-                    try {
-                        Thread.sleep(poolingInterval * 1000);
-                    } catch (InterruptedException e) {
-                        listener.onError(e);
-                    }
-                    ConsoleLog consoleLog = null;
-                    try {
-                        consoleLog = getConsoleOutputText(bufferOffset);
-                    } catch (IOException e) {
-                        listener.onError(e);
-                    }
-                    String logString = consoleLog.getConsoleLog();
-                    if (logString != null && !logString.isEmpty()) {
-                        listener.onData(logString);
-                    }
-                    if (consoleLog.getHasMoreData()) {
-                        bufferOffset = consoleLog.getCurrentBufferSize();
-                    } else {
-                        listener.finished();
-                        break;
-                    }
-                    long currentTime = System.currentTimeMillis();
-
-                    if(currentTime> timeoutTime){
-                        LOGGER.warn("Pooling for build {0} for {2} timeout! Check if job stuck in jenkins",
-                                BuildWithDetails.this.getDisplayName(), BuildWithDetails.this.getNumber());
-                        break;
-                    }
-                }
+        int bufferOffset = 0;
+        while (true) {
+            Thread.sleep(poolingInterval * 1000);
+
+            ConsoleLog consoleLog = null;
+            consoleLog = getConsoleOutputText(bufferOffset);
+            String logString = consoleLog.getConsoleLog();
+            if (logString != null && !logString.isEmpty()) {
+                listener.onData(logString);
+            }
+            if (consoleLog.getHasMoreData()) {
+                bufferOffset = consoleLog.getCurrentBufferSize();
+            } else {
+                listener.finished();
+                break;
             }
-        }).start();
+            long currentTime = System.currentTimeMillis();
+
+            if (currentTime > timeoutTime) {
+                LOGGER.warn("Pooling for build {0} for {2} timeout! Check if job stuck in jenkins",
+                        BuildWithDetails.this.getDisplayName(), BuildWithDetails.this.getNumber());
+                break;
+            }
+        }
     }
 
     /**

jenkins-client/src/test/java/com/offbytwo/jenkins/model/BuildWithDetailsTest.java

@@ -5,15 +5,12 @@
 import com.offbytwo.jenkins.helper.BuildConsoleStreamListener;
 import org.apache.http.HttpResponse;
 import org.apache.http.NameValuePair;
-import org.apache.http.concurrent.BasicFuture;
 import org.apache.http.entity.StringEntity;
 import org.apache.http.message.BasicHeader;
 import org.junit.Before;
 import org.junit.Test;
 
 import java.io.IOException;
-import java.util.concurrent.CompletableFuture;
-import java.util.concurrent.Future;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.fail;
@@ -76,25 +73,50 @@ public void poolBuildLog() throws InterruptedException {
             given(response.getFirstHeader(BuildWithDetails.MORE_DATA_HEADER)).willReturn(moreDataHeader);
             given(response.getFirstHeader(BuildWithDetails.TEXT_SIZE_HEADER)).willReturn(textSizeHeader);
             given(client.post_form_with_result(anyString(),anyListOf(NameValuePair.class),anyBoolean())).willReturn(response);
+            final StringBuffer buffer=new StringBuffer();
             buildWithDetails.streamConsoleOutput(new BuildConsoleStreamListener() {
                 @Override
                 public void onData(String newLogChunk) {
                     assertThat(newLogChunk).isEqualTo(text);
+                    buffer.append(text);
                 }
 
                 @Override
-                public void onError(Exception error) {
-                    fail("No error expected",error);
+                public void finished() {
+                    assertThat(buffer.toString()).isEqualTo(text);
+                }
+            },1,2);
+        } catch (IOException e) {
+            fail("Should not return exception",e);
+        }
+    }
+
+    @Test
+    public void poolBuildLogShouldTimeout() throws InterruptedException {
+        try {
+            HttpResponse response = mock(HttpResponse.class);
+            final String text = "test test test";
+            int textLength = text.length();
+            given(response.getEntity()).willReturn(new StringEntity(text));
+            BasicHeader moreDataHeader = new BasicHeader(BuildWithDetails.MORE_DATA_HEADER, "true");
+            BasicHeader textSizeHeader = new BasicHeader(BuildWithDetails.TEXT_SIZE_HEADER, Integer.toString(textLength));
+            given(response.getFirstHeader(BuildWithDetails.MORE_DATA_HEADER)).willReturn(moreDataHeader);
+            given(response.getFirstHeader(BuildWithDetails.TEXT_SIZE_HEADER)).willReturn(textSizeHeader);
+            given(client.post_form_with_result(anyString(),anyListOf(NameValuePair.class),anyBoolean())).willReturn(response);
+            buildWithDetails.streamConsoleOutput(new BuildConsoleStreamListener() {
+                @Override
+                public void onData(String newLogChunk) {
+                    assertThat(newLogChunk).isEqualTo(text);
                 }
 
                 @Override
                 public void finished() {
+                    fail("Should timeout");
                 }
             },1,2);
-
-
         } catch (IOException e) {
             fail("Should not return exception",e);
         }
     }
+
 }