Optimise trace handling code to use streaming instead of full read

12 files changed, 629 insertions, 383 deletions

diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb
index 87a0c95c4dc..b62def83ee4 100644
--- a/spec/factories/ci/builds.rb
+++ b/spec/factories/ci/builds.rb
@@ -111,7 +111,7 @@ FactoryGirl.define do
 
     trait :trace do
       after(:create) do |build, evaluator|
-        build.trace = 'BUILD TRACE'
+        build.trace.set('BUILD TRACE')
       end
     end
 
diff --git a/spec/features/projects/builds_spec.rb b/spec/features/projects/builds_spec.rb
index 2116721b224..ab10434e10c 100644
--- a/spec/features/projects/builds_spec.rb
+++ b/spec/features/projects/builds_spec.rb
@@ -205,21 +205,13 @@ 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
       end
-
-      context 'when build does not have an initial trace' do
-        let(:build) { create(:ci_build, pipeline: pipeline) }
-
-        it 'loads new trace' do
-          build.append_trace('build trace', 0)
-
-          expect(page).to have_content 'build trace'
-        end
-      end
     end
 
     feature 'Variables' do
@@ -390,7 +382,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.send(:current_path))
         end
       end
 
@@ -409,43 +401,24 @@ feature 'Builds', :feature do
 
     context 'storage form' do
       let(:existing_file) { Tempfile.new('existing-trace-file').path }
-      let(:non_existing_file) do
-        file = Tempfile.new('non-existing-trace-file')
-        path = file.path
-        file.unlink
-        path
-      end
 
-      context 'when build has trace in file' do
-        before do
-          Capybara.current_session.driver.header('X-Sendfile-Type', 'X-Sendfile')
-          build.run!
-          visit namespace_project_build_path(project.namespace, project, build)
+      before do
+        Capybara.current_session.driver.header('X-Sendfile-Type', 'X-Sendfile')
 
-          allow_any_instance_of(Project).to receive(:ci_id).and_return(nil)
-          allow_any_instance_of(Ci::Build).to receive(:path_to_trace).and_return(existing_file)
-          allow_any_instance_of(Ci::Build).to receive(:old_path_to_trace).and_return(non_existing_file)
+        build.run!
 
-          page.within('.js-build-sidebar') { click_link 'Raw' }
-        end
+        allow_any_instance_of(Gitlab::Ci::Trace).to receive(:paths)
+          .and_return(paths)
 
-        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(existing_file)
-        end
+        visit namespace_project_build_path(project.namespace, project, build)
       end
 
-      context 'when build has trace in old file' do
-        before do
-          Capybara.current_session.driver.header('X-Sendfile-Type', 'X-Sendfile')
-          build.run!
-          visit namespace_project_build_path(project.namespace, project, build)
-
-          allow_any_instance_of(Project).to receive(:ci_id).and_return(999)
-          allow_any_instance_of(Ci::Build).to receive(:path_to_trace).and_return(non_existing_file)
-          allow_any_instance_of(Ci::Build).to receive(:old_path_to_trace).and_return(existing_file)
+      context 'when build has trace in file' do
+        let(:paths) do
+          [existing_file]
+        end
 
+        before do
           page.within('.js-build-sidebar') { click_link 'Raw' }
         end
 
@@ -457,20 +430,10 @@ feature 'Builds', :feature do
       end
 
       context 'when build has trace in DB' do
-        before do
-          Capybara.current_session.driver.header('X-Sendfile-Type', 'X-Sendfile')
-          build.run!
-          visit namespace_project_build_path(project.namespace, project, build)
-
-          allow_any_instance_of(Project).to receive(:ci_id).and_return(nil)
-          allow_any_instance_of(Ci::Build).to receive(:path_to_trace).and_return(non_existing_file)
-          allow_any_instance_of(Ci::Build).to receive(:old_path_to_trace).and_return(non_existing_file)
-
-          page.within('.js-build-sidebar') { click_link 'Raw' }
-        end
+        let(:paths) { [] }
 
         it 'sends the right headers' do
-          expect(page.status_code).to eq(404)
+          expect(page.status_code).not_to have_link('Raw')
         end
       end
     end
diff --git a/spec/javascripts/build_spec.js b/spec/javascripts/build_spec.js
index beee6cb2969..cb48f14fd9a 100644
--- a/spec/javascripts/build_spec.js
+++ b/spec/javascripts/build_spec.js
@@ -72,12 +72,14 @@ describe('Build', () => {
       it('displays the initial build trace', () => {
         expect($.ajax.calls.count()).toBe(1);
         const [{ url, dataType, success, context }] = $.ajax.calls.argsFor(0);
-        expect(url).toBe(`${BUILD_URL}.json`);
+        expect(url).toBe(
+          `${BUILD_URL}/trace.json`,
+        );
         expect(dataType).toBe('json');
         expect(success).toEqual(jasmine.any(Function));
         spyOn(gl.utils, 'setCiStatusFavicon').and.callFake(() => {});
 
-        success.call(context, { trace_html: '<span>Example</span>', status: 'running' });
+        success.call(context, { html: '<span>Example</span>', status: 'running' });
 
         expect($('#build-trace .js-build-output').text()).toMatch(/Example/);
       });
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..f1a1a71c528
--- /dev/null
+++ b/spec/lib/gitlab/ci/trace/stream_spec.rb
@@ -0,0 +1,201 @@
+require 'spec_helper'
+
+describe Gitlab::Ci::Trace::Stream do
+  describe 'delegates' do
+    subject { described_class.new { nil } }
+
+    it { is_expected.to delegate_method(:close).to(:stream) }
+    it { is_expected.to delegate_method(:tell).to(:stream) }
+    it { is_expected.to delegate_method(:seek).to(:stream) }
+    it { is_expected.to delegate_method(:size).to(:stream) }
+    it { is_expected.to delegate_method(:path).to(:stream) }
+    it { is_expected.to delegate_method(:truncate).to(:stream) }
+    it { is_expected.to delegate_method(:valid?).to(:stream).as(:present?) }
+    it { is_expected.to delegate_method(:file?).to(:path).as(:present?) }
+  end
+
+  describe '#limit' do
+    let(:stream) do
+      described_class.new do
+        StringIO.new("12345678")
+      end
+    end
+
+    it 'if size is larger we start from beggining' do
+      stream.limit(10)
+
+      expect(stream.tell).to eq(0)
+    end
+
+    it 'if size is smaller we start from the end' do
+      stream.limit(2)
+
+      expect(stream.tell).to eq(6)
+    end
+  end
+
+  describe '#append' do
+    let(:stream) do
+      described_class.new do
+        StringIO.new("12345678")
+      end
+    end
+
+    it "truncates and append content" do
+      stream.append("89", 4)
+      stream.seek(0)
+
+      expect(stream.size).to eq(6)
+      expect(stream.raw).to eq("123489")
+    end
+  end
+
+  describe '#set' do
+    let(:stream) do
+      described_class.new do
+        StringIO.new("12345678")
+      end
+    end
+
+    before do
+      stream.set("8901")
+    end
+
+    it "overwrite content" do
+      stream.seek(0)
+
+      expect(stream.size).to eq(4)
+      expect(stream.raw).to eq("8901")
+    end
+  end
+
+  describe '#raw' do
+    let(:path) { __FILE__ }
+    let(:lines) { File.readlines(path) }
+    let(:stream) do
+      described_class.new do
+        File.open(path)
+      end
+    end
+
+    it 'returns all contents if last_lines is not specified' do
+      result = stream.raw
+
+      expect(result).to eq(lines.join)
+      expect(result.encoding).to eq(Encoding.default_external)
+    end
+
+    context 'limit max lines' do
+      before do
+        # specifying BUFFER_SIZE forces to seek backwards
+        allow(described_class).to receive(:BUFFER_SIZE)
+          .and_return(2)
+      end
+
+      it 'returns last few lines' do
+        result = stream.raw(last_lines: 2)
+
+        expect(result).to eq(lines.last(2).join)
+        expect(result.encoding).to eq(Encoding.default_external)
+      end
+
+      it 'returns everything if trying to get too many lines' do
+        result = stream.raw(last_lines: lines.size * 2)
+
+        expect(result).to eq(lines.join)
+        expect(result.encoding).to eq(Encoding.default_external)
+      end
+    end
+  end
+
+  describe '#html_with_state' do
+    let(:stream) do
+      described_class.new do
+        StringIO.new("1234")
+      end
+    end
+
+    it 'returns html content with state' do
+      result = stream.html_with_state
+
+      expect(result.html).to eq("1234")
+    end
+
+    context 'follow-up state' do
+      let!(:last_result) { stream.html_with_state }
+
+      before do
+        stream.append("5678", 4)
+        stream.seek(0)
+      end
+
+      it "returns appended trace" do
+        result = stream.html_with_state(last_result.state)
+
+        expect(result.append).to be_truthy
+        expect(result.html).to eq("5678")
+      end
+    end
+  end
+
+  describe '#html' do
+    let(:stream) do
+      described_class.new do
+        StringIO.new("12\n34\n56")
+      end
+    end
+
+    it "returns html" do
+      expect(stream.html).to eq("12<br>34<br>56")
+    end
+
+    it "returns html for last line only" do
+      expect(stream.html(last_lines: 1)).to eq("56")
+    end
+  end
+
+  describe '#extract_coverage' do
+    let(:stream) do
+      described_class.new do
+        StringIO.new(data)
+      end
+    end
+
+    subject { stream.extract_coverage(regex) }
+
+    context 'valid content & regex' do
+      let(:data) { 'Coverage 1033 / 1051 LOC (98.29%) covered' }
+      let(:regex) { '\(\d+.\d+\%\) covered' }
+
+      it { is_expected.to eq(98.29) }
+    end
+
+    context 'valid content & bad regex' do
+      let(:data) { 'Coverage 1033 / 1051 LOC (98.29%) covered\n' }
+      let(:regex) { 'very covered' }
+
+      it { is_expected.to be_nil }
+    end
+
+    context 'no coverage content & regex' do
+      let(:data) { 'No coverage for today :sad:' }
+      let(:regex) { '\(\d+.\d+\%\) covered' }
+
+      it { is_expected.to be_nil }
+    end
+
+    context 'multiple results in content & regex' do
+      let(:data) { ' (98.39%) covered. (98.29%) covered' }
+      let(:regex) { '\(\d+.\d+\%\) covered' }
+
+      it { is_expected.to eq(98.29) }
+    end
+
+    context 'using a regex capture' do
+      let(:data) { 'TOTAL      9926   3489    65%' }
+      let(:regex) { 'TOTAL\s+\d+\s+\d+\s+(\d{1,3}\%)' }
+
+      it { is_expected.to eq(65) }
+    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 ff5551bf703..00000000000
--- a/spec/lib/gitlab/ci/trace_reader_spec.rb
+++ /dev/null
@@ -1,52 +0,0 @@
-require 'spec_helper'
-
-describe Gitlab::Ci::TraceReader 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/lib/gitlab/ci/trace_spec.rb b/spec/lib/gitlab/ci/trace_spec.rb
new file mode 100644
index 00000000000..69e8dc9220d
--- /dev/null
+++ b/spec/lib/gitlab/ci/trace_spec.rb
@@ -0,0 +1,216 @@
+require 'spec_helper'
+
+describe Gitlab::Ci::Trace do
+  let(:build) { create(:ci_build) }
+  let(:trace) { described_class.new(build) }
+
+  describe "associations" do
+    it { expect(trace).to respond_to(:job) }
+    it { expect(trace).to delegate_method(:old_trace).to(:job) }
+  end
+
+  describe '#html' do
+    before do
+      trace.set("12\n34")
+    end
+
+    it "returns formatted html" do
+      expect(trace.html).to eq("12<br>34")
+    end
+
+    it "returns last line of formatted html" do
+      expect(trace.html(last_lines: 1)).to eq("34")
+    end
+  end
+
+  describe '#raw' do
+    before do
+      trace.set("12\n34")
+    end
+
+    it "returns raw output" do
+      expect(trace.raw).to eq("12\n34")
+    end
+
+    it "returns last line of raw output" do
+      expect(trace.raw(last_lines: 1)).to eq("34")
+    end
+  end
+
+  describe '#extract_coverage' do
+    let(:regex) { '\(\d+.\d+\%\) covered' }
+
+    before do
+      trace.set('Coverage 1033 / 1051 LOC (98.29%) covered')
+    end
+
+    it "returns valid coverage" do
+      expect(trace.extract_coverage(regex)).to eq(98.29)
+    end
+  end
+
+  describe '#set' do
+    before do
+      trace.set("12")
+    end
+
+    it "returns trace" do
+      expect(trace.raw).to eq("12")
+    end
+
+    context 'overwrite trace' do
+      before do
+        trace.set("34")
+      end
+
+      it "returns new trace" do
+        expect(trace.raw).to eq("34")
+      end
+    end
+
+    context 'runners token' do
+      let(:token) { 'my_secret_token' }
+
+      before do
+        build.project.update(runners_token: token)
+        trace.set(token)
+      end
+
+      it "hides token" do
+        expect(trace.raw).not_to include(token)
+      end
+    end
+
+    context 'hides build token' do
+      let(:token) { 'my_secret_token' }
+
+      before do
+        build.update(token: token)
+        trace.set(token)
+      end
+
+      it "hides token" do
+        expect(trace.raw).not_to include(token)
+      end
+    end
+  end
+
+  describe '#append' do
+    before do
+      trace.set("1234")
+    end
+
+    it "returns correct trace" do
+      expect(trace.append("56", 4)).to eq(6)
+      expect(trace.raw).to eq("123456")
+    end
+
+    context 'tries to append trace at different offset' do
+      it "fails with append" do
+        expect(trace.append("56", 2)).to eq(-4)
+        expect(trace.raw).to eq("1234")
+      end
+    end
+
+    context 'runners token' do
+      let(:token) { 'my_secret_token' }
+
+      before do
+        build.project.update(runners_token: token)
+        trace.append(token, 0)
+      end
+
+      it "hides token" do
+        expect(trace.raw).not_to include(token)
+      end
+    end
+
+    context 'build token' do
+      let(:token) { 'my_secret_token' }
+
+      before do
+        build.update(token: token)
+        trace.append(token, 0)
+      end
+
+      it "hides token" do
+        expect(trace.raw).not_to include(token)
+      end
+    end
+  end
+
+  describe 'trace handling' do
+    context 'trace does not exist' do
+      it { expect(trace.exist?).to be(false) }
+    end
+
+    context 'new trace path is used' do
+      before do
+        trace.send(:ensure_directory)
+
+        File.open(trace.send(:default_path), "w") do |file|
+          file.write("data")
+        end
+      end
+
+      it "trace exist" do
+        expect(trace.exist?).to be(true)
+      end
+
+      it "can be erased" do
+        trace.erase!
+        expect(trace.exist?).to be(false)
+      end
+    end
+
+    context 'deprecated path' do
+      let(:path) { trace.send(:deprecated_path) }
+
+      context 'with valid ci_id' do
+        before do
+          build.project.update(ci_id: 1000)
+
+          FileUtils.mkdir_p(File.dirname(path))
+
+          File.open(path, "w") do |file|
+            file.write("data")
+          end
+        end
+
+        it "trace exist" do
+          expect(trace.exist?).to be(true)
+        end
+
+        it "can be erased" do
+          trace.erase!
+          expect(trace.exist?).to be(false)
+        end
+      end
+
+      context 'without valid ci_id' do
+        it "does not return deprecated path" do
+          expect(path).to be_nil
+        end
+      end
+    end
+
+    context 'stored in database' do
+      before do
+        build.send(:write_attribute, :trace, "data")
+      end
+
+      it "trace exist" do
+        expect(trace.exist?).to be(true)
+      end
+
+      it "can be erased" do
+        trace.erase!
+        expect(trace.exist?).to be(false)
+      end
+
+      it "returns database data" do
+        expect(trace.raw).to eq("data")
+      end
+    end
+  end
+end
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index 8dbcf50ee0c..f876cb00fc7 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
@@ -78,32 +79,6 @@ describe Ci::Build, :models do
     end
   end
 
-  describe '#append_trace' do
-    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)
-      end
-
-      it { is_expected.not_to include(token) }
-    end
-
-    context 'when build.trace hides build token' do
-      let(:token) { 'my_secret_token' }
-
-      before do
-        build.update(token: token)
-        build.append_trace(token, 0)
-      end
-
-      it { is_expected.not_to include(token) }
-    end
-  end
-
   describe '#artifacts?' do
     subject { build.artifacts? }
 
@@ -272,12 +247,98 @@ 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
+
+  describe '#trace' do
+    subject { build.trace }
+
+    it { is_expected.to be_a(Gitlab::Ci::Trace) }
+  end
+
+  describe '#has_trace?' do
+    subject { build.has_trace? }
+
+    it "expect to call exist? method" do
+      expect_any_instance_of(Gitlab::Ci::Trace).to receive(:exist?)
+        .and_return(true)
+
+      is_expected.to be(true)
+    end
+  end
+
+  describe '#trace=' do
+    it "expect to fail trace=" do
+      expect { build.trace = "new" }.to raise_error(NotImplementedError)
+    end
+  end
+
+  describe '#old_trace' do
+    subject { build.old_trace }
+
+    before do
+      build.update_column(:trace, 'old trace')
+    end
+
+    it "expect to receive data from database" do
+      is_expected.to eq('old trace')
+    end
+  end
+
+  describe '#erase_old_trace!' do
+    subject { build.send(:read_attribute, :trace) }
+
+    before do
+      build.send(:write_attribute, :trace, 'old trace')
+    end
+
+    it "expect to receive data from database" do
+      build.erase_old_trace!
+
+      is_expected.to be_nil
+    end
+  end
+
+  describe '#hide_secrets' do
+    let(:subject) { build.hide_secrets(data) }
+
+    context 'hide runners token' do
+      let(:data) { 'new token data'}
+
+      before do
+        build.project.update(runners_token: 'token')
       end
+
+      it { is_expected.to eq('new xxxxx data') }
+    end
+
+    context 'hide build token' do
+      let(:data) { 'new token data'}
+
+      before do
+        build.update(token: 'token')
+      end
+
+      it { is_expected.to eq('new xxxxx data') }
+    end
+
+    context 'hide build token' do
+      let(:data) { 'new token data'}
+
+      before do
+        build.update(token: 'token')
+      end
+
+      it { is_expected.to eq('new xxxxx data') }
     end
   end
 
@@ -438,7 +499,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
@@ -532,38 +593,6 @@ describe Ci::Build, :models do
     end
   end
 
-  describe '#extract_coverage' do
-    context 'valid content & regex' do
-      subject { build.extract_coverage('Coverage 1033 / 1051 LOC (98.29%) covered', '\(\d+.\d+\%\) covered') }
-
-      it { is_expected.to eq(98.29) }
-    end
-
-    context 'valid content & bad regex' do
-      subject { build.extract_coverage('Coverage 1033 / 1051 LOC (98.29%) covered', 'very covered') }
-
-      it { is_expected.to be_nil }
-    end
-
-    context 'no coverage content & regex' do
-      subject { build.extract_coverage('No coverage for today :sad:', '\(\d+.\d+\%\) covered') }
-
-      it { is_expected.to be_nil }
-    end
-
-    context 'multiple results in content & regex' do
-      subject { build.extract_coverage(' (98.39%) covered. (98.29%) covered', '\(\d+.\d+\%\) covered') }
-
-      it { is_expected.to eq(98.29) }
-    end
-
-    context 'using a regex capture' do
-      subject { build.extract_coverage('TOTAL      9926   3489    65%', 'TOTAL\s+\d+\s+\d+\s+(\d{1,3}\%)') }
-
-      it { is_expected.to eq(65) }
-    end
-  end
-
   describe '#first_pending' do
     let!(:first) { create(:ci_build, pipeline: pipeline, status: 'pending', created_at: Date.yesterday) }
     let!(:second) { create(:ci_build, pipeline: pipeline, status: 'pending') }
@@ -983,32 +1012,6 @@ describe Ci::Build, :models do
     it { is_expected.to eq(project.name) }
   end
 
-  describe '#raw_trace' do
-    subject { build.raw_trace }
-
-    context 'when build.trace hides runners token' do
-      let(:token) { 'my_secret_token' }
-
-      before do
-        build.project.update(runners_token: token)
-        build.update(trace: token)
-      end
-
-      it { is_expected.not_to include(token) }
-    end
-
-    context 'when build.trace hides build token' do
-      let(:token) { 'my_secret_token' }
-
-      before do
-        build.update(token: token)
-        build.update(trace: token)
-      end
-
-      it { is_expected.not_to include(token) }
-    end
-  end
-
   describe '#ref_slug' do
     {
       'master'    => 'master',
@@ -1074,61 +1077,6 @@ describe Ci::Build, :models do
     end
   end
 
-  describe '#trace' 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")
-    end
-
-    it 'empty project runners token' do
-      allow(build).to receive(:raw_trace).and_return(test_trace)
-      # 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)
-    end
-
-    context 'when build does not have trace' do
-      it 'is is empty' do
-        expect(build.trace).to be_nil
-      end
-    end
-
-    context 'when trace contains text' do
-      let(:text) { 'example output' }
-      before do
-        build.trace = text
-      end
-
-      it { expect(build.trace).to eq(text) }
-    end
-
-    context 'when trace hides runners token' do
-      let(:token) { 'my_secret_token' }
-
-      before do
-        build.update(trace: token)
-        build.project.update(runners_token: token)
-      end
-
-      it { expect(build.trace).not_to include(token) }
-      it { expect(build.raw_trace).to include(token) }
-    end
-
-    context 'when build.trace hides build token' do
-      let(:token) { 'my_secret_token' }
-
-      before do
-        build.update(trace: token)
-        build.update(token: token)
-      end
-
-      it { expect(build.trace).not_to include(token) }
-      it { expect(build.raw_trace).to include(token) }
-    end
-  end
-
   describe '#has_expiring_artifacts?' do
     context 'when artifacts have expiration date set' do
       before { build.update(artifacts_expire_at: 1.day.from_now) }
@@ -1147,66 +1095,6 @@ describe Ci::Build, :models do
     end
   end
 
-  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 }
-    end
-
-    context 'when there is a trace' do
-      context 'when trace is stored in file' 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') }
-      end
-
-      context 'when trace is stored in old file' do
-        before do
-          allow(build.project).to receive(:ci_id).and_return(999)
-          allow(File).to receive(:exist?).with(build.path_to_trace).and_return(false)
-          allow(File).to receive(:exist?).with(build.old_path_to_trace).and_return(true)
-          allow(File).to receive(:read).with(build.old_path_to_trace).and_return(test_trace)
-        end
-
-        it { expect(build.has_trace_file?).to be_truthy }
-        it { expect(build.trace).to eq(test_trace) }
-      end
-
-      context 'when trace is stored in DB' do
-        before do
-          allow(build.project).to receive(:ci_id).and_return(nil)
-          allow(build).to receive(:read_attribute).with(:trace).and_return(test_trace)
-          allow(File).to receive(:exist?).with(build.path_to_trace).and_return(false)
-          allow(File).to receive(:exist?).with(build.old_path_to_trace).and_return(false)
-        end
-
-        it { expect(build.has_trace_file?).to be_falsey }
-        it { expect(build.trace).to eq(test_trace) }
-      end
-    end
-  end
-
-  describe '#trace_file_path' do
-    context 'when trace is stored in file' do
-      before do
-        allow(build).to receive(:has_trace_file?).and_return(true)
-        allow(build).to receive(:has_old_trace_file?).and_return(false)
-      end
-
-      it { expect(build.trace_file_path).to eq(build.path_to_trace) }
-    end
-
-    context 'when trace is stored in old file' do
-      before do
-        allow(build).to receive(:has_trace_file?).and_return(true)
-        allow(build).to receive(:has_old_trace_file?).and_return(true)
-      end
-
-      it { expect(build.trace_file_path).to eq(build.old_path_to_trace) }
-    end
-  end
-
   describe '#update_project_statistics' do
     let!(:build) { create(:ci_build, artifacts_size: 23) }
 
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 1cfac7353d4..409a59d6c23 100644
--- a/spec/requests/api/runner_spec.rb
+++ b/spec/requests/api/runner_spec.rb
@@ -592,7 +592,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
 
@@ -600,7 +600,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
 
@@ -631,7 +631,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
@@ -642,7 +642,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
@@ -651,7 +651,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
@@ -664,7 +664,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
@@ -673,7 +673,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
@@ -698,7 +698,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
@@ -738,9 +738,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