[FIXED JENKINS-17779]

The fast-forwarding logic had a problem dealing with empty lines
jenkinsci · Jul 3, 2013 · d7ace70 · d7ace70
1 parent 00fed40
commit d7ace70
Show file tree

Hide file tree

Showing 3 changed files with 96 additions and 2 deletions.
diff --git a/pom.xml b/pom.xml
@@ -91,6 +91,12 @@
       <artifactId>equalsverifier</artifactId>
       <version>1.1.3</version>
     </dependency>
+    <dependency>
+      <groupId>com.google.code.findbugs</groupId>
+      <artifactId>jsr305</artifactId>
+      <version>2.0.1</version>
+      <scope>provided</scope>
+    </dependency>
   </dependencies>
 
   <developers>

diff --git a/src/main/java/hudson/plugins/timestamper/TimestampsIO.java b/src/main/java/hudson/plugins/timestamper/TimestampsIO.java
@@ -278,7 +278,7 @@ public Timestamp find(long consoleFilePointerToFind, Run<?, ?> build)
         throws IOException {
       BufferedInputStream logInputStream = null;
       try {
-        Timestamp found = null;
+        Timestamp found;
         boolean previousNewLine = true;
         byte[] buffer = new byte[1024];
         long bytesReadTotal = 0;
@@ -290,7 +290,7 @@ public Timestamp find(long consoleFilePointerToFind, Run<?, ?> build)
           }
           for (int i = 0; i < bytesRead; i++) {
             boolean newLine = buffer[i] == 0x0A;
-            if (previousNewLine && !newLine) {
+            if (previousNewLine) {
               found = next();
             } else {
               found = null;

diff --git a/src/test/java/hudson/plugins/timestamper/annotator/Jenkins17779Test.java b/src/test/java/hudson/plugins/timestamper/annotator/Jenkins17779Test.java
@@ -0,0 +1,88 @@
+package hudson.plugins.timestamper.annotator;
+
+import com.google.common.io.Files;
+import hudson.MarkupText;
+import hudson.console.ConsoleAnnotator;
+import hudson.model.Run;
+import hudson.plugins.timestamper.TimestampFormatter;
+import hudson.plugins.timestamper.TimestamperTestAssistant;
+import hudson.plugins.timestamper.TimestampsIO;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import org.jvnet.hudson.test.Bug;
+import org.mockito.invocation.InvocationOnMock;
+import org.mockito.stubbing.Answer;
+
+import javax.servlet.http.HttpServletRequest;
+import java.io.ByteArrayInputStream;
+import java.io.File;
+import java.io.InputStream;
+import java.util.concurrent.TimeUnit;
+
+import static org.hamcrest.Matchers.is;
+import static org.junit.Assert.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+/**
+ * @author Kohsuke Kawaguchi
+ */
+public class Jenkins17779Test {
+  @Rule
+  public TemporaryFolder folder = new TemporaryFolder();
+
+  private Run<?, ?> build;
+
+  /**
+   * Creates a dummy timestamp record that says each line took 1ms to render.
+   */
+  @Before
+  public void setUp() throws Exception {
+    final byte[] consoleLogContents = "AB\n\n\n\nCD\n".getBytes("US-ASCII");
+    File consoleLog = folder.newFile();
+    Files.write(consoleLogContents, consoleLog);
+
+    build = mock(Run.class);
+    when(build.getRootDir()).thenReturn(folder.getRoot());
+    when(build.getLogInputStream()).thenAnswer(new Answer<InputStream>() {
+      public InputStream answer(InvocationOnMock invocation) throws Throwable {
+        return new ByteArrayInputStream(consoleLogContents);
+      }
+    });
+    when(build.getLogFile()).thenReturn(consoleLog);
+
+    TimestampsIO.Writer writer = new TimestampsIO.Writer(build);
+    try {
+      for (int i = 0; i < 10; i++) {
+        writer.write(TimeUnit.MILLISECONDS.toNanos(i), i, 1);
+      }
+    } finally {
+      writer.close();
+    }
+  }
+
+  /**
+   * Regression test for JENKINS-17779.
+   */
+  @Test @Bug(17779)
+  public void fastForwardShouldHandleDoubleEmptyLines() throws Exception {
+    HttpServletRequest request = mock(HttpServletRequest.class);
+    TimestampFormatter f = new TimestampFormatter("S", "", request);
+
+    // fast-forward to 'CD' which is line 5
+    ConsoleAnnotator a = new TimestampAnnotator(f,6);
+    MarkupText text = new MarkupText("CD");
+    a.annotate(build, text);
+    assertThat(text.toString(true),is(TimestamperTestAssistant.span("4")+"CD"));
+
+    // should get the same result if we go line by line
+    a = new TimestampAnnotator(f,0);
+    for (int i=0; i<5; i++) {
+      text = new MarkupText("CD");
+      a.annotate(build, text);
+    }
+    assertThat(text.toString(true),is(TimestamperTestAssistant.span("4")+"CD"));
+  }
+}