diff --git a/app/assets/javascripts/jobs/store/actions.js b/app/assets/javascripts/jobs/store/actions.js index a47c1d961d23cf28e7fae8bed6537599c5e0bb32..6da8f8b455eca8fc27e66cd40f35d2ce3830c72c 100644 --- a/app/assets/javascripts/jobs/store/actions.js +++ b/app/assets/javascripts/jobs/store/actions.js @@ -21,7 +21,8 @@ export const init = ({ dispatch }, { endpoint, logState, pagePath }) => { logState, pagePath, }); - dispatch('fetchJob'); + + return Promise.all([dispatch('fetchJob'), dispatch('fetchTrace')]); }; export const setJobEndpoint = ({ commit }, endpoint) => commit(types.SET_JOB_ENDPOINT, endpoint); @@ -39,7 +40,6 @@ export const toggleSidebar = ({ dispatch, state }) => { }; let eTagPoll; -let isTraceReadyForRender; export const clearEtagPoll = () => { eTagPoll = null; @@ -71,14 +71,7 @@ export const fetchJob = ({ state, dispatch }) => { }); if (!Visibility.hidden()) { - // eslint-disable-next-line promise/catch-or-return - eTagPoll.makeRequest().then(() => { - // if a job is canceled we still need to dispatch - // fetchTrace to get the trace so we check for has_trace - if (state.job.started || state.job.has_trace) { - dispatch('fetchTrace'); - } - }); + eTagPoll.makeRequest(); } else { axios .get(state.jobEndpoint) @@ -88,15 +81,9 @@ export const fetchJob = ({ state, dispatch }) => { Visibility.change(() => { if (!Visibility.hidden()) { - // This check is needed to ensure the loading icon - // is not shown for a finished job during a visibility change - if (!isTraceReadyForRender && state.job.started) { - dispatch('startPollingTrace'); - } dispatch('restartPolling'); } else { dispatch('stopPolling'); - dispatch('stopPollingTrace'); } }); }; @@ -177,8 +164,6 @@ export const fetchTrace = ({ dispatch, state }) => params: { state: state.traceState }, }) .then(({ data }) => { - isTraceReadyForRender = data.complete; - dispatch('toggleScrollisInBottom', isScrolledToBottom()); dispatch('receiveTraceSuccess', data); @@ -258,7 +243,7 @@ export const receiveJobsForStageError = ({ commit }) => { flash(__('An error occurred while fetching the jobs.')); }; -export const triggerManualJob = ({ state, dispatch }, variables) => { +export const triggerManualJob = ({ state }, variables) => { const parsedVariables = variables.map((variable) => { const copyVar = { ...variable }; delete copyVar.id; @@ -269,6 +254,5 @@ export const triggerManualJob = ({ state, dispatch }, variables) => { .post(state.job.status.action.path, { job_variables_attributes: parsedVariables, }) - .then(() => dispatch('fetchTrace')) .catch(() => flash(__('An error occurred while triggering the job.'))); }; diff --git a/app/assets/javascripts/jobs/store/mutations.js b/app/assets/javascripts/jobs/store/mutations.js index dea53f715a75e891dcbb3b497ac457995b3c7fba..924b811d0d6c2896deea3c507c02b41d5e1946fd 100644 --- a/app/assets/javascripts/jobs/store/mutations.js +++ b/app/assets/javascripts/jobs/store/mutations.js @@ -49,7 +49,6 @@ export default { [types.SET_TRACE_TIMEOUT](state, id) { state.traceTimeout = id; - state.isTraceComplete = false; }, /** diff --git a/app/controllers/projects/jobs_controller.rb b/app/controllers/projects/jobs_controller.rb index 900ebc61856105543583d4c66afddc6428a329dd..d2703f5cc383ae0c2b68e7c6650ef867edeefa48 100644 --- a/app/controllers/projects/jobs_controller.rb +++ b/app/controllers/projects/jobs_controller.rb @@ -49,21 +49,25 @@ def show # rubocop: enable CodeReuse/ActiveRecord def trace - @build.trace.read do |stream| - respond_to do |format| - format.json do - @build.trace.being_watched! - - build_trace = Ci::BuildTrace.new( - build: @build, - stream: stream, - state: params[:state]) - - render json: BuildTraceSerializer - .new(project: @project, current_user: @current_user) - .represent(build_trace) + @build.trace.being_watched! if @build.running? + + if @build.has_trace? + @build.trace.read do |stream| + respond_to do |format| + format.json do + build_trace = Ci::BuildTrace.new( + build: @build, + stream: stream, + state: params[:state]) + + render json: BuildTraceSerializer + .new(project: @project, current_user: @current_user) + .represent(build_trace) + end end end + else + head :no_content end end diff --git a/spec/controllers/projects/jobs_controller_spec.rb b/spec/controllers/projects/jobs_controller_spec.rb index 3309b15b276dea643ef5d762055cdf8e368cdbc2..eb5e62f3d44cc3d5388d1fa88c0fc1c10d82e73f 100644 --- a/spec/controllers/projects/jobs_controller_spec.rb +++ b/spec/controllers/projects/jobs_controller_spec.rb @@ -700,10 +700,22 @@ def get_show(**extra_params) expect(json_response['lines']).to eq [{ 'content' => [{ 'text' => 'BUILD TRACE' }], 'offset' => 0 }] end - it 'sets being-watched flag for the job' do - expect(response).to have_gitlab_http_status(:ok) + context 'when job is running' do + let(:job) { create(:ci_build, :trace_live, :running, pipeline: pipeline) } + + it 'sets being-watched flag for the job' do + expect(response).to have_gitlab_http_status(:ok) + + expect(job.trace.being_watched?).to be(true) + end + end - expect(job.trace.being_watched?).to be(true) + context 'when job is not running' do + it 'does not set being-watched flag for the job' do + expect(response).to have_gitlab_http_status(:ok) + + expect(job.trace.being_watched?).to be(false) + end end end @@ -711,11 +723,7 @@ def get_show(**extra_params) let(:job) { create(:ci_build, pipeline: pipeline) } it 'returns no traces' do - expect(response).to have_gitlab_http_status(:ok) - expect(response).to match_response_schema('job/build_trace') - expect(json_response['id']).to eq job.id - expect(json_response['status']).to eq job.status - expect(json_response['lines']).to be_nil + expect(response).to have_gitlab_http_status(:no_content) end end diff --git a/spec/features/projects/jobs_spec.rb b/spec/features/projects/jobs_spec.rb index 4edda9febbeee308cb4b78dbc2dc41fbbb3ec0be..1557a8a2d72bd60a47275d7254d0fb66cf674a9f 100644 --- a/spec/features/projects/jobs_spec.rb +++ b/spec/features/projects/jobs_spec.rb @@ -1212,9 +1212,11 @@ end describe "GET /:project/jobs/:id/trace.json" do + let(:build) { create(:ci_build, :trace_artifact, pipeline: pipeline) } + context "Job from project" do before do - visit trace_project_job_path(project, job, format: :json) + visit trace_project_job_path(project, build, format: :json) end it { expect(page.status_code).to eq(200) } diff --git a/spec/features/security/project/internal_access_spec.rb b/spec/features/security/project/internal_access_spec.rb index cb9f9a6e680b90536752cdea8ba8056503049040..2440b738db3ff19af330aba5fa7b51c09d06b8b7 100644 --- a/spec/features/security/project/internal_access_spec.rb +++ b/spec/features/security/project/internal_access_spec.rb @@ -430,7 +430,7 @@ describe 'GET /:project_path/builds/:id/trace' do let(:pipeline) { create(:ci_pipeline, project: project) } - let(:build) { create(:ci_build, pipeline: pipeline) } + let(:build) { create(:ci_build, :trace_artifact, pipeline: pipeline) } subject { trace_project_job_path(project, build.id) } diff --git a/spec/features/security/project/private_access_spec.rb b/spec/features/security/project/private_access_spec.rb index dda218c5de530f50bb20c1b1ab13b746380eb523..9d3109b92e6b1b26a40de3bb42e1222119492051 100644 --- a/spec/features/security/project/private_access_spec.rb +++ b/spec/features/security/project/private_access_spec.rb @@ -423,7 +423,7 @@ describe 'GET /:project_path/builds/:id/trace' do let(:pipeline) { create(:ci_pipeline, project: project) } - let(:build) { create(:ci_build, pipeline: pipeline) } + let(:build) { create(:ci_build, :trace_artifact, pipeline: pipeline) } subject { trace_project_job_path(project, build.id) } diff --git a/spec/features/security/project/public_access_spec.rb b/spec/features/security/project/public_access_spec.rb index f2dbab72a4856547c3bf72ba18ddb687a75cdf9f..28a1f1cda7f4f7c61347bb7fcd72dc1b3c8e6ec5 100644 --- a/spec/features/security/project/public_access_spec.rb +++ b/spec/features/security/project/public_access_spec.rb @@ -238,7 +238,7 @@ describe 'GET /:project_path/builds/:id/trace' do let(:pipeline) { create(:ci_pipeline, project: project) } - let(:build) { create(:ci_build, pipeline: pipeline) } + let(:build) { create(:ci_build, :trace_artifact, pipeline: pipeline) } subject { trace_project_job_path(project, build.id) } diff --git a/spec/frontend/jobs/store/actions_spec.js b/spec/frontend/jobs/store/actions_spec.js index ade755045a5ac18b5294b7be072dbb91a616b817..c6b8735c6495b2279c2ef6035e0ff787496f25ac 100644 --- a/spec/frontend/jobs/store/actions_spec.js +++ b/spec/frontend/jobs/store/actions_spec.js @@ -27,7 +27,6 @@ import { hideSidebar, showSidebar, toggleSidebar, - triggerManualJob, } from '~/jobs/store/actions'; import state from '~/jobs/store/state'; import * as types from '~/jobs/store/mutation_types'; @@ -159,32 +158,6 @@ describe('Job State actions', () => { ); }); }); - - it('fetchTrace is called only if the job has started or has a trace', (done) => { - mock.onGet(`${TEST_HOST}/endpoint.json`).replyOnce(200, { id: 121212, name: 'karma' }); - - mockedState.job.started = true; - - testAction( - fetchJob, - null, - mockedState, - [], - [ - { - type: 'requestJob', - }, - { - payload: { id: 121212, name: 'karma' }, - type: 'receiveJobSuccess', - }, - { - type: 'fetchTrace', - }, - ], - done, - ); - }); }); describe('receiveJobSuccess', () => { @@ -536,43 +509,4 @@ describe('Job State actions', () => { ); }); }); - - describe('triggerManualJob', () => { - let mock; - - beforeEach(() => { - mock = new MockAdapter(axios); - }); - - afterEach(() => { - mock.restore(); - }); - - it('should dispatch fetchTrace', (done) => { - const playManualJobEndpoint = `${TEST_HOST}/manual-job/jobs/1000/play`; - - mock.onPost(playManualJobEndpoint).reply(200); - - mockedState.job = { - status: { - action: { - path: playManualJobEndpoint, - }, - }, - }; - - testAction( - triggerManualJob, - [{ id: '1', key: 'test_var', secret_value: 'test_value' }], - mockedState, - [], - [ - { - type: 'fetchTrace', - }, - ], - done, - ); - }); - }); }); diff --git a/spec/frontend/jobs/store/mutations_spec.js b/spec/frontend/jobs/store/mutations_spec.js index a8146ba93eb196d0a8917824ba581ba9ebcfb5d0..608abc8f7c4d3aecfad4001c000fa22ee332b09f 100644 --- a/spec/frontend/jobs/store/mutations_spec.js +++ b/spec/frontend/jobs/store/mutations_spec.js @@ -153,7 +153,6 @@ describe('Jobs Store Mutations', () => { mutations[types.SET_TRACE_TIMEOUT](stateCopy, id); expect(stateCopy.traceTimeout).toEqual(id); - expect(stateCopy.isTraceComplete).toBe(false); }); });