Fix most test failures

14 files changed, 257 insertions, 211 deletions

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 62f66a71c38..0674b40a898 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -235,14 +235,14 @@ module Ci
       update_attributes(coverage: coverage) if coverage.present?
     end
 
-    def has_trace?
-      trace.has_trace?
-    end
-
     def trace
       Gitlab::Ci::Trace.new(self)
     end
 
+    def has_trace?
+      trace.exist?
+    end
+
     def trace=(data)
       raise NotImplementedError
     end
diff --git a/lib/api/runner.rb b/lib/api/runner.rb
index 56618d48b9f..6fbb02cb3aa 100644
--- a/lib/api/runner.rb
+++ b/lib/api/runner.rb
@@ -115,7 +115,7 @@ module API
       put '/:id' do
         job = authenticate_job!
 
-        job.update_attributes(trace: params[:trace]) if params[:trace]
+        job.trace.set(params[:trace]) if params[:trace]
 
         Gitlab::Metrics.add_event(:update_build,
                                   project: job.project.path_with_namespace)
@@ -145,19 +145,14 @@ module API
         content_range = request.headers['Content-Range']
         content_range = content_range.split('-')
 
-        job.trace.write do |stream|
-          current_length = stream.size
-          unless current_length == content_range[0].to_i
-            return error!('416 Range Not Satisfiable', 416, { 'Range' => "0-#{current_length}" })
-          end
-
-          trace_part = job.hide_secrets(request.body.read)
-          stream.append(trace_part, content_range[0].to_i)
-
-          status 202
-          header 'Job-Status', job.status
-          header 'Range', "0-#{trace.size}"
+        stream_size = job.trace.append(request.body.read, content_range[0].to_i)
+        if stream_size < 0
+          return error!('416 Range Not Satisfiable', 416, { 'Range' => "0-#{-stream_size}" })
         end
+
+        status 202
+        header 'Job-Status', job.status
+        header 'Range', "0-#{stream_size}"
       end
 
       desc 'Authorize artifacts uploading for job' do
diff --git a/lib/ci/api/builds.rb b/lib/ci/api/builds.rb
index e8c6652b618..1e80dd7d5db 100644
--- a/lib/ci/api/builds.rb
+++ b/lib/ci/api/builds.rb
@@ -61,7 +61,7 @@ module Ci
 
           update_runner_info
 
-          build.update_attributes(trace: params[:trace]) if params[:trace]
+          build.trace.set(params[:trace]) if params[:trace]
 
           Gitlab::Metrics.add_event(:update_build,
                                     project: build.project.path_with_namespace)
@@ -92,19 +92,14 @@ module Ci
           content_range = request.headers['Content-Range']
           content_range = content_range.split('-')
 
-          job.trace.write do |stream|
-            current_length = stream.size
-            unless current_length == content_range[0].to_i
-              return error!('416 Range Not Satisfiable', 416, { 'Range' => "0-#{current_length}" })
-            end
-
-            trace_part = build.hide_secrets(request.body.read)
-            stream.append(trace_part, content_range[0].to_i)
-
-            status 202
-            header 'Build-Status', build.status
-            header 'Range', "0-#{trace.size}"
+          stream_size = build.trace.append(request.body.read, content_range[0].to_i)
+          if stream_size < 0
+            return error!('416 Range Not Satisfiable', 416, { 'Range' => "0-#{-stream_size}" })
           end
+
+          status 202
+          header 'Job-Status', build.status
+          header 'Range', "0-#{stream_size}"
         end
 
         # Authorize artifacts uploading for build - Runners only
diff --git a/lib/gitlab/ci/trace.rb b/lib/gitlab/ci/trace.rb
index 445911ea9fb..7c0f4a016e2 100644
--- a/lib/gitlab/ci/trace.rb
+++ b/lib/gitlab/ci/trace.rb
@@ -9,8 +9,8 @@ module Gitlab
         @job = job
       end
 
-      def has_trace?
-        current_trace_path.present? || old_trace.present?
+      def exist?
+        current_path.present? || old_trace.present?
       end
 
       def html(max_lines: nil)
@@ -38,9 +38,20 @@ module Gitlab
         end
       end
 
+      def append(data, offset)
+        write do |stream|
+          current_length = stream.size
+          return -current_length unless current_length == offset
+
+          data = job.hide_secrets(data)
+          stream.append(data, offset)
+          stream.size
+        end
+      end
+
       def erase_trace!
-        trace_paths.find do |trace_path|
-          File.rm(trace_path, force: true)
+        paths.find do |trace_path|
+          FileUtils.rm(trace_path, force: true)
         end
 
         job.erase_old_trace!
@@ -48,8 +59,8 @@ module Gitlab
 
       def read
         stream = Gitlab::Ci::Trace::Stream.new do
-          if current_trace_path
-            File.open(current_trace_path, "rb")
+          if current_path
+            File.open(current_path, "rb")
           elsif old_trace
             StringIO.new(old_trace)
           end
@@ -62,43 +73,45 @@ module Gitlab
 
       def write
         stream = Gitlab::Ci::Trace::Stream.new do
-          File.open(ensure_trace_path, "a+b")
+          File.open(ensure_path, "a+b")
         end
 
         yield stream
+
+        job.touch if job.needs_touch?
       ensure
         stream&.close
       end
 
       private
 
-      def ensure_trace_path
-        return current_trace_path if current_trace_path
+      def ensure_path
+        return current_path if current_path
 
-        ensure_trace_directory
-        default_trace_path
+        ensure_directory
+        default_path
       end
 
-      def ensure_trace_directory
-        unless Dir.exist?(default_trace_directory)
-          FileUtils.mkdir_p(default_trace_directory)
+      def ensure_directory
+        unless Dir.exist?(default_directory)
+          FileUtils.mkdir_p(default_directory)
         end
       end
 
-      def current_trace_path
-        @current_trace_path ||= trace_paths.find do |trace_path|
+      def current_path
+        @current_path ||= paths.find do |trace_path|
           File.exist?(trace_path)
         end
       end
 
-      def trace_paths
+      def paths
         [
-          default_trace_path,
-          deprecated_trace_path
+          default_path,
+          deprecated_path
         ].compact
       end
 
-      def default_trace_directory
+      def default_directory
         File.join(
           Settings.gitlab_ci.builds_path,
           job.created_at.utc.strftime("%Y_%m"),
@@ -106,11 +119,11 @@ module Gitlab
         )
       end
 
-      def default_trace_path
-        File.join(default_trace_directory, "#{job.id}.log")
+      def default_path
+        File.join(default_directory, "#{job.id}.log")
       end
 
-      def deprecated_trace_path
+      def deprecated_path
         File.join(
           Settings.gitlab_ci.builds_path,
           job.created_at.utc.strftime("%Y_%m"),
diff --git a/lib/gitlab/ci/trace/stream.rb b/lib/gitlab/ci/trace/stream.rb
index 8cd44903b1f..6632a1fdfe6 100644
--- a/lib/gitlab/ci/trace/stream.rb
+++ b/lib/gitlab/ci/trace/stream.rb
@@ -4,7 +4,7 @@ module Gitlab
       # This was inspired from: http://stackoverflow.com/a/10219411/1520132
       class Stream
         BUFFER_SIZE = 4096
-        LIMIT_SIZE = 60
+        LIMIT_SIZE = 50.kilobytes
 
         attr_reader :stream
 
@@ -22,10 +22,10 @@ module Gitlab
           self.path.present?
         end
 
-        def limit(max_bytes = LIMIT_SIZE)
+        def limit(last_bytes = LIMIT_SIZE)
           stream_size = size
-          if stream_size < max_bytes
-            max_bytes = stream_size
+          if stream_size < last_bytes
+            last_bytes = stream_size
           end
           stream.seek(-max_bytes, IO::SEEK_END)
         end
@@ -43,22 +43,22 @@ module Gitlab
           stream.flush()
         end
 
-        def html_with_state(state = nil)
-          ::Ci::Ansi2html.convert(stream, state)
-        end
-
-        def raw(max_lines: nil)
+        def raw(last_lines: nil)
           return unless valid?
 
-          if max_lines
-            read_last_lines(max_lines)
+          if last_lines
+            read_last_lines(last_lines)
           else
             stream.read
           end
         end
 
-        def html(max_lines: nil)
-          text = raw(max_lines: max_lines)
+        def html_with_state(state = nil)
+          ::Ci::Ansi2html.convert(stream, state)
+        end
+        
+        def html(last_lines: nil)
+          text = raw(last_lines: last_lines)
           stream = StringIO.new(text)
           ::Ci::Ansi2html.convert(stream).html
         end
@@ -88,13 +88,13 @@ module Gitlab
 
         private
 
-        def read_last_lines(max_lines)
+        def read_last_lines(last_lines)
           chunks = []
           pos = lines = 0
           max = stream.size
 
           # We want an extra line to make sure fist line has full contents
-          while lines <= max_lines && pos < max
+          while lines <= last_lines && pos < max
             pos += BUFFER_SIZE
 
             buf =
@@ -110,7 +110,7 @@ module Gitlab
             chunks.unshift(buf)
           end
 
-          chunks.join.lines.last(max_lines).join
+          chunks.join.lines.last(last_lines).join
             .force_encoding(Encoding.default_external)
         end
       end
diff --git a/spec/features/projects/builds_spec.rb b/spec/features/projects/builds_spec.rb
index 2116721b224..0ff98b34c30 100644
--- a/spec/features/projects/builds_spec.rb
+++ b/spec/features/projects/builds_spec.rb
@@ -205,7 +205,9 @@ feature 'Builds', :feature do
         it 'loads job trace' do
           expect(page).to have_content 'BUILD TRACE'
 
-          build.append_trace(' and more trace', 11)
+          build.trace.write do |stream|
+            stream.append(' and more trace', 11)
+          end
 
           expect(page).to have_content 'BUILD TRACE and more trace'
         end
@@ -390,7 +392,7 @@ feature 'Builds', :feature do
         it 'sends the right headers' do
           expect(page.status_code).to eq(200)
           expect(page.response_headers['Content-Type']).to eq('text/plain; charset=utf-8')
-          expect(page.response_headers['X-Sendfile']).to eq(build.path_to_trace)
+          expect(page.response_headers['X-Sendfile']).to eq(build.trace.current_path)
         end
       end
 
diff --git a/spec/lib/ci/ansi2html_spec.rb b/spec/lib/ci/ansi2html_spec.rb
index 0762fd7e56a..a5dfb49478a 100644
--- a/spec/lib/ci/ansi2html_spec.rb
+++ b/spec/lib/ci/ansi2html_spec.rb
@@ -1,159 +1,160 @@
 require 'spec_helper'
 
 describe Ci::Ansi2html, lib: true do
-  subject { Ci::Ansi2html }
+  subject { described_class }
 
   it "prints non-ansi as-is" do
-    expect(subject.convert("Hello")[:html]).to eq('Hello')
+    expect(convert_html("Hello")).to eq('Hello')
   end
 
   it "strips non-color-changing controll sequences" do
-    expect(subject.convert("Hello \e[2Kworld")[:html]).to eq('Hello world')
+    expect(convert_html("Hello \e[2Kworld")).to eq('Hello world')
   end
 
   it "prints simply red" do
-    expect(subject.convert("\e[31mHello\e[0m")[:html]).to eq('<span class="term-fg-red">Hello</span>')
+    expect(convert_html("\e[31mHello\e[0m")).to eq('<span class="term-fg-red">Hello</span>')
   end
 
   it "prints simply red without trailing reset" do
-    expect(subject.convert("\e[31mHello")[:html]).to eq('<span class="term-fg-red">Hello</span>')
+    expect(convert_html("\e[31mHello")).to eq('<span class="term-fg-red">Hello</span>')
   end
 
   it "prints simply yellow" do
-    expect(subject.convert("\e[33mHello\e[0m")[:html]).to eq('<span class="term-fg-yellow">Hello</span>')
+    expect(convert_html("\e[33mHello\e[0m")).to eq('<span class="term-fg-yellow">Hello</span>')
   end
 
   it "prints default on blue" do
-    expect(subject.convert("\e[39;44mHello")[:html]).to eq('<span class="term-bg-blue">Hello</span>')
+    expect(convert_html("\e[39;44mHello")).to eq('<span class="term-bg-blue">Hello</span>')
   end
 
   it "prints red on blue" do
-    expect(subject.convert("\e[31;44mHello")[:html]).to eq('<span class="term-fg-red term-bg-blue">Hello</span>')
+    expect(convert_html("\e[31;44mHello")).to eq('<span class="term-fg-red term-bg-blue">Hello</span>')
   end
 
   it "resets colors after red on blue" do
-    expect(subject.convert("\e[31;44mHello\e[0m world")[:html]).to eq('<span class="term-fg-red term-bg-blue">Hello</span> world')
+    expect(convert_html("\e[31;44mHello\e[0m world")).to eq('<span class="term-fg-red term-bg-blue">Hello</span> world')
   end
 
   it "performs color change from red/blue to yellow/blue" do
-    expect(subject.convert("\e[31;44mHello \e[33mworld")[:html]).to eq('<span class="term-fg-red term-bg-blue">Hello </span><span class="term-fg-yellow term-bg-blue">world</span>')
+    expect(convert_html("\e[31;44mHello \e[33mworld")).to eq('<span class="term-fg-red term-bg-blue">Hello </span><span class="term-fg-yellow term-bg-blue">world</span>')
   end
 
   it "performs color change from red/blue to yellow/green" do
-    expect(subject.convert("\e[31;44mHello \e[33;42mworld")[:html]).to eq('<span class="term-fg-red term-bg-blue">Hello </span><span class="term-fg-yellow term-bg-green">world</span>')
+    expect(convert_html("\e[31;44mHello \e[33;42mworld")).to eq('<span class="term-fg-red term-bg-blue">Hello </span><span class="term-fg-yellow term-bg-green">world</span>')
   end
 
   it "performs color change from red/blue to reset to yellow/green" do
-    expect(subject.convert("\e[31;44mHello\e[0m \e[33;42mworld")[:html]).to eq('<span class="term-fg-red term-bg-blue">Hello</span> <span class="term-fg-yellow term-bg-green">world</span>')
+    expect(convert_html("\e[31;44mHello\e[0m \e[33;42mworld")).to eq('<span class="term-fg-red term-bg-blue">Hello</span> <span class="term-fg-yellow term-bg-green">world</span>')
   end
 
   it "ignores unsupported codes" do
-    expect(subject.convert("\e[51mHello\e[0m")[:html]).to eq('Hello')
+    expect(convert_html("\e[51mHello\e[0m")).to eq('Hello')
   end
 
   it "prints light red" do
-    expect(subject.convert("\e[91mHello\e[0m")[:html]).to eq('<span class="term-fg-l-red">Hello</span>')
+    expect(convert_html("\e[91mHello\e[0m")).to eq('<span class="term-fg-l-red">Hello</span>')
   end
 
   it "prints default on light red" do
-    expect(subject.convert("\e[101mHello\e[0m")[:html]).to eq('<span class="term-bg-l-red">Hello</span>')
+    expect(convert_html("\e[101mHello\e[0m")).to eq('<span class="term-bg-l-red">Hello</span>')
   end
 
   it "performs color change from red/blue to default/blue" do
-    expect(subject.convert("\e[31;44mHello \e[39mworld")[:html]).to eq('<span class="term-fg-red term-bg-blue">Hello </span><span class="term-bg-blue">world</span>')
+    expect(convert_html("\e[31;44mHello \e[39mworld")).to eq('<span class="term-fg-red term-bg-blue">Hello </span><span class="term-bg-blue">world</span>')
   end
 
   it "performs color change from light red/blue to default/blue" do
-    expect(subject.convert("\e[91;44mHello \e[39mworld")[:html]).to eq('<span class="term-fg-l-red term-bg-blue">Hello </span><span class="term-bg-blue">world</span>')
+    expect(convert_html("\e[91;44mHello \e[39mworld")).to eq('<span class="term-fg-l-red term-bg-blue">Hello </span><span class="term-bg-blue">world</span>')
   end
 
   it "prints bold text" do
-    expect(subject.convert("\e[1mHello")[:html]).to eq('<span class="term-bold">Hello</span>')
+    expect(convert_html("\e[1mHello")).to eq('<span class="term-bold">Hello</span>')
   end
 
   it "resets bold text" do
-    expect(subject.convert("\e[1mHello\e[21m world")[:html]).to eq('<span class="term-bold">Hello</span> world')
-    expect(subject.convert("\e[1mHello\e[22m world")[:html]).to eq('<span class="term-bold">Hello</span> world')
+    expect(convert_html("\e[1mHello\e[21m world")).to eq('<span class="term-bold">Hello</span> world')
+    expect(convert_html("\e[1mHello\e[22m world")).to eq('<span class="term-bold">Hello</span> world')
   end
 
   it "prints italic text" do
-    expect(subject.convert("\e[3mHello")[:html]).to eq('<span class="term-italic">Hello</span>')
+    expect(convert_html("\e[3mHello")).to eq('<span class="term-italic">Hello</span>')
   end
 
   it "resets italic text" do
-    expect(subject.convert("\e[3mHello\e[23m world")[:html]).to eq('<span class="term-italic">Hello</span> world')
+    expect(convert_html("\e[3mHello\e[23m world")).to eq('<span class="term-italic">Hello</span> world')
   end
 
   it "prints underlined text" do
-    expect(subject.convert("\e[4mHello")[:html]).to eq('<span class="term-underline">Hello</span>')
+    expect(convert_html("\e[4mHello")).to eq('<span class="term-underline">Hello</span>')
   end
 
   it "resets underlined text" do
-    expect(subject.convert("\e[4mHello\e[24m world")[:html]).to eq('<span class="term-underline">Hello</span> world')
+    expect(convert_html("\e[4mHello\e[24m world")).to eq('<span class="term-underline">Hello</span> world')
   end
 
   it "prints concealed text" do
-    expect(subject.convert("\e[8mHello")[:html]).to eq('<span class="term-conceal">Hello</span>')
+    expect(convert_html("\e[8mHello")).to eq('<span class="term-conceal">Hello</span>')
   end
 
   it "resets concealed text" do
-    expect(subject.convert("\e[8mHello\e[28m world")[:html]).to eq('<span class="term-conceal">Hello</span> world')
+    expect(convert_html("\e[8mHello\e[28m world")).to eq('<span class="term-conceal">Hello</span> world')
   end
 
   it "prints crossed-out text" do
-    expect(subject.convert("\e[9mHello")[:html]).to eq('<span class="term-cross">Hello</span>')
+    expect(convert_html("\e[9mHello")).to eq('<span class="term-cross">Hello</span>')
   end
 
   it "resets crossed-out text" do
-    expect(subject.convert("\e[9mHello\e[29m world")[:html]).to eq('<span class="term-cross">Hello</span> world')
+    expect(convert_html("\e[9mHello\e[29m world")).to eq('<span class="term-cross">Hello</span> world')
   end
 
   it "can print 256 xterm fg colors" do
-    expect(subject.convert("\e[38;5;16mHello")[:html]).to eq('<span class="xterm-fg-16">Hello</span>')
+    expect(convert_html("\e[38;5;16mHello")).to eq('<span class="xterm-fg-16">Hello</span>')
   end
 
   it "can print 256 xterm fg colors on normal magenta background" do
-    expect(subject.convert("\e[38;5;16;45mHello")[:html]).to eq('<span class="xterm-fg-16 term-bg-magenta">Hello</span>')
+    expect(convert_html("\e[38;5;16;45mHello")).to eq('<span class="xterm-fg-16 term-bg-magenta">Hello</span>')
   end
 
   it "can print 256 xterm bg colors" do
-    expect(subject.convert("\e[48;5;240mHello")[:html]).to eq('<span class="xterm-bg-240">Hello</span>')
+    expect(convert_html("\e[48;5;240mHello")).to eq('<span class="xterm-bg-240">Hello</span>')
   end
 
   it "can print 256 xterm bg colors on normal magenta foreground" do
-    expect(subject.convert("\e[48;5;16;35mHello")[:html]).to eq('<span class="term-fg-magenta xterm-bg-16">Hello</span>')
+    expect(convert_html("\e[48;5;16;35mHello")).to eq('<span class="term-fg-magenta xterm-bg-16">Hello</span>')
   end
 
   it "prints bold colored text vividly" do
-    expect(subject.convert("\e[1;31mHello\e[0m")[:html]).to eq('<span class="term-fg-l-red term-bold">Hello</span>')
+    expect(convert_html("\e[1;31mHello\e[0m")).to eq('<span class="term-fg-l-red term-bold">Hello</span>')
   end
 
   it "prints bold light colored text correctly" do
-    expect(subject.convert("\e[1;91mHello\e[0m")[:html]).to eq('<span class="term-fg-l-red term-bold">Hello</span>')
+    expect(convert_html("\e[1;91mHello\e[0m")).to eq('<span class="term-fg-l-red term-bold">Hello</span>')
   end
 
   it "prints &lt;" do
-    expect(subject.convert("<")[:html]).to eq('&lt;')
+    expect(convert_html("<")).to eq('&lt;')
   end
 
   it "replaces newlines with line break tags" do
-    expect(subject.convert("\n")[:html]).to eq('<br>')
+    expect(convert_html("\n")).to eq('<br>')
   end
 
   it "groups carriage returns with newlines" do
-    expect(subject.convert("\r\n")[:html]).to eq('<br>')
+    expect(convert_html("\r\n")).to eq('<br>')
   end
 
   describe "incremental update" do
     shared_examples 'stateable converter' do
-      let(:pass1) { subject.convert(pre_text) }
-      let(:pass2) { subject.convert(pre_text + text, pass1[:state]) }
+      let(:pass1_stream) { StringIO.new(pre_text) }
+      let(:pass2_stream) { StringIO.new(pre_text + text) }
+      let(:pass1) { subject.convert(pass1_stream) }
+      let(:pass2) { subject.convert(pass2_stream, pass1.state) }
 
       it "to returns html to append" do
-        expect(pass2[:append]).to be_truthy
-        expect(pass2[:html]).to eq(html)
-        expect(pass1[:text] + pass2[:text]).to eq(pre_text + text)
-        expect(pass1[:html] + pass2[:html]).to eq(pre_html + html)
+        expect(pass2.append).to be_truthy
+        expect(pass2.html).to eq(html)
+        expect(pass1.html + pass2.html).to eq(pre_html + html)
       end
     end
 
@@ -193,4 +194,27 @@ describe Ci::Ansi2html, lib: true do
       it_behaves_like 'stateable converter'
     end
   end
+
+  describe "truncates" do
+    let(:text) { "Hello World" }
+    let(:stream) { StringIO.new(text) }
+    let(:subject) { described_class.convert(stream) }
+
+    before do
+      stream.seek(3, IO::SEEK_SET)
+    end
+
+    it "returns truncated output" do
+      expect(subject.truncated).to be_truthy
+    end
+
+    it "does not append output" do
+      expect(subject.append).to be_falsey
+    end
+  end
+
+  def convert_html(data)
+    stream = StringIO.new(data)
+    subject.convert(stream).html
+  end
 end
diff --git a/spec/lib/gitlab/ci/trace/stream_spec.rb b/spec/lib/gitlab/ci/trace/stream_spec.rb
new file mode 100644
index 00000000000..0fe149defc1
--- /dev/null
+++ b/spec/lib/gitlab/ci/trace/stream_spec.rb
@@ -0,0 +1,61 @@
+require 'spec_helper'
+
+describe Gitlab::Ci::Trace::Stream do
+
+  describe '#raw' do
+    context 'limit max lines' do
+      let(:path) { __FILE__ }
+      let(:lines) { File.readlines(path) }
+
+      before do
+        allow(described_class).to receive(:LIMIT_SIZE).and_return(random_buffer)
+      end
+
+      subject do
+        described_class.new do
+          File.open(path)
+        end
+      end
+
+      it 'returns last few lines' do
+        10.times do
+          last_lines = random_lines
+
+          expected = lines.last(last_lines).join
+          result = subject.raw(last_lines: last_lines)
+
+          expect(result).to eq(expected)
+          expect(result.encoding).to eq(Encoding.default_external)
+        end
+      end
+
+      it 'returns everything if trying to get too many lines' do
+        result = subject.raw(last_lines: lines.size * 2)
+
+        expect(result).to eq(lines.join)
+        expect(result.encoding).to eq(Encoding.default_external)
+      end
+
+      it 'returns all contents if last_lines is not specified' do
+        result = subject.raw
+
+        expect(result).to eq(lines.join)
+        expect(result.encoding).to eq(Encoding.default_external)
+      end
+
+      it 'raises an error if not passing an integer for last_lines' do
+        expect do
+          subject.raw(max_lines: lines)
+        end.to raise_error(ArgumentError)
+      end
+
+      def random_lines
+        Random.rand(lines.size) + 1
+      end
+
+      def random_buffer
+        Random.rand(subject.size) + 1
+      end
+    end
+  end
+end
diff --git a/spec/lib/gitlab/ci/trace_reader_spec.rb b/spec/lib/gitlab/ci/trace_reader_spec.rb
deleted file mode 100644
index bec08bc2af0..00000000000
--- a/spec/lib/gitlab/ci/trace_reader_spec.rb
+++ /dev/null
@@ -1,52 +0,0 @@
-require 'spec_helper'
-
-describe Gitlab::Ci::Trace do
-  let(:path) { __FILE__ }
-  let(:lines) { File.readlines(path) }
-  let(:bytesize) { lines.sum(&:bytesize) }
-
-  it 'returns last few lines' do
-    10.times do
-      subject = build_subject
-      last_lines = random_lines
-
-      expected = lines.last(last_lines).join
-      result = subject.read(last_lines: last_lines)
-
-      expect(result).to eq(expected)
-      expect(result.encoding).to eq(Encoding.default_external)
-    end
-  end
-
-  it 'returns everything if trying to get too many lines' do
-    result = build_subject.read(last_lines: lines.size * 2)
-
-    expect(result).to eq(lines.join)
-    expect(result.encoding).to eq(Encoding.default_external)
-  end
-
-  it 'returns all contents if last_lines is not specified' do
-    result = build_subject.read
-
-    expect(result).to eq(lines.join)
-    expect(result.encoding).to eq(Encoding.default_external)
-  end
-
-  it 'raises an error if not passing an integer for last_lines' do
-    expect do
-      build_subject.read(last_lines: lines)
-    end.to raise_error(ArgumentError)
-  end
-
-  def random_lines
-    Random.rand(lines.size) + 1
-  end
-
-  def random_buffer
-    Random.rand(bytesize) + 1
-  end
-
-  def build_subject
-    described_class.new(__FILE__, buffer_size: random_buffer)
-  end
-end
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index 8dbcf50ee0c..e8a9336d4ab 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -17,8 +17,9 @@ describe Ci::Build, :models do
   it { is_expected.to belong_to(:trigger_request) }
   it { is_expected.to belong_to(:erased_by) }
   it { is_expected.to have_many(:deployments) }
-  it { is_expected.to validate_presence_of :ref }
-  it { is_expected.to respond_to :trace_html }
+  it { is_expected.to validate_presence_of(:ref) }
+  it { is_expected.to respond_to(:has_trace?) }
+  it { is_expected.to respond_to(:trace) }
 
   describe '#actionize' do
     context 'when build is a created' do
@@ -79,14 +80,14 @@ describe Ci::Build, :models do
   end
 
   describe '#append_trace' do
-    subject { build.trace_html }
+    subject { build.trace.html }
 
     context 'when build.trace hides runners token' do
       let(:token) { 'my_secret_token' }
 
       before do
         build.project.update(runners_token: token)
-        build.append_trace(token, 0)
+        build.trace.append(token, 0)
       end
 
       it { is_expected.not_to include(token) }
@@ -97,7 +98,7 @@ describe Ci::Build, :models do
 
       before do
         build.update(token: token)
-        build.append_trace(token, 0)
+        build.trace.append(token, 0)
       end
 
       it { is_expected.not_to include(token) }
@@ -272,11 +273,14 @@ describe Ci::Build, :models do
 
   describe '#update_coverage' do
     context "regarding coverage_regex's value," do
-      it "saves the correct extracted coverage value" do
+      before do
         build.coverage_regex = '\(\d+.\d+\%\) covered'
-        allow(build).to receive(:trace) { 'Coverage 1033 / 1051 LOC (98.29%) covered' }
-        expect(build).to receive(:update_attributes).with(coverage: 98.29) { true }
-        expect(build.update_coverage).to be true
+        build.trace.set('Coverage 1033 / 1051 LOC (98.29%) covered')
+      end
+
+      it "saves the correct extracted coverage value" do
+        expect(build.update_coverage).to be(true)
+        expect(build.coverage).to eq(98.29)
       end
     end
   end
@@ -438,7 +442,7 @@ describe Ci::Build, :models do
       end
 
       it 'erases build trace in trace file' do
-        expect(build.trace).to be_empty
+        expect(build).not_to have_trace
       end
 
       it 'sets erased to true' do
@@ -1078,7 +1082,7 @@ describe Ci::Build, :models do
     it 'obfuscates project runners token' do
       allow(build).to receive(:raw_trace).and_return("Test: #{build.project.runners_token}")
 
-      expect(build.trace).to eq("Test: xxxxxxxxxxxxxxxxxxxx")
+      expect(build.trace.raw).to eq("Test: xxxxxxxxxxxxxxxxxxxx")
     end
 
     it 'empty project runners token' do
@@ -1086,12 +1090,12 @@ describe Ci::Build, :models do
       # runners_token can't normally be set to nil
       allow(build.project).to receive(:runners_token).and_return(nil)
 
-      expect(build.trace).to eq(test_trace)
+      expect(build.trace.raw).to eq(test_trace)
     end
 
     context 'when build does not have trace' do
       it 'is is empty' do
-        expect(build.trace).to be_nil
+        expect(build.trace.raw).to be_nil
       end
     end
 
@@ -1101,7 +1105,7 @@ describe Ci::Build, :models do
         build.trace = text
       end
 
-      it { expect(build.trace).to eq(text) }
+      it { expect(build.trace.raw).to eq(text) }
     end
 
     context 'when trace hides runners token' do
@@ -1112,7 +1116,7 @@ describe Ci::Build, :models do
         build.project.update(runners_token: token)
       end
 
-      it { expect(build.trace).not_to include(token) }
+      it { expect(build.trace.raw).not_to include(token) }
       it { expect(build.raw_trace).to include(token) }
     end
 
@@ -1124,7 +1128,7 @@ describe Ci::Build, :models do
         build.update(token: token)
       end
 
-      it { expect(build.trace).not_to include(token) }
+      it { expect(build.trace.raw).not_to include(token) }
       it { expect(build.raw_trace).to include(token) }
     end
   end
@@ -1150,7 +1154,7 @@ describe Ci::Build, :models do
   describe '#has_trace_file?' do
     context 'when there is no trace' do
       it { expect(build.has_trace_file?).to be_falsey }
-      it { expect(build.trace).to be_nil }
+      it { expect(build.trace.raw).to be_nil }
     end
 
     context 'when there is a trace' do
@@ -1158,7 +1162,7 @@ describe Ci::Build, :models do
         let(:build_with_trace) { create(:ci_build, :trace) }
 
         it { expect(build_with_trace.has_trace_file?).to be_truthy }
-        it { expect(build_with_trace.trace).to eq('BUILD TRACE') }
+        it { expect(build_with_trace.trace.raw).to eq('BUILD TRACE') }
       end
 
       context 'when trace is stored in old file' do
@@ -1170,7 +1174,7 @@ describe Ci::Build, :models do
         end
 
         it { expect(build.has_trace_file?).to be_truthy }
-        it { expect(build.trace).to eq(test_trace) }
+        it { expect(build.trace.raw).to eq(test_trace) }
       end
 
       context 'when trace is stored in DB' do
@@ -1182,7 +1186,7 @@ describe Ci::Build, :models do
         end
 
         it { expect(build.has_trace_file?).to be_falsey }
-        it { expect(build.trace).to eq(test_trace) }
+        it { expect(build.trace.raw).to eq(test_trace) }
       end
     end
   end
diff --git a/spec/requests/api/jobs_spec.rb b/spec/requests/api/jobs_spec.rb
index 9450701064b..d8a56c02a63 100644
--- a/spec/requests/api/jobs_spec.rb
+++ b/spec/requests/api/jobs_spec.rb
@@ -320,7 +320,7 @@ describe API::Jobs, api: true do
     context 'authorized user' do
       it 'returns specific job trace' do
         expect(response).to have_http_status(200)
-        expect(response.body).to eq(build.trace)
+        expect(response.body).to eq(build.trace.raw)
       end
     end
 
@@ -408,7 +408,7 @@ describe API::Jobs, api: true do
 
       it 'erases job content' do
         expect(response).to have_http_status(201)
-        expect(build.trace).to be_empty
+        expect(build).not_to have_trace
         expect(build.artifacts_file.exists?).to be_falsy
         expect(build.artifacts_metadata.exists?).to be_falsy
       end
diff --git a/spec/requests/api/runner_spec.rb b/spec/requests/api/runner_spec.rb
index 044b989e5ba..8afaadd6bcd 100644
--- a/spec/requests/api/runner_spec.rb
+++ b/spec/requests/api/runner_spec.rb
@@ -569,7 +569,7 @@ describe API::Runner do
           update_job(trace: 'BUILD TRACE UPDATED')
 
           expect(response).to have_http_status(200)
-          expect(job.reload.trace).to eq 'BUILD TRACE UPDATED'
+          expect(job.reload.trace.raw).to eq 'BUILD TRACE UPDATED'
         end
       end
 
@@ -577,7 +577,7 @@ describe API::Runner do
         it 'does not override trace information' do
           update_job
 
-          expect(job.reload.trace).to eq 'BUILD TRACE'
+          expect(job.reload.trace.raw).to eq 'BUILD TRACE'
         end
       end
 
@@ -608,7 +608,7 @@ describe API::Runner do
       context 'when request is valid' do
         it 'gets correct response' do
           expect(response.status).to eq 202
-          expect(job.reload.trace).to eq 'BUILD TRACE appended'
+          expect(job.reload.trace.raw).to eq 'BUILD TRACE appended'
           expect(response.header).to have_key 'Range'
           expect(response.header).to have_key 'Job-Status'
         end
@@ -619,7 +619,7 @@ describe API::Runner do
           it "changes the job's trace" do
             patch_the_trace
 
-            expect(job.reload.trace).to eq 'BUILD TRACE appended appended'
+            expect(job.reload.trace.raw).to eq 'BUILD TRACE appended appended'
           end
 
           context 'when Runner makes a force-patch' do
@@ -628,7 +628,7 @@ describe API::Runner do
             it "doesn't change the build.trace" do
               force_patch_the_trace
 
-              expect(job.reload.trace).to eq 'BUILD TRACE appended'
+              expect(job.reload.trace.raw).to eq 'BUILD TRACE appended'
             end
           end
         end
@@ -641,7 +641,7 @@ describe API::Runner do
           it 'changes the job.trace' do
             patch_the_trace
 
-            expect(job.reload.trace).to eq 'BUILD TRACE appended appended'
+            expect(job.reload.trace.raw).to eq 'BUILD TRACE appended appended'
           end
 
           context 'when Runner makes a force-patch' do
@@ -650,7 +650,7 @@ describe API::Runner do
             it "doesn't change the job.trace" do
               force_patch_the_trace
 
-              expect(job.reload.trace).to eq 'BUILD TRACE appended'
+              expect(job.reload.trace.raw).to eq 'BUILD TRACE appended'
             end
           end
         end
@@ -675,7 +675,7 @@ describe API::Runner do
 
         it 'gets correct response' do
           expect(response.status).to eq 202
-          expect(job.reload.trace).to eq 'BUILD TRACE appended'
+          expect(job.reload.trace.raw).to eq 'BUILD TRACE appended'
           expect(response.header).to have_key 'Range'
           expect(response.header).to have_key 'Job-Status'
         end
@@ -715,9 +715,11 @@ describe API::Runner do
 
       def patch_the_trace(content = ' appended', request_headers = nil)
         unless request_headers
-          offset = job.trace_length
-          limit = offset + content.length - 1
-          request_headers = headers.merge({ 'Content-Range' => "#{offset}-#{limit}" })
+          job.trace.read do |stream|
+            offset = stream.size
+            limit = offset + content.length - 1
+            request_headers = headers.merge({ 'Content-Range' => "#{offset}-#{limit}" })
+          end
         end
 
         Timecop.travel(job.updated_at + update_interval) do
diff --git a/spec/requests/api/v3/builds_spec.rb b/spec/requests/api/v3/builds_spec.rb
index a50c22a6dd1..e97d2b0cee0 100644
--- a/spec/requests/api/v3/builds_spec.rb
+++ b/spec/requests/api/v3/builds_spec.rb
@@ -330,7 +330,7 @@ describe API::V3::Builds, api: true do
     context 'authorized user' do
       it 'returns specific job trace' do
         expect(response).to have_http_status(200)
-        expect(response.body).to eq(build.trace)
+        expect(response.body).to eq(build.trace.raw)
       end
     end
 
@@ -418,7 +418,7 @@ describe API::V3::Builds, api: true do
 
       it 'erases job content' do
         expect(response.status).to eq 201
-        expect(build.trace).to be_empty
+        expect(build).not_to have_trace
         expect(build.artifacts_file.exists?).to be_falsy
         expect(build.artifacts_metadata.exists?).to be_falsy
       end
diff --git a/spec/requests/ci/api/builds_spec.rb b/spec/requests/ci/api/builds_spec.rb
index c879f37f50d..ef30d8638dd 100644
--- a/spec/requests/ci/api/builds_spec.rb
+++ b/spec/requests/ci/api/builds_spec.rb
@@ -285,7 +285,7 @@ describe Ci::API::Builds do
       end
 
       it 'does not override trace information when no trace is given' do
-        expect(build.reload.trace).to eq 'BUILD TRACE'
+        expect(build.reload.trace.raw).to eq 'BUILD TRACE'
       end
 
       context 'job has been erased' do
@@ -309,9 +309,11 @@ describe Ci::API::Builds do
 
       def patch_the_trace(content = ' appended', request_headers = nil)
         unless request_headers
-          offset = build.trace_length
-          limit = offset + content.length - 1
-          request_headers = headers.merge({ 'Content-Range' => "#{offset}-#{limit}" })
+          build.trace.read do |stream|
+            offset = stream.size
+            limit = offset + content.length - 1
+            request_headers = headers.merge({ 'Content-Range' => "#{offset}-#{limit}" })
+          end
         end
 
         Timecop.travel(build.updated_at + update_interval) do
@@ -335,7 +337,7 @@ describe Ci::API::Builds do
       context 'when request is valid' do
         it 'gets correct response' do
           expect(response.status).to eq 202
-          expect(build.reload.trace).to eq 'BUILD TRACE appended'
+          expect(build.reload.trace.raw).to eq 'BUILD TRACE appended'
           expect(response.header).to have_key 'Range'
           expect(response.header).to have_key 'Build-Status'
         end
@@ -346,7 +348,7 @@ describe Ci::API::Builds do
           it 'changes the build trace' do
             patch_the_trace
 
-            expect(build.reload.trace).to eq 'BUILD TRACE appended appended'
+            expect(build.reload.trace.raw).to eq 'BUILD TRACE appended appended'
           end
 
           context 'when Runner makes a force-patch' do
@@ -355,7 +357,7 @@ describe Ci::API::Builds do
             it "doesn't change the build.trace" do
               force_patch_the_trace
 
-              expect(build.reload.trace).to eq 'BUILD TRACE appended'
+              expect(build.reload.trace.raw).to eq 'BUILD TRACE appended'
             end
           end
         end
@@ -368,7 +370,7 @@ describe Ci::API::Builds do
           it 'changes the build.trace' do
             patch_the_trace
 
-            expect(build.reload.trace).to eq 'BUILD TRACE appended appended'
+            expect(build.reload.trace.raw).to eq 'BUILD TRACE appended appended'
           end
 
           context 'when Runner makes a force-patch' do
@@ -377,7 +379,7 @@ describe Ci::API::Builds do
             it "doesn't change the build.trace" do
               force_patch_the_trace
 
-              expect(build.reload.trace).to eq 'BUILD TRACE appended'
+              expect(build.reload.trace.raw).to eq 'BUILD TRACE appended'
             end
           end
         end
@@ -403,7 +405,7 @@ describe Ci::API::Builds do
 
         it 'gets correct response' do
           expect(response.status).to eq 202
-          expect(build.reload.trace).to eq 'BUILD TRACE appended'
+          expect(build.reload.trace.raw).to eq 'BUILD TRACE appended'
           expect(response.header).to have_key 'Range'
           expect(response.header).to have_key 'Build-Status'
         end