Seek for previous line in case it would never show

the first line, which is bad. This also means that
it would read more than LIMIT_SIZE, but that might
be ok because it's merely more than one line. Well,
if someone has a long line, that would be another
story.

2 files changed, 32 insertions, 13 deletions

diff --git a/lib/gitlab/ci/trace/stream.rb b/lib/gitlab/ci/trace/stream.rb
index 5e634bcac71..11dbe4de02c 100644
--- a/lib/gitlab/ci/trace/stream.rb
+++ b/lib/gitlab/ci/trace/stream.rb
@@ -14,6 +14,7 @@ module Gitlab
 
         def initialize
           @stream = yield
+          @stream.binmode
         end
 
         def valid?
@@ -30,7 +31,7 @@ module Gitlab
             last_bytes = stream_size
           end
           stream.seek(-last_bytes, IO::SEEK_END)
-          stream.readline
+          seek_previous_line(stream_size)
         end
 
         def append(data, offset)
@@ -53,7 +54,7 @@ module Gitlab
             read_last_lines(last_lines)
           else
             stream.read
-          end
+          end.force_encoding(Encoding.default_external)
         end
 
         def html_with_state(state = nil)
@@ -115,7 +116,24 @@ module Gitlab
           end
 
           chunks.join.lines.last(last_lines).join
-            .force_encoding(Encoding.default_external)
+        end
+
+        def seek_previous_line(max = stream_size)
+          return if stream.pos.zero?
+
+          seek_size = [stream.pos, BUFFER_SIZE].min
+
+          stream.seek(-seek_size, IO::SEEK_CUR)
+
+          buf = stream.read(seek_size)
+          idx = buf.rindex("\n")
+
+          if idx
+            stream.seek(-seek_size + idx + 1, IO::SEEK_CUR)
+          else
+            stream.seek(-seek_size, IO::SEEK_CUR)
+            seek_previous_line(max)
+          end
         end
       end
     end
diff --git a/spec/lib/gitlab/ci/trace/stream_spec.rb b/spec/lib/gitlab/ci/trace/stream_spec.rb
index 61ac76401ea..aa4d8ade538 100644
--- a/spec/lib/gitlab/ci/trace/stream_spec.rb
+++ b/spec/lib/gitlab/ci/trace/stream_spec.rb
@@ -2,7 +2,7 @@ require 'spec_helper'
 
 describe Gitlab::Ci::Trace::Stream do
   describe 'delegates' do
-    subject { described_class.new { nil } }
+    subject { described_class.new { StringIO.new } }
 
     it { is_expected.to delegate_method(:close).to(:stream) }
     it { is_expected.to delegate_method(:tell).to(:stream) }
@@ -17,12 +17,12 @@ describe Gitlab::Ci::Trace::Stream do
   describe '#limit' do
     let(:stream) do
       described_class.new do
-        StringIO.new("12345678")
+        StringIO.new((1..8).to_a.join("\n"))
       end
     end
 
     it 'if size is larger we start from beginning' do
-      stream.limit(10)
+      stream.limit(20)
 
       expect(stream.tell).to eq(0)
     end
@@ -30,26 +30,27 @@ describe Gitlab::Ci::Trace::Stream do
     it 'if size is smaller we start from the end' do
       stream.limit(2)
 
-      expect(stream.tell).to eq(6)
+      expect(stream.raw).to eq("7\n8")
     end
 
     context 'when the trace contains ANSI sequence and Unicode' do
-      let(:stream) do
-        described_class.new do
-          File.open(expand_fixture_path('trace/ansi-sequence-and-unicode'))
-        end
+      let(:trace_path) do
+        expand_fixture_path('trace/ansi-sequence-and-unicode')
       end
 
+      let(:stream) { described_class.new { File.open(trace_path) } }
+      let(:content) { File.read(trace_path) }
+
       it 'forwards to the next linefeed, case 1' do
         stream.limit(7)
 
-        expect(stream.raw).to eq('')
+        expect(stream.raw).to eq(content.lines.last)
       end
 
       it 'forwards to the next linefeed, case 2' do
         stream.limit(29)
 
-        expect(stream.raw).to eq("\e[01;32m許功蓋\e[0m\n")
+        expect(stream.raw).to eq(content.lines.last(2).join)
       end
     end
   end