Skip to content

G
gitlab-ce
Commit 3b291822 authored by Shinya Maeda's avatar Shinya Maeda
Browse files
Options

Merge branch 'remove-ff-job-log-json' into 'master' 

Remove FF job_log_json flag from BE and FE

See merge request gitlab-org/gitlab!34538
parents 8812a456 9ad5bb11
Hide whitespace changes
Inline Side-by-side
Showing with 70 additions and 483 deletions
app/assets/javascripts/jobs/components/job_app.vue
View file @ 3b291822
......@@ -17,7 +17,7 @@ import UnmetPrerequisitesBlock from './unmet_prerequisites_block.vue';
import Sidebar from './sidebar.vue';
import { sprintf } from '~/locale';
import delayedJobMixin from '../mixins/delayed_job_mixin';
import { isNewJobLogActive } from '../store/utils';
import Log from './log/log.vue';
export default {
name: 'JobPageApp',
......@@ -28,7 +28,7 @@ export default {
EnvironmentsBlock,
ErasedBlock,
Icon,
Log: () => (isNewJobLogActive() ? import('./log/log.vue') : import('./job_log.vue')),
Log,
LogTopBar,
StuckBlock,
UnmetPrerequisitesBlock,
......
app/assets/javascripts/jobs/components/job_log.vue deleted 100644 → 0
View file @ 8812a456
<script>
import { mapState, mapActions } from 'vuex';
export default {
name: 'JobLog',
props: {
trace: {
type: String,
required: true,
},
isComplete: {
type: Boolean,
required: true,
},
},
computed: {
...mapState(['isScrolledToBottomBeforeReceivingTrace']),
},
updated() {
this.$nextTick(() => {
this.handleScrollDown();
});
},
mounted() {
this.$nextTick(() => {
this.handleScrollDown();
});
},
methods: {
...mapActions(['scrollBottom']),
/**
* The job log is sent in HTML, which means we need to use `v-html` to render it
* Using the updated hook with $nextTick is not enough to wait for the DOM to be updated
* in this case because it runs before `v-html` has finished running, since there's no
* Vue binding.
* In order to scroll the page down after `v-html` has finished, we need to use setTimeout
*/
handleScrollDown() {
if (this.isScrolledToBottomBeforeReceivingTrace) {
setTimeout(() => {
this.scrollBottom();
}, 0);
}
},
},
};
</script>
<template>
<pre class="js-build-trace build-trace qa-build-trace">
<code class="bash" v-html="trace">
</code>
<div v-if="!isComplete" class="js-log-animation build-loader-animation">
<div class="dot"></div>
<div class="dot"></div>
<div class="dot"></div>
</div>
</pre>
</template>
app/assets/javascripts/jobs/store/mutations.js
View file @ 3b291822
import Vue from 'vue';
import * as types from './mutation_types';
import { logLinesParser, updateIncrementalTrace, isNewJobLogActive } from './utils';
import { logLinesParser, updateIncrementalTrace } from './utils';
export default {
[types.SET_JOB_ENDPOINT](state, endpoint) {
......@@ -25,22 +25,16 @@ export default {
}
if (log.append) {
if (isNewJobLogActive()) {
state.trace = log.lines ? updateIncrementalTrace(log.lines, state.trace) : state.trace;
} else {
state.trace += log.html;
}
state.trace = log.lines ? updateIncrementalTrace(log.lines, state.trace) : state.trace;
state.traceSize += log.size;
} else {
// When the job still does not have a trace
// the trace response will not have a defined
// html or size. We keep the old value otherwise these
// will be set to `null`
if (isNewJobLogActive()) {
state.trace = log.lines ? logLinesParser(log.lines) : state.trace;
} else {
state.trace = log.html || state.trace;
}
state.trace = log.lines ? logLinesParser(log.lines) : state.trace;
state.traceSize = log.size || state.traceSize;
}
......
app/assets/javascripts/jobs/store/state.js
View file @ 3b291822
import { isNewJobLogActive } from './utils';
export default () => ({
jobEndpoint: null,
traceEndpoint: null,
......@@ -18,7 +16,7 @@ export default () => ({
// Used to check if we should keep the automatic scroll
isScrolledToBottomBeforeReceivingTrace: true,
trace: isNewJobLogActive() ? [] : '',
trace: [],
isTraceComplete: false,
traceSize: 0,
isTraceSizeVisible: false,
......
app/assets/javascripts/jobs/store/utils.js
View file @ 3b291822
......@@ -177,5 +177,3 @@ export const updateIncrementalTrace = (newLog = [], oldParsed = []) => {
return logLinesParser(newLog, parsedLog);
};
export const isNewJobLogActive = () => gon && gon.features && gon.features.jobLogJson;
app/controllers/projects/jobs_controller.rb
View file @ 3b291822
......@@ -11,9 +11,6 @@ class Projects::JobsController < Projects::ApplicationController
before_action :authorize_erase_build!, only: [:erase]
before_action :authorize_use_build_terminal!, only: [:terminal, :terminal_websocket_authorize]
before_action :verify_api_request!, only: :terminal_websocket_authorize
before_action only: [:show] do
push_frontend_feature_flag(:job_log_json, project, default_enabled: true)
end
before_action :authorize_create_proxy_build!, only: :proxy_websocket_authorize
before_action :verify_proxy_request!, only: :proxy_websocket_authorize
......@@ -55,15 +52,10 @@ class Projects::JobsController < Projects::ApplicationController
format.json do
build.trace.being_watched!
# TODO: when the feature flag is removed we should not pass
# content_format to serialize method.
content_format = Feature.enabled?(:job_log_json, @project, default_enabled: true) ? :json : :html
build_trace = Ci::BuildTrace.new(
build: @build,
stream: stream,
state: params[:state],
content_format: content_format)
state: params[:state])
render json: BuildTraceSerializer
.new(project: @project, current_user: @current_user)
......
app/models/ci/build_trace.rb
View file @ 3b291822
......@@ -2,40 +2,22 @@
module Ci
class BuildTrace
CONVERTERS = {
html: Gitlab::Ci::Ansi2html,
json: Gitlab::Ci::Ansi2json
}.freeze
attr_reader :trace, :build
delegate :state, :append, :truncated, :offset, :size, :total, to: :trace, allow_nil: true
delegate :id, :status, :complete?, to: :build, prefix: true
def initialize(build:, stream:, state:, content_format:)
def initialize(build:, stream:, state:)
@build = build
@content_format = content_format
if stream.valid?
stream.limit
@trace = CONVERTERS.fetch(content_format).convert(stream.stream, state)
@trace = Gitlab::Ci::Ansi2json.convert(stream.stream, state)
end
end
def json?
@content_format == :json
end
def html?
@content_format == :html
end
def json_lines
@trace&.lines if json?
end
def html_lines
@trace&.html if html?
def lines
@trace&.lines
end
end
end
app/serializers/build_trace_entity.rb
View file @ 3b291822
......@@ -12,6 +12,5 @@ class BuildTraceEntity < Grape::Entity
expose :size
expose :total
expose :json_lines, as: :lines, if: ->(*) { object.json? }
expose :html_lines, as: :html, if: ->(*) { object.html? }
expose :lines
end
changelogs/unreleased/remove-ff-job-log-json.yml 0 → 100644
View file @ 3b291822
---
title: Remove legacy job log rendering
merge_request: 34538
author:
type: other
qa/qa/specs/features/ee/browser_ui/secure/create_merge_request_with_secure_spec.rb
View file @ 3b291822
......@@ -13,8 +13,6 @@ module QA
after do
@runner.remove_via_api! if @runner
Runtime::Feature.enable('job_log_json') if @job_log_json_flag_enabled
end
before do
......
qa/qa/specs/features/ee/browser_ui/secure/vulnerability_management_spec.rb
View file @ 3b291822
......@@ -13,8 +13,6 @@ module QA
after(:all) do
@runner.remove_via_api!
Runtime::Feature.enable('job_log_json') if @job_log_json_flag_enabled
end
before(:all) do
......
spec/controllers/projects/jobs_controller_spec.rb
View file @ 3b291822
......@@ -646,109 +646,6 @@ RSpec.describe Projects::JobsController, :clean_gitlab_redis_shared_state do
end
end
describe 'GET legacy trace.json' do
before do
stub_feature_flags(job_log_json: false)
get_trace
end
context 'when job has a trace artifact' do
let(:job) { create(:ci_build, :trace_artifact, pipeline: pipeline) }
it 'returns a trace' do
expect(response).to have_gitlab_http_status(:ok)
expect(json_response['id']).to eq job.id
expect(json_response['status']).to eq job.status
expect(json_response['state']).to be_present
expect(json_response['append']).not_to be_nil
expect(json_response['truncated']).not_to be_nil
expect(json_response['size']).to be_present
expect(json_response['total']).to be_present
expect(json_response['html']).to eq(job.trace.html)
end
end
context 'when job has a trace' do
let(:job) { create(:ci_build, :trace_live, pipeline: pipeline) }
it 'returns a trace' do
expect(response).to have_gitlab_http_status(:ok)
expect(json_response['id']).to eq job.id
expect(json_response['status']).to eq job.status
expect(json_response['html']).to eq('<span>BUILD TRACE</span>')
end
end
context 'when job has no traces' do
let(:job) { create(:ci_build, pipeline: pipeline) }
it 'returns no traces' do
expect(response).to have_gitlab_http_status(:ok)
expect(json_response['id']).to eq job.id
expect(json_response['status']).to eq job.status
expect(json_response['html']).to be_nil
end
end
context 'when job has a trace with ANSI sequence and Unicode' do
let(:job) { create(:ci_build, :unicode_trace_live, pipeline: pipeline) }
it 'returns a trace with Unicode' do
expect(response).to have_gitlab_http_status(:ok)
expect(json_response['id']).to eq job.id
expect(json_response['status']).to eq job.status
expect(json_response['html']).to include("ヾ(´༎ຶД༎ຶ`)ﾉ")
end
end
context 'when trace artifact is in ObjectStorage' do
let(:url) { 'http://object-storage/trace' }
let(:file_path) { expand_fixture_path('trace/sample_trace') }
let!(:job) { create(:ci_build, :success, :trace_artifact, pipeline: pipeline) }
before do
allow_any_instance_of(JobArtifactUploader).to receive(:file_storage?) { false }
allow_any_instance_of(JobArtifactUploader).to receive(:url) { url }
allow_any_instance_of(JobArtifactUploader).to receive(:size) { File.size(file_path) }
end
context 'when there are no network issues' do
before do
stub_remote_url_206(url, file_path)
get_trace
end
it 'returns a trace' do
expect(response).to have_gitlab_http_status(:ok)
expect(json_response['id']).to eq job.id
expect(json_response['status']).to eq job.status
expect(json_response['html']).to eq(job.trace.html)
end
end
context 'when there is a network issue' do
before do
stub_remote_url_500(url)
end
it 'returns a trace' do
expect { get_trace }.to raise_error(Gitlab::HttpIO::FailedToGetChunkError)
end
end
end
def get_trace
get :trace,
params: {
namespace_id: project.namespace,
project_id: project,
id: job.id
},
format: :json
end
end
describe 'GET status.json' do
let(:job) { create(:ci_build, pipeline: pipeline) }
let(:status) { job.detailed_status(double('user')) }
......
spec/features/projects/jobs_spec.rb
View file @ 3b291822
......@@ -837,7 +837,7 @@ RSpec.describe 'Jobs', :clean_gitlab_redis_shared_state do
it 'renders empty state' do
expect(page).to have_content(job.detailed_status(user).illustration[:title])
expect(page).not_to have_selector('.js-build-trace')
expect(page).not_to have_selector('.job-log')
expect(page).to have_content('This job has been canceled')
end
end
......@@ -852,7 +852,7 @@ RSpec.describe 'Jobs', :clean_gitlab_redis_shared_state do
it 'renders empty state' do
expect(page).to have_content(job.detailed_status(user).illustration[:title])
expect(page).not_to have_selector('.js-build-trace')
expect(page).not_to have_selector('.job-log')
expect(page).to have_content('This job has been skipped')
end
end
......
spec/frontend/jobs/components/job_app_spec.js
View file @ 3b291822
......@@ -397,132 +397,31 @@ describe('Job App', () => {
});
});
describe('trace output', () => {
describe('with append flag', () => {
it('appends the log content to the existing one', () =>
setupAndMount({
traceData: {
html: '<span>More<span>',
status: 'running',
state: 'newstate',
append: true,
complete: true,
},
})
.then(() => {
store.state.trace = 'Update';
return wrapper.vm.$nextTick();
})
.then(() => {
expect(
wrapper
.find('.js-build-trace')
.text()
.trim(),
).toEqual('Update');
}));
describe('trace controls', () => {
beforeEach(() =>
setupAndMount({
traceData: {
html: '<span>Update</span>',
status: 'success',
append: false,
size: 50,
total: 100,
complete: true,
},
}),
);
it('should render scroll buttons', () => {
expect(wrapper.find('.js-scroll-top').exists()).toBe(true);
expect(wrapper.find('.js-scroll-bottom').exists()).toBe(true);
});
describe('without append flag', () => {
it('replaces the trace', () =>
setupAndMount({
traceData: {
html: '<span>Different<span>',
status: 'running',
append: false,
complete: true,
},
}).then(() => {
expect(
wrapper
.find('.js-build-trace')
.text()
.trim(),
).toEqual('Different');
}));
});
describe('truncated information', () => {
describe('when size is less than total', () => {
it('shows information about truncated log', () => {
mock.onGet(`${props.pagePath}/trace.json`).reply(200, {
html: '<span>Update</span>',
status: 'success',
append: false,
size: 50,
total: 100,
complete: true,
});
return setupAndMount({
traceData: {
html: '<span>Update</span>',
status: 'success',
append: false,
size: 50,
total: 100,
complete: true,
},
}).then(() => {
expect(
wrapper
.find('.js-truncated-info')
.text()
.trim(),
).toContain('Showing last 50 bytes');
});
});
});
describe('when size is equal than total', () => {
it('does not show the truncated information', () =>
setupAndMount({
traceData: {
html: '<span>Update</span>',
status: 'success',
append: false,
size: 100,
total: 100,
complete: true,
},
}).then(() => {
expect(
wrapper
.find('.js-truncated-info')
.text()
.trim(),
).toEqual('');
}));
});
it('should render link to raw ouput', () => {
expect(wrapper.find('.js-raw-link-controller').exists()).toBe(true);
});
describe('trace controls', () => {
beforeEach(() =>
setupAndMount({
traceData: {
html: '<span>Update</span>',
status: 'success',
append: false,
size: 50,
total: 100,
complete: true,
},
}),
);
it('should render scroll buttons', () => {
expect(wrapper.find('.js-scroll-top').exists()).toBe(true);
expect(wrapper.find('.js-scroll-bottom').exists()).toBe(true);
});
it('should render link to raw ouput', () => {
expect(wrapper.find('.js-raw-link-controller').exists()).toBe(true);
});
it('should render link to erase job', () => {
expect(wrapper.find('.js-erase-link').exists()).toBe(true);
});
it('should render link to erase job', () => {
expect(wrapper.find('.js-erase-link').exists()).toBe(true);
});
});
});
spec/frontend/jobs/components/job_log_spec.js deleted 100644 → 0
View file @ 8812a456
import Vue from 'vue';
import { mountComponentWithStore } from 'helpers/vue_mount_component_helper';
import component from '~/jobs/components/job_log.vue';
import createStore from '~/jobs/store';
import { resetStore } from '../store/helpers';
describe('Job Log', () => {
const Component = Vue.extend(component);
let store;
let vm;
const trace =
'<span>Running with gitlab-runner 12.1.0 (de7731dd)<br/></span><span> on docker-auto-scale-com d5ae8d25<br/></span><div class="gl-mr-3" data-timestamp="1565502765" data-section="prepare-executor" role="button"></div><span class="section section-header js-s-prepare-executor">Using Docker executor with image ruby:2.6 ...<br/></span>';
beforeEach(() => {
store = createStore();
});
afterEach(() => {
resetStore(store);
vm.$destroy();
});
it('renders provided trace', () => {
vm = mountComponentWithStore(Component, {
props: {
trace,
isComplete: true,
},
store,
});
expect(vm.$el.querySelector('code').textContent).toContain(
'Running with gitlab-runner 12.1.0 (de7731dd)',
);
});
describe('while receiving trace', () => {
it('renders animation', () => {
vm = mountComponentWithStore(Component, {
props: {
trace,
isComplete: false,
},
store,
});
expect(vm.$el.querySelector('.js-log-animation')).not.toBeNull();
});
});
describe('when build trace has finishes', () => {
it('does not render animation', () => {
vm = mountComponentWithStore(Component, {
props: {
trace,
isComplete: true,
},
store,
});
expect(vm.$el.querySelector('.js-log-animation')).toBeNull();
});
});
});
spec/frontend/jobs/store/mutations_spec.js
View file @ 3b291822
......@@ -76,28 +76,15 @@ describe('Jobs Store Mutations', () => {
lines: [],
});
expect(stateCopy.trace).toEqual(html);
expect(stateCopy.traceSize).toEqual(511846);
expect(stateCopy.isTraceComplete).toEqual(true);
});
describe('with new job log', () => {
let stateWithNewLog;
beforeEach(() => {
gon.features = gon.features || {};
gon.features.jobLogJson = true;
stateWithNewLog = state();
});
afterEach(() => {
gon.features.jobLogJson = false;
});
describe('log.lines', () => {
describe('when append is true', () => {
it('sets the parsed log ', () => {
mutations[types.RECEIVE_TRACE_SUCCESS](stateWithNewLog, {
mutations[types.RECEIVE_TRACE_SUCCESS](stateCopy, {
append: true,
size: 511846,
complete: true,
......@@ -109,7 +96,7 @@ describe('Jobs Store Mutations', () => {
],
});
expect(stateWithNewLog.trace).toEqual([
expect(stateCopy.trace).toEqual([
{
offset: 1,
content: [{ text: 'Running with gitlab-runner 11.12.1 (5a147c92)' }],
......@@ -121,7 +108,7 @@ describe('Jobs Store Mutations', () => {
describe('when it is defined', () => {
it('sets the parsed log ', () => {
mutations[types.RECEIVE_TRACE_SUCCESS](stateWithNewLog, {
mutations[types.RECEIVE_TRACE_SUCCESS](stateCopy, {
append: false,
size: 511846,
complete: true,
......@@ -130,7 +117,7 @@ describe('Jobs Store Mutations', () => {
],
});
expect(stateWithNewLog.trace).toEqual([
expect(stateCopy.trace).toEqual([
{
offset: 0,
content: [{ text: 'Running with gitlab-runner 11.11.1 (5a147c92)' }],
......@@ -142,7 +129,7 @@ describe('Jobs Store Mutations', () => {
describe('when it is null', () => {
it('sets the default value', () => {
mutations[types.RECEIVE_TRACE_SUCCESS](stateWithNewLog, {
mutations[types.RECEIVE_TRACE_SUCCESS](stateCopy, {
append: true,
html,
size: 511846,
......@@ -150,7 +137,7 @@ describe('Jobs Store Mutations', () => {
lines: null,
});
expect(stateWithNewLog.trace).toEqual([]);
expect(stateCopy.trace).toEqual([]);
});
});
});
......
spec/models/ci/build_trace_spec.rb
View file @ 3b291822
......@@ -11,7 +11,7 @@ RSpec.describe Ci::BuildTrace do
Gitlab::Ci::Trace::Stream.new { data }
end
subject { described_class.new(build: build, stream: stream, state: state, content_format: content_format) }
subject { described_class.new(build: build, stream: stream, state: state) }
shared_examples 'delegates methods' do
it { is_expected.to delegate_method(:state).to(:trace) }
......@@ -25,29 +25,11 @@ RSpec.describe Ci::BuildTrace do
it { is_expected.to delegate_method(:complete?).to(:build).with_prefix }
end
context 'with :json content format' do
let(:content_format) { :json }
it_behaves_like 'delegates methods'
it_behaves_like 'delegates methods'
it { is_expected.to be_json }
it 'returns formatted trace' do
expect(subject.trace.lines).to eq([
{ offset: 0, content: [{ text: 'the-stream' }] }
])
end
end
context 'with :html content format' do
let(:content_format) { :html }
it_behaves_like 'delegates methods'
it { is_expected.to be_html }
it 'returns formatted trace' do
expect(subject.trace.html).to eq('<span>the-stream</span>')
end
it 'returns formatted trace' do
expect(subject.lines).to eq([
{ offset: 0, content: [{ text: 'the-stream' }] }
])
end
end
spec/serializers/build_trace_entity_spec.rb
View file @ 3b291822
......@@ -13,7 +13,7 @@ RSpec.describe BuildTraceEntity do
end
let(:build_trace) do
Ci::BuildTrace.new(build: build, stream: stream, content_format: content_format, state: nil)
Ci::BuildTrace.new(build: build, stream: stream, state: nil)
end
let(:entity) do
......@@ -22,42 +22,24 @@ RSpec.describe BuildTraceEntity do
subject { entity.as_json }
shared_examples 'includes build and trace metadata' do
it 'includes build attributes' do
expect(subject[:id]).to eq(build.id)
expect(subject[:status]).to eq(build.status)
expect(subject[:complete]).to eq(build.complete?)
end
it 'includes trace metadata' do
expect(subject).to include(:state)
expect(subject).to include(:append)
expect(subject).to include(:truncated)
expect(subject).to include(:offset)
expect(subject).to include(:size)
expect(subject).to include(:total)
end
it 'includes build attributes' do
expect(subject[:id]).to eq(build.id)
expect(subject[:status]).to eq(build.status)
expect(subject[:complete]).to eq(build.complete?)
end
context 'when content format is :json' do
let(:content_format) { :json }
it_behaves_like 'includes build and trace metadata'
it 'includes the trace content in json' do
expect(subject[:lines]).to eq([
{ offset: 0, content: [{ text: 'the-trace' }] }
])
end
it 'includes trace metadata' do
expect(subject).to include(:state)
expect(subject).to include(:append)
expect(subject).to include(:truncated)
expect(subject).to include(:offset)
expect(subject).to include(:size)
expect(subject).to include(:total)
end
context 'when content format is :html' do
let(:content_format) { :html }
it_behaves_like 'includes build and trace metadata'
it 'includes the trace content in json' do
expect(subject[:html]).to eq('<span>the-trace</span>')
end
it 'includes the trace content in json' do
expect(subject[:lines]).to eq([
{ offset: 0, content: [{ text: 'the-trace' }] }
])
end
end
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7