BigW Consortium Gitlab

Skip to content

G
gitlab-ce
Commit 3cefaca2 by Kamil Trzciński
Options

Merge branch 'fix/build-without-trace-exceptions' into 'master'

Fix exceptions when loading build trace ## What does this MR do? This MR fixes exceptions when loading build trace. - [x] [Changelog entry](https://docs.gitlab.com/ce/development/changelog.html) added - [x] Tests added for this feature/bug ## What are the relevant issue numbers? Closes #24638 See merge request !7658
parents 09ae1be1 de249028
Showing with 124 additions and 81 deletions
app/assets/javascripts/build.js
...@@ -12,7 +12,7 @@ ...@@ -12,7 +12,7 @@
this.pageUrl = options.pageUrl; this.pageUrl = options.pageUrl;
this.buildUrl = options.buildUrl; this.buildUrl = options.buildUrl;
this.buildStatus = options.buildStatus; this.buildStatus = options.buildStatus;
this.state = options.state1; this.state = options.logState;
this.buildStage = options.buildStage; this.buildStage = options.buildStage;
this.updateDropdown = bind(this.updateDropdown, this); this.updateDropdown = bind(this.updateDropdown, this);
this.$document = $(document); this.$document = $(document);
......
app/helpers/builds_helper.rb
...@@ -12,7 +12,7 @@ module BuildsHelper ...@@ -12,7 +12,7 @@ module BuildsHelper
build_url: namespace_project_build_url(@project.namespace, @project, @build, :json), build_url: namespace_project_build_url(@project.namespace, @project, @build, :json),
build_status: @build.status, build_status: @build.status,
build_stage: @build.stage, build_stage: @build.stage,
state1: @build.trace_with_state[:state] log_state: @build.trace_with_state[:state].to_s
} }
end end
end end
changelogs/unreleased/fix-build-without-trace-exceptions.yml 0 → 100644
---
title: Fix exceptions when loading build trace
merge_request: 7658
author:
spec/features/projects/builds_spec.rb
require 'spec_helper' require 'spec_helper'
require 'tempfile' require 'tempfile'
describe "Builds" do feature 'Builds', :feature do
let(:artifacts_file) { fixture_file_upload(Rails.root + 'spec/fixtures/banana_sample.gif', 'image/gif') } let(:user) { create(:user) }
let(:project) { create(:project) }
let(:pipeline) { create(:ci_pipeline, project: project) }
let(:build) { create(:ci_build, :trace, pipeline: pipeline) }
let(:build2) { create(:ci_build) }
let(:artifacts_file) do
fixture_file_upload(Rails.root + 'spec/fixtures/banana_sample.gif', 'image/gif')
end
before do before do
login_as(:user) project.team << [user, :developer]
@commit = FactoryGirl.create :ci_pipeline login_as(user)
@build = FactoryGirl.create :ci_build, :trace, pipeline: @commit
@build2 = FactoryGirl.create :ci_build
@project = @commit.project
@project.team << [@user, :developer]
end end
describe "GET /:project/builds" do describe "GET /:project/builds" do
let!(:build) { create(:ci_build, pipeline: pipeline) }
context "Pending scope" do context "Pending scope" do
before do before do
visit namespace_project_builds_path(@project.namespace, @project, scope: :pending) visit namespace_project_builds_path(project.namespace, project, scope: :pending)
end end
it "shows Pending tab builds" do it "shows Pending tab builds" do
expect(page).to have_link 'Cancel running' expect(page).to have_link 'Cancel running'
expect(page).to have_selector('.nav-links li.active', text: 'Pending') expect(page).to have_selector('.nav-links li.active', text: 'Pending')
expect(page).to have_content @build.short_sha expect(page).to have_content build.short_sha
expect(page).to have_content @build.ref expect(page).to have_content build.ref
expect(page).to have_content @build.name expect(page).to have_content build.name
end end
end end
context "Running scope" do context "Running scope" do
before do before do
@build.run! build.run!
visit namespace_project_builds_path(@project.namespace, @project, scope: :running) visit namespace_project_builds_path(project.namespace, project, scope: :running)
end end
it "shows Running tab builds" do it "shows Running tab builds" do
expect(page).to have_selector('.nav-links li.active', text: 'Running') expect(page).to have_selector('.nav-links li.active', text: 'Running')
expect(page).to have_link 'Cancel running' expect(page).to have_link 'Cancel running'
expect(page).to have_content @build.short_sha expect(page).to have_content build.short_sha
expect(page).to have_content @build.ref expect(page).to have_content build.ref
expect(page).to have_content @build.name expect(page).to have_content build.name
end end
end end
context "Finished scope" do context "Finished scope" do
before do before do
@build.run! build.run!
visit namespace_project_builds_path(@project.namespace, @project, scope: :finished) visit namespace_project_builds_path(project.namespace, project, scope: :finished)
end end
it "shows Finished tab builds" do it "shows Finished tab builds" do
...@@ -58,15 +65,15 @@ describe "Builds" do ...@@ -58,15 +65,15 @@ describe "Builds" do
context "All builds" do context "All builds" do
before do before do
@project.builds.running_or_pending.each(&:success) project.builds.running_or_pending.each(&:success)
visit namespace_project_builds_path(@project.namespace, @project) visit namespace_project_builds_path(project.namespace, project)
end end
it "shows All tab builds" do it "shows All tab builds" do
expect(page).to have_selector('.nav-links li.active', text: 'All') expect(page).to have_selector('.nav-links li.active', text: 'All')
expect(page).to have_content @build.short_sha expect(page).to have_content build.short_sha
expect(page).to have_content @build.ref expect(page).to have_content build.ref
expect(page).to have_content @build.name expect(page).to have_content build.name
expect(page).not_to have_link 'Cancel running' expect(page).not_to have_link 'Cancel running'
end end
end end
...@@ -74,17 +81,17 @@ describe "Builds" do ...@@ -74,17 +81,17 @@ describe "Builds" do
describe "POST /:project/builds/:id/cancel_all" do describe "POST /:project/builds/:id/cancel_all" do
before do before do
@build.run! build.run!
visit namespace_project_builds_path(@project.namespace, @project) visit namespace_project_builds_path(project.namespace, project)
click_link "Cancel running" click_link "Cancel running"
end end
it 'shows all necessary content' do it 'shows all necessary content' do
expect(page).to have_selector('.nav-links li.active', text: 'All') expect(page).to have_selector('.nav-links li.active', text: 'All')
expect(page).to have_content 'canceled' expect(page).to have_content 'canceled'
expect(page).to have_content @build.short_sha expect(page).to have_content build.short_sha
expect(page).to have_content @build.ref expect(page).to have_content build.ref
expect(page).to have_content @build.name expect(page).to have_content build.name
expect(page).not_to have_link 'Cancel running' expect(page).not_to have_link 'Cancel running'
end end
end end
...@@ -92,20 +99,20 @@ describe "Builds" do ...@@ -92,20 +99,20 @@ describe "Builds" do
describe "GET /:project/builds/:id" do describe "GET /:project/builds/:id" do
context "Build from project" do context "Build from project" do
before do before do
visit namespace_project_build_path(@project.namespace, @project, @build) visit namespace_project_build_path(project.namespace, project, build)
end end
it 'shows commit`s data' do it 'shows commit`s data' do
expect(page.status_code).to eq(200) expect(page.status_code).to eq(200)
expect(page).to have_content @commit.sha[0..7] expect(page).to have_content pipeline.sha[0..7]
expect(page).to have_content @commit.git_commit_message expect(page).to have_content pipeline.git_commit_message
expect(page).to have_content @commit.git_author_name expect(page).to have_content pipeline.git_author_name
end end
end end
context "Build from other project" do context "Build from other project" do
before do before do
visit namespace_project_build_path(@project.namespace, @project, @build2) visit namespace_project_build_path(project.namespace, project, build2)
end end
it { expect(page.status_code).to eq(404) } it { expect(page.status_code).to eq(404) }
...@@ -113,8 +120,8 @@ describe "Builds" do ...@@ -113,8 +120,8 @@ describe "Builds" do
context "Download artifacts" do context "Download artifacts" do
before do before do
@build.update_attributes(artifacts_file: artifacts_file) build.update_attributes(artifacts_file: artifacts_file)
visit namespace_project_build_path(@project.namespace, @project, @build) visit namespace_project_build_path(project.namespace, project, build)
end end
it 'has button to download artifacts' do it 'has button to download artifacts' do
...@@ -124,8 +131,8 @@ describe "Builds" do ...@@ -124,8 +131,8 @@ describe "Builds" do
context 'Artifacts expire date' do context 'Artifacts expire date' do
before do before do
@build.update_attributes(artifacts_file: artifacts_file, artifacts_expire_at: expire_at) build.update_attributes(artifacts_file: artifacts_file, artifacts_expire_at: expire_at)
visit namespace_project_build_path(@project.namespace, @project, @build) visit namespace_project_build_path(project.namespace, project, build)
end end
context 'no expire date defined' do context 'no expire date defined' do
...@@ -158,10 +165,10 @@ describe "Builds" do ...@@ -158,10 +165,10 @@ describe "Builds" do
end end
end end
context 'Build raw trace' do feature 'Raw trace' do
before do before do
@build.run! build.run!
visit namespace_project_build_path(@project.namespace, @project, @build) visit namespace_project_build_path(project.namespace, project, build)
end end
it do it do
...@@ -169,11 +176,43 @@ describe "Builds" do ...@@ -169,11 +176,43 @@ describe "Builds" do
end end
end end
describe 'Variables' do feature 'HTML trace', :js do
before do
build.run!
visit namespace_project_build_path(project.namespace, project, build)
end
context 'when build has an initial trace' do
it 'loads build trace' do
expect(page).to have_content 'BUILD TRACE'
build.append_trace(' and more trace', 11)
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
let(:trigger_request) { create(:ci_trigger_request_with_variables) }
let(:build) do
create :ci_build, pipeline: pipeline, trigger_request: trigger_request
end
before do before do
@trigger_request = create :ci_trigger_request_with_variables visit namespace_project_build_path(project.namespace, project, build)
@build = create :ci_build, pipeline: @commit, trigger_request: @trigger_request
visit namespace_project_build_path(@project.namespace, @project, @build)
end end
it 'shows variable key and value after click', js: true do it 'shows variable key and value after click', js: true do
...@@ -193,8 +232,8 @@ describe "Builds" do ...@@ -193,8 +232,8 @@ describe "Builds" do
describe "POST /:project/builds/:id/cancel" do describe "POST /:project/builds/:id/cancel" do
context "Build from project" do context "Build from project" do
before do before do
@build.run! build.run!
visit namespace_project_build_path(@project.namespace, @project, @build) visit namespace_project_build_path(project.namespace, project, build)
click_link "Cancel" click_link "Cancel"
end end
...@@ -207,9 +246,9 @@ describe "Builds" do ...@@ -207,9 +246,9 @@ describe "Builds" do
context "Build from other project" do context "Build from other project" do
before do before do
@build.run! build.run!
visit namespace_project_build_path(@project.namespace, @project, @build) visit namespace_project_build_path(project.namespace, project, build)
page.driver.post(cancel_namespace_project_build_path(@project.namespace, @project, @build2)) page.driver.post(cancel_namespace_project_build_path(project.namespace, project, build2))
end end
it { expect(page.status_code).to eq(404) } it { expect(page.status_code).to eq(404) }
...@@ -219,8 +258,8 @@ describe "Builds" do ...@@ -219,8 +258,8 @@ describe "Builds" do
describe "POST /:project/builds/:id/retry" do describe "POST /:project/builds/:id/retry" do
context "Build from project" do context "Build from project" do
before do before do
@build.run! build.run!
visit namespace_project_build_path(@project.namespace, @project, @build) visit namespace_project_build_path(project.namespace, project, build)
click_link 'Cancel' click_link 'Cancel'
page.within('.build-header') do page.within('.build-header') do
click_link 'Retry build' click_link 'Retry build'
...@@ -238,10 +277,10 @@ describe "Builds" do ...@@ -238,10 +277,10 @@ describe "Builds" do
context "Build from other project" do context "Build from other project" do
before do before do
@build.run! build.run!
visit namespace_project_build_path(@project.namespace, @project, @build) visit namespace_project_build_path(project.namespace, project, build)
click_link 'Cancel' click_link 'Cancel'
page.driver.post(retry_namespace_project_build_path(@project.namespace, @project, @build2)) page.driver.post(retry_namespace_project_build_path(project.namespace, project, build2))
end end
it { expect(page).to have_http_status(404) } it { expect(page).to have_http_status(404) }
...@@ -249,13 +288,13 @@ describe "Builds" do ...@@ -249,13 +288,13 @@ describe "Builds" do
context "Build that current user is not allowed to retry" do context "Build that current user is not allowed to retry" do
before do before do
@build.run! build.run!
@build.cancel! build.cancel!
@project.update(visibility_level: Gitlab::VisibilityLevel::PUBLIC) project.update(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
logout_direct logout_direct
login_with(create(:user)) login_with(create(:user))
visit namespace_project_build_path(@project.namespace, @project, @build) visit namespace_project_build_path(project.namespace, project, build)
end end
it 'does not show the Retry button' do it 'does not show the Retry button' do
...@@ -268,15 +307,15 @@ describe "Builds" do ...@@ -268,15 +307,15 @@ describe "Builds" do
describe "GET /:project/builds/:id/download" do describe "GET /:project/builds/:id/download" do
before do before do
@build.update_attributes(artifacts_file: artifacts_file) build.update_attributes(artifacts_file: artifacts_file)
visit namespace_project_build_path(@project.namespace, @project, @build) visit namespace_project_build_path(project.namespace, project, build)
click_link 'Download' click_link 'Download'
end end
context "Build from other project" do context "Build from other project" do
before do before do
@build2.update_attributes(artifacts_file: artifacts_file) build2.update_attributes(artifacts_file: artifacts_file)
visit download_namespace_project_build_artifacts_path(@project.namespace, @project, @build2) visit download_namespace_project_build_artifacts_path(project.namespace, project, build2)
end end
it { expect(page.status_code).to eq(404) } it { expect(page.status_code).to eq(404) }
...@@ -288,23 +327,23 @@ describe "Builds" do ...@@ -288,23 +327,23 @@ describe "Builds" do
context 'build from project' do context 'build from project' do
before do before do
Capybara.current_session.driver.header('X-Sendfile-Type', 'X-Sendfile') Capybara.current_session.driver.header('X-Sendfile-Type', 'X-Sendfile')
@build.run! build.run!
visit namespace_project_build_path(@project.namespace, @project, @build) visit namespace_project_build_path(project.namespace, project, build)
page.within('.js-build-sidebar') { click_link 'Raw' } page.within('.js-build-sidebar') { click_link 'Raw' }
end end
it 'sends the right headers' do it 'sends the right headers' do
expect(page.status_code).to eq(200) expect(page.status_code).to eq(200)
expect(page.response_headers['Content-Type']).to eq('text/plain; charset=utf-8') 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.path_to_trace)
end end
end end
context 'build from other project' do context 'build from other project' do
before do before do
Capybara.current_session.driver.header('X-Sendfile-Type', 'X-Sendfile') Capybara.current_session.driver.header('X-Sendfile-Type', 'X-Sendfile')
@build2.run! build2.run!
visit raw_namespace_project_build_path(@project.namespace, @project, @build2) visit raw_namespace_project_build_path(project.namespace, project, build2)
end end
it 'sends the right headers' do it 'sends the right headers' do
...@@ -325,8 +364,8 @@ describe "Builds" do ...@@ -325,8 +364,8 @@ describe "Builds" do
context 'when build has trace in file' do context 'when build has trace in file' do
before do before do
Capybara.current_session.driver.header('X-Sendfile-Type', 'X-Sendfile') Capybara.current_session.driver.header('X-Sendfile-Type', 'X-Sendfile')
@build.run! build.run!
visit namespace_project_build_path(@project.namespace, @project, @build) 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(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(:path_to_trace).and_return(existing_file)
...@@ -345,8 +384,8 @@ describe "Builds" do ...@@ -345,8 +384,8 @@ describe "Builds" do
context 'when build has trace in old file' do context 'when build has trace in old file' do
before do before do
Capybara.current_session.driver.header('X-Sendfile-Type', 'X-Sendfile') Capybara.current_session.driver.header('X-Sendfile-Type', 'X-Sendfile')
@build.run! build.run!
visit namespace_project_build_path(@project.namespace, @project, @build) 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(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(:path_to_trace).and_return(non_existing_file)
...@@ -365,8 +404,8 @@ describe "Builds" do ...@@ -365,8 +404,8 @@ describe "Builds" do
context 'when build has trace in DB' do context 'when build has trace in DB' do
before do before do
Capybara.current_session.driver.header('X-Sendfile-Type', 'X-Sendfile') Capybara.current_session.driver.header('X-Sendfile-Type', 'X-Sendfile')
@build.run! build.run!
visit namespace_project_build_path(@project.namespace, @project, @build) 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(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(:path_to_trace).and_return(non_existing_file)
...@@ -385,7 +424,7 @@ describe "Builds" do ...@@ -385,7 +424,7 @@ describe "Builds" do
describe "GET /:project/builds/:id/trace.json" do describe "GET /:project/builds/:id/trace.json" do
context "Build from project" do context "Build from project" do
before do before do
visit trace_namespace_project_build_path(@project.namespace, @project, @build, format: :json) visit trace_namespace_project_build_path(project.namespace, project, build, format: :json)
end end
it { expect(page.status_code).to eq(200) } it { expect(page.status_code).to eq(200) }
...@@ -393,7 +432,7 @@ describe "Builds" do ...@@ -393,7 +432,7 @@ describe "Builds" do
context "Build from other project" do context "Build from other project" do
before do before do
visit trace_namespace_project_build_path(@project.namespace, @project, @build2, format: :json) visit trace_namespace_project_build_path(project.namespace, project, build2, format: :json)
end end
it { expect(page.status_code).to eq(404) } it { expect(page.status_code).to eq(404) }
...@@ -403,7 +442,7 @@ describe "Builds" do ...@@ -403,7 +442,7 @@ describe "Builds" do
describe "GET /:project/builds/:id/status" do describe "GET /:project/builds/:id/status" do
context "Build from project" do context "Build from project" do
before do before do
visit status_namespace_project_build_path(@project.namespace, @project, @build) visit status_namespace_project_build_path(project.namespace, project, build)
end end
it { expect(page.status_code).to eq(200) } it { expect(page.status_code).to eq(200) }
...@@ -411,7 +450,7 @@ describe "Builds" do ...@@ -411,7 +450,7 @@ describe "Builds" do
context "Build from other project" do context "Build from other project" do
before do before do
visit status_namespace_project_build_path(@project.namespace, @project, @build2) visit status_namespace_project_build_path(project.namespace, project, build2)
end end
it { expect(page.status_code).to eq(404) } it { expect(page.status_code).to eq(404) }
......
spec/javascripts/fixtures/build.html.haml
...@@ -54,7 +54,7 @@ ...@@ -54,7 +54,7 @@
build_url: 'http://example.com/root/test-build/builds/2.json', build_url: 'http://example.com/root/test-build/builds/2.json',
build_status: 'passed', build_status: 'passed',
build_stage: 'test', build_stage: 'test',
state1: 'buildstate' }} log_state: 'buildstate' }}
%p.build-detail-row %p.build-detail-row
The artifacts will be removed in The artifacts will be removed in
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment