diff options
29 files changed, 988 insertions, 633 deletions
diff --git a/app/assets/javascripts/build.js b/app/assets/javascripts/build.js index fe54ecffdfe..7cb022c848a 100644 --- a/app/assets/javascripts/build.js +++ b/app/assets/javascripts/build.js @@ -84,10 +84,10 @@ window.Build = (function() { var removeRefreshStatuses = ['success', 'failed', 'canceled', 'skipped']; return $.ajax({ - url: this.buildUrl, + url: this.pageUrl + "/trace.json", dataType: 'json', success: function(buildData) { - $('.js-build-output').html(buildData.trace_html); + $('.js-build-output').html(buildData.html); gl.utils.setCiStatusFavicon(`${this.pageUrl}/status.json`); if (window.location.hash === DOWN_BUILD_TRACE) { $("html,body").scrollTop(this.$buildTrace.height()); diff --git a/app/controllers/projects/builds_controller.rb b/app/controllers/projects/builds_controller.rb index 3f3c90a49ab..add66ce9f84 100644 --- a/app/controllers/projects/builds_controller.rb +++ b/app/controllers/projects/builds_controller.rb @@ -31,25 +31,25 @@ class Projects::BuildsController < Projects::ApplicationController @builds = @project.pipelines.find_by_sha(@build.sha).builds.order('id DESC') @builds = @builds.where("id not in (?)", @build.id) @pipeline = @build.pipeline - - respond_to do |format| - format.html - format.json do - render json: { - id: @build.id, - status: @build.status, - trace_html: @build.trace_html - } - end - end end def trace - respond_to do |format| - format.json do - state = params[:state].presence - render json: @build.trace_with_state(state: state). - merge!(id: @build.id, status: @build.status) + build.trace.read do |stream| + respond_to do |format| + format.json do + result = { + id: @build.id, status: @build.status, complete: @build.complete? + } + + if stream.valid? + stream.limit + state = params[:state].presence + trace = stream.html_with_state(state) + result.merge!(trace.to_h) + end + + render json: result + end end end end @@ -86,10 +86,12 @@ class Projects::BuildsController < Projects::ApplicationController end def raw - if @build.has_trace_file? - send_file @build.trace_file_path, type: 'text/plain; charset=utf-8', disposition: 'inline' - else - render_404 + build.trace.read do |stream| + if stream.file? + send_file stream.path, type: 'text/plain; charset=utf-8', disposition: 'inline' + else + render_404 + end end end diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 8431c5f228c..70470414bc4 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -171,19 +171,6 @@ module Ci latest_builds.where('stage_idx < ?', stage_idx) end - def trace_html(**args) - trace_with_state(**args)[:html] || '' - end - - def trace_with_state(state: nil, last_lines: nil) - trace_ansi = trace(last_lines: last_lines) - if trace_ansi.present? - Ci::Ansi2html.convert(trace_ansi, state) - else - {} - end - end - def timeout project.build_timeout end @@ -244,136 +231,35 @@ module Ci end def update_coverage - coverage = extract_coverage(trace, coverage_regex) + coverage = trace.extract_coverage(coverage_regex) update_attributes(coverage: coverage) if coverage.present? end - def extract_coverage(text, regex) - return unless regex - - matches = text.scan(Regexp.new(regex)).last - matches = matches.last if matches.is_a?(Array) - coverage = matches.gsub(/\d+(\.\d+)?/).first - - if coverage.present? - coverage.to_f - end - rescue - # if bad regex or something goes wrong we dont want to interrupt transition - # so we just silentrly ignore error for now - end - - def has_trace_file? - File.exist?(path_to_trace) || has_old_trace_file? + def trace + Gitlab::Ci::Trace.new(self) end def has_trace? - raw_trace.present? + trace.exist? end - def raw_trace(last_lines: nil) - if File.exist?(trace_file_path) - Gitlab::Ci::TraceReader.new(trace_file_path). - read(last_lines: last_lines) - else - # backward compatibility - read_attribute :trace - end + def trace=(data) + raise NotImplementedError end - ## - # Deprecated - # - # This is a hotfix for CI build data integrity, see #4246 - def has_old_trace_file? - project.ci_id && File.exist?(old_path_to_trace) - end - - def trace(last_lines: nil) - hide_secrets(raw_trace(last_lines: last_lines)) + def old_trace + read_attribute(:trace) end - def trace_length - if raw_trace - raw_trace.bytesize - else - 0 - end - end - - def trace=(trace) - recreate_trace_dir - trace = hide_secrets(trace) - File.write(path_to_trace, trace) - end - - def recreate_trace_dir - unless Dir.exist?(dir_to_trace) - FileUtils.mkdir_p(dir_to_trace) - end - end - private :recreate_trace_dir - - def append_trace(trace_part, offset) - recreate_trace_dir - touch if needs_touch? - - trace_part = hide_secrets(trace_part) - - File.truncate(path_to_trace, offset) if File.exist?(path_to_trace) - File.open(path_to_trace, 'ab') do |f| - f.write(trace_part) - end + def erase_old_trace! + write_attribute(:trace, nil) + save end def needs_touch? Time.now - updated_at > 15.minutes.to_i end - def trace_file_path - if has_old_trace_file? - old_path_to_trace - else - path_to_trace - end - end - - def dir_to_trace - File.join( - Settings.gitlab_ci.builds_path, - created_at.utc.strftime("%Y_%m"), - project.id.to_s - ) - end - - def path_to_trace - "#{dir_to_trace}/#{id}.log" - end - - ## - # Deprecated - # - # This is a hotfix for CI build data integrity, see #4246 - # Should be removed in 8.4, after CI files migration has been done. - # - def old_dir_to_trace - File.join( - Settings.gitlab_ci.builds_path, - created_at.utc.strftime("%Y_%m"), - project.ci_id.to_s - ) - end - - ## - # Deprecated - # - # This is a hotfix for CI build data integrity, see #4246 - # Should be removed in 8.4, after CI files migration has been done. - # - def old_path_to_trace - "#{old_dir_to_trace}/#{id}.log" - end - ## # Deprecated # @@ -555,6 +441,15 @@ module Ci options[:dependencies]&.empty? end + def hide_secrets(trace) + return unless trace + + trace = trace.dup + Ci::MaskSecret.mask!(trace, project.runners_token) if project + Ci::MaskSecret.mask!(trace, token) + trace + end + private def update_artifacts_size @@ -566,7 +461,7 @@ module Ci end def erase_trace! - self.trace = nil + trace.erase! end def update_erased!(user = nil) @@ -628,15 +523,6 @@ module Ci pipeline.config_processor.build_attributes(name) end - def hide_secrets(trace) - return unless trace - - trace = trace.dup - Ci::MaskSecret.mask!(trace, project.runners_token) if project - Ci::MaskSecret.mask!(trace, token) - trace - end - def update_project_statistics return unless project diff --git a/app/views/notify/pipeline_failed_email.html.haml b/app/views/notify/pipeline_failed_email.html.haml index 4beb6fcee5d..a83faa839df 100644 --- a/app/views/notify/pipeline_failed_email.html.haml +++ b/app/views/notify/pipeline_failed_email.html.haml @@ -137,6 +137,6 @@ - if build.has_trace? %td{ colspan: "2", style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;padding:0 0 15px;" } %pre{ style: "font-family:Monaco,'Lucida Console','Courier New',Courier,monospace;background-color:#fafafa;border-radius:3px;overflow:hidden;white-space:pre-wrap;word-break:break-all;font-size:13px;line-height:1.4;padding:12px;color:#333333;margin:0;" } - = build.trace_html(last_lines: 10).html_safe + = build.trace.html(last_lines: 10).html_safe - else %td{ colspan: "2" } diff --git a/app/views/notify/pipeline_failed_email.text.erb b/app/views/notify/pipeline_failed_email.text.erb index c1a4ea40cf5..294238eee51 100644 --- a/app/views/notify/pipeline_failed_email.text.erb +++ b/app/views/notify/pipeline_failed_email.text.erb @@ -35,7 +35,7 @@ had <%= failed.size %> failed <%= 'build'.pluralize(failed.size) %>. Stage: <%= build.stage %> Name: <%= build.name %> <% if build.has_trace? -%> -Trace: <%= build.trace_with_state(last_lines: 10)[:text] %> +Trace: <%= build.trace.raw(last_lines: 10) %> <% end -%> <% end -%> diff --git a/app/views/projects/builds/_sidebar.html.haml b/app/views/projects/builds/_sidebar.html.haml index 6f45d5b0689..f4a66398c85 100644 --- a/app/views/projects/builds/_sidebar.html.haml +++ b/app/views/projects/builds/_sidebar.html.haml @@ -68,7 +68,7 @@ - elsif @build.runner \##{@build.runner.id} .btn-group.btn-group-justified{ role: :group } - - if @build.has_trace_file? + - if @build.has_trace? = link_to 'Raw', raw_namespace_project_build_path(@project.namespace, @project, @build), class: 'btn btn-sm btn-default' - if @build.active? = link_to "Cancel", cancel_namespace_project_build_path(@project.namespace, @project, @build), class: 'btn btn-sm btn-default', method: :post diff --git a/db/fixtures/development/14_pipelines.rb b/db/fixtures/development/14_pipelines.rb index 534847a7107..3c42f7db6d5 100644 --- a/db/fixtures/development/14_pipelines.rb +++ b/db/fixtures/development/14_pipelines.rb @@ -130,7 +130,7 @@ class Gitlab::Seeder::Pipelines def setup_build_log(build) if %w(running success failed).include?(build.status) - build.trace = FFaker::Lorem.paragraphs(6).join("\n\n") + build.trace.set(FFaker::Lorem.paragraphs(6).join("\n\n")) end end diff --git a/features/steps/project/builds/summary.rb b/features/steps/project/builds/summary.rb index 19ff92f6dc6..124582de6b9 100644 --- a/features/steps/project/builds/summary.rb +++ b/features/steps/project/builds/summary.rb @@ -22,9 +22,9 @@ class Spinach::Features::ProjectBuildsSummary < Spinach::FeatureSteps end step 'recent build has been erased' do + expect(@build).not_to have_trace expect(@build.artifacts_file.exists?).to be_falsy expect(@build.artifacts_metadata.exists?).to be_falsy - expect(@build.trace).to be_empty end step 'recent build summary does not have artifacts widget' do diff --git a/features/steps/shared/builds.rb b/features/steps/shared/builds.rb index 5bc3a1f5ac4..5549fc25525 100644 --- a/features/steps/shared/builds.rb +++ b/features/steps/shared/builds.rb @@ -47,7 +47,7 @@ module SharedBuilds end step 'recent build has a build trace' do - @build.trace = 'job trace' + @build.trace.set('job trace') end step 'download of build artifacts archive starts' do diff --git a/lib/api/jobs.rb b/lib/api/jobs.rb index ffab0aafe59..288b03d940c 100644 --- a/lib/api/jobs.rb +++ b/lib/api/jobs.rb @@ -118,7 +118,7 @@ module API content_type 'text/plain' env['api.format'] = :binary - trace = build.trace + trace = build.trace.raw body trace end diff --git a/lib/api/runner.rb b/lib/api/runner.rb index d288369e362..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,16 +145,14 @@ module API content_range = request.headers['Content-Range'] content_range = content_range.split('-') - current_length = job.trace_length - unless current_length == content_range[0].to_i - return error!('416 Range Not Satisfiable', 416, { 'Range' => "0-#{current_length}" }) + 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 - job.append_trace(request.body.read, content_range[0].to_i) - status 202 header 'Job-Status', job.status - header 'Range', "0-#{job.trace_length}" + header 'Range', "0-#{stream_size}" end desc 'Authorize artifacts uploading for job' do diff --git a/lib/api/v3/builds.rb b/lib/api/v3/builds.rb index 6f97102c6ef..4dd03cdf24b 100644 --- a/lib/api/v3/builds.rb +++ b/lib/api/v3/builds.rb @@ -120,7 +120,7 @@ module API content_type 'text/plain' env['api.format'] = :binary - trace = build.trace + trace = build.trace.raw body trace end diff --git a/lib/ci/ansi2html.rb b/lib/ci/ansi2html.rb index b3ccad7b28d..1020452480a 100644 --- a/lib/ci/ansi2html.rb +++ b/lib/ci/ansi2html.rb @@ -132,34 +132,54 @@ module Ci STATE_PARAMS = [:offset, :n_open_tags, :fg_color, :bg_color, :style_mask].freeze - def convert(raw, new_state) + def convert(stream, new_state) reset_state - restore_state(raw, new_state) if new_state.present? - - start = @offset - ansi = raw[@offset..-1] + restore_state(new_state, stream) if new_state.present? + + append = false + truncated = false + + cur_offset = stream.tell + if cur_offset > @offset + @offset = cur_offset + truncated = true + else + stream.seek(@offset) + append = @offset > 0 + end + start_offset = @offset open_new_tag - s = StringScanner.new(ansi) - until s.eos? - if s.scan(/\e([@-_])(.*?)([@-~])/) - handle_sequence(s) - elsif s.scan(/\e(([@-_])(.*?)?)?$/) - break - elsif s.scan(/</) - @out << '<' - elsif s.scan(/\r?\n/) - @out << '<br>' - else - @out << s.scan(/./m) + stream.each_line do |line| + s = StringScanner.new(line) + until s.eos? + if s.scan(/\e([@-_])(.*?)([@-~])/) + handle_sequence(s) + elsif s.scan(/\e(([@-_])(.*?)?)?$/) + break + elsif s.scan(/</) + @out << '<' + elsif s.scan(/\r?\n/) + @out << '<br>' + else + @out << s.scan(/./m) + end + @offset += s.matched_size end - @offset += s.matched_size end close_open_tags() - { state: state, html: @out, text: ansi[0, @offset - start], append: start > 0 } + OpenStruct.new( + html: @out, + state: state, + append: append, + truncated: truncated, + offset: start_offset, + size: stream.tell - start_offset, + total: stream.size + ) end def handle_sequence(s) @@ -240,10 +260,10 @@ module Ci Base64.urlsafe_encode64(state.to_json) end - def restore_state(raw, new_state) + def restore_state(new_state, stream) state = Base64.urlsafe_decode64(new_state) state = JSON.parse(state, symbolize_names: true) - return if state[:offset].to_i > raw.length + return if state[:offset].to_i > stream.size STATE_PARAMS.each do |param| send("#{param}=".to_sym, state[param]) diff --git a/lib/ci/api/builds.rb b/lib/ci/api/builds.rb index 95cc6308c3b..67b269b330c 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,16 +92,14 @@ module Ci content_range = request.headers['Content-Range'] content_range = content_range.split('-') - current_length = build.trace_length - unless current_length == content_range[0].to_i - return error!('416 Range Not Satisfiable', 416, { 'Range' => "0-#{current_length}" }) + 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 - build.append_trace(request.body.read, content_range[0].to_i) - status 202 header 'Build-Status', build.status - header 'Range', "0-#{build.trace_length}" + 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 new file mode 100644 index 00000000000..5b835bb669a --- /dev/null +++ b/lib/gitlab/ci/trace.rb @@ -0,0 +1,136 @@ +module Gitlab + module Ci + class Trace + attr_reader :job + + delegate :old_trace, to: :job + + def initialize(job) + @job = job + end + + def html(last_lines: nil) + read do |stream| + stream.html(last_lines: last_lines) + end + end + + def raw(last_lines: nil) + read do |stream| + stream.raw(last_lines: last_lines) + end + end + + def extract_coverage(regex) + read do |stream| + stream.extract_coverage(regex) + end + end + + def set(data) + write do |stream| + data = job.hide_secrets(data) + stream.set(data) + 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 exist? + current_path.present? || old_trace.present? + end + + def read + stream = Gitlab::Ci::Trace::Stream.new do + if current_path + File.open(current_path, "rb") + elsif old_trace + StringIO.new(old_trace) + end + end + + yield stream + ensure + stream&.close + end + + def write + stream = Gitlab::Ci::Trace::Stream.new do + File.open(ensure_path, "a+b") + end + + yield(stream).tap do + job.touch if job.needs_touch? + end + ensure + stream&.close + end + + def erase! + paths.each do |trace_path| + FileUtils.rm(trace_path, force: true) + end + + job.erase_old_trace! + end + + private + + def ensure_path + return current_path if current_path + + ensure_directory + default_path + end + + def ensure_directory + unless Dir.exist?(default_directory) + FileUtils.mkdir_p(default_directory) + end + end + + def current_path + @current_path ||= paths.find do |trace_path| + File.exist?(trace_path) + end + end + + def paths + [ + default_path, + deprecated_path + ].compact + end + + def default_directory + File.join( + Settings.gitlab_ci.builds_path, + job.created_at.utc.strftime("%Y_%m"), + job.project_id.to_s + ) + end + + def default_path + File.join(default_directory, "#{job.id}.log") + end + + def deprecated_path + File.join( + Settings.gitlab_ci.builds_path, + job.created_at.utc.strftime("%Y_%m"), + job.project.ci_id.to_s, + "#{job.id}.log" + ) if job.project&.ci_id + end + end + end +end diff --git a/lib/gitlab/ci/trace/stream.rb b/lib/gitlab/ci/trace/stream.rb new file mode 100644 index 00000000000..2af94e2c60e --- /dev/null +++ b/lib/gitlab/ci/trace/stream.rb @@ -0,0 +1,119 @@ +module Gitlab + module Ci + class Trace + # This was inspired from: http://stackoverflow.com/a/10219411/1520132 + class Stream + BUFFER_SIZE = 4096 + LIMIT_SIZE = 50.kilobytes + + attr_reader :stream + + delegate :close, :tell, :seek, :size, :path, :truncate, to: :stream, allow_nil: true + + delegate :valid?, to: :stream, as: :present?, allow_nil: true + + def initialize + @stream = yield + end + + def valid? + self.stream.present? + end + + def file? + self.path.present? + end + + def limit(last_bytes = LIMIT_SIZE) + stream_size = size + if stream_size < last_bytes + last_bytes = stream_size + end + stream.seek(-last_bytes, IO::SEEK_END) + end + + def append(data, offset) + stream.truncate(offset) + stream.seek(0, IO::SEEK_END) + stream.write(data) + stream.flush() + end + + def set(data) + truncate(0) + stream.write(data) + stream.flush() + end + + def raw(last_lines: nil) + return unless valid? + + if last_lines.to_i > 0 + read_last_lines(last_lines) + else + stream.read + end + end + + 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 + + def extract_coverage(regex) + return unless valid? + return unless regex + + regex = Regexp.new(regex) + + match = "" + + stream.each_line do |line| + matches = line.scan(regex) + next unless matches.is_a?(Array) + + match = matches.flatten.last + coverage = match.gsub(/\d+(\.\d+)?/).first + return coverage.to_f if coverage.present? + end + rescue + # if bad regex or something goes wrong we dont want to interrupt transition + # so we just silentrly ignore error for now + end + + private + + 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 <= last_lines && pos < max + pos += BUFFER_SIZE + + buf = + if pos <= max + stream.seek(-pos, IO::SEEK_END) + stream.read(BUFFER_SIZE) + else # Reached the head, read only left + stream.seek(0) + stream.read(BUFFER_SIZE - (pos - max)) + end + + lines += buf.count("\n") + chunks.unshift(buf) + end + + chunks.join.lines.last(last_lines).join + .force_encoding(Encoding.default_external) + end + end + end + end +end diff --git a/lib/gitlab/ci/trace_reader.rb b/lib/gitlab/ci/trace_reader.rb deleted file mode 100644 index 1d7ddeb3e0f..00000000000 --- a/lib/gitlab/ci/trace_reader.rb +++ /dev/null @@ -1,50 +0,0 @@ -module Gitlab - module Ci - # This was inspired from: http://stackoverflow.com/a/10219411/1520132 - class TraceReader - BUFFER_SIZE = 4096 - - attr_accessor :path, :buffer_size - - def initialize(new_path, buffer_size: BUFFER_SIZE) - self.path = new_path - self.buffer_size = Integer(buffer_size) - end - - def read(last_lines: nil) - if last_lines - read_last_lines(last_lines) - else - File.read(path) - end - end - - def read_last_lines(max_lines) - File.open(path) do |file| - chunks = [] - pos = lines = 0 - max = file.size - - # We want an extra line to make sure fist line has full contents - while lines <= max_lines && pos < max - pos += buffer_size - - buf = if pos <= max - file.seek(-pos, IO::SEEK_END) - file.read(buffer_size) - else # Reached the head, read only left - file.seek(0) - file.read(buffer_size - (pos - max)) - end - - lines += buf.count("\n") - chunks.unshift(buf) - end - - chunks.join.lines.last(max_lines).join - .force_encoding(Encoding.default_external) - end - end - end - end -end 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 <" do - expect(subject.convert("<")[:html]).to eq('<') + expect(convert_html("<")).to eq('<') 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 |