diff --git a/app/assets/javascripts/jobs/components/job_app.vue b/app/assets/javascripts/jobs/components/job_app.vue
index c7d4d7c4b9b..36701a95673 100644
--- a/app/assets/javascripts/jobs/components/job_app.vue
+++ b/app/assets/javascripts/jobs/components/job_app.vue
@@ -18,6 +18,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';
export default {
name: 'JobPageApp',
@@ -29,10 +30,7 @@ export default {
EnvironmentsBlock,
ErasedBlock,
Icon,
- Log: () =>
- gon && gon.features && gon.features.jobLogJson
- ? import('./job_log_json.vue')
- : import('./job_log.vue'),
+ Log: () => (isNewJobLogActive() ? import('./job_log_json.vue') : import('./job_log.vue')),
LogTopBar,
StuckBlock,
UnmetPrerequisitesBlock,
diff --git a/app/assets/javascripts/jobs/components/log/log.vue b/app/assets/javascripts/jobs/components/log/log.vue
new file mode 100644
index 00000000000..5db866afe5a
--- /dev/null
+++ b/app/assets/javascripts/jobs/components/log/log.vue
@@ -0,0 +1,45 @@
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/app/assets/javascripts/jobs/store/actions.js b/app/assets/javascripts/jobs/store/actions.js
index a2daef96a2d..41cc5a181dc 100644
--- a/app/assets/javascripts/jobs/store/actions.js
+++ b/app/assets/javascripts/jobs/store/actions.js
@@ -177,6 +177,14 @@ export const receiveTraceError = ({ commit }) => {
clearTimeout(traceTimeout);
flash(__('An error occurred while fetching the job log.'));
};
+/**
+ * When the user clicks a collpasible line in the job
+ * log, we commit a mutation to update the state
+ *
+ * @param {Object} section
+ */
+export const toggleCollapsibleLine = ({ commit }, section) =>
+ commit(types.TOGGLE_COLLAPSIBLE_LINE, section);
/**
* Jobs list on sidebar - depend on stages dropdown
diff --git a/app/assets/javascripts/jobs/store/mutation_types.js b/app/assets/javascripts/jobs/store/mutation_types.js
index 39146b2eefd..858fa3b73ab 100644
--- a/app/assets/javascripts/jobs/store/mutation_types.js
+++ b/app/assets/javascripts/jobs/store/mutation_types.js
@@ -23,6 +23,7 @@ export const REQUEST_TRACE = 'REQUEST_TRACE';
export const STOP_POLLING_TRACE = 'STOP_POLLING_TRACE';
export const RECEIVE_TRACE_SUCCESS = 'RECEIVE_TRACE_SUCCESS';
export const RECEIVE_TRACE_ERROR = 'RECEIVE_TRACE_ERROR';
+export const TOGGLE_COLLAPSIBLE_LINE = 'TOGGLE_COLLAPSIBLE_LINE';
export const SET_SELECTED_STAGE = 'SET_SELECTED_STAGE';
export const REQUEST_JOBS_FOR_STAGE = 'REQUEST_JOBS_FOR_STAGE';
diff --git a/app/assets/javascripts/jobs/store/mutations.js b/app/assets/javascripts/jobs/store/mutations.js
index ad08f27b147..540c3e2ad69 100644
--- a/app/assets/javascripts/jobs/store/mutations.js
+++ b/app/assets/javascripts/jobs/store/mutations.js
@@ -1,4 +1,6 @@
+import Vue from 'vue';
import * as types from './mutation_types';
+import { logLinesParser, updateIncrementalTrace, isNewJobLogActive } from './utils';
export default {
[types.SET_JOB_ENDPOINT](state, endpoint) {
@@ -23,14 +25,24 @@ export default {
}
if (log.append) {
- state.trace += log.html;
+ if (isNewJobLogActive()) {
+ state.originalTrace = state.originalTrace.concat(log.trace);
+ state.trace = updateIncrementalTrace(state.originalTrace, state.trace, log.lines);
+ } else {
+ state.trace += log.html;
+ }
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 `undefined`
- state.trace = log.html || state.trace;
+ if (isNewJobLogActive()) {
+ state.originalTrace = log.lines || state.trace;
+ state.trace = logLinesParser(log.lines) || state.trace;
+ } else {
+ state.trace = log.html || state.trace;
+ }
state.traceSize = log.size || state.traceSize;
}
@@ -57,6 +69,18 @@ export default {
state.isTraceComplete = true;
},
+ /**
+ * Instead of filtering the array of lines to find the one that must be updated
+ * we use Vue.set to make this process more performant
+ *
+ * https://vuex.vuejs.org/guide/mutations.html#mutations-follow-vue-s-reactivity-rules
+ * @param {Object} state
+ * @param {Object} section
+ */
+ [types.TOGGLE_COLLAPSIBLE_LINE](state, section) {
+ Vue.set(section, 'isClosed', !section.isClosed);
+ },
+
[types.REQUEST_JOB](state) {
state.isLoading = true;
},
diff --git a/app/assets/javascripts/jobs/store/state.js b/app/assets/javascripts/jobs/store/state.js
index 6019214e62c..585878f8240 100644
--- a/app/assets/javascripts/jobs/store/state.js
+++ b/app/assets/javascripts/jobs/store/state.js
@@ -1,3 +1,5 @@
+import { isNewJobLogActive } from '../store/utils';
+
export default () => ({
jobEndpoint: null,
traceEndpoint: null,
@@ -16,7 +18,8 @@ export default () => ({
// Used to check if we should keep the automatic scroll
isScrolledToBottomBeforeReceivingTrace: true,
- trace: '',
+ trace: isNewJobLogActive() ? [] : '',
+ originalTrace: [],
isTraceComplete: false,
traceSize: 0,
isTraceSizeVisible: false,
diff --git a/app/assets/javascripts/jobs/store/utils.js b/app/assets/javascripts/jobs/store/utils.js
index de7de92ed2e..f6a87b9a212 100644
--- a/app/assets/javascripts/jobs/store/utils.js
+++ b/app/assets/javascripts/jobs/store/utils.js
@@ -11,15 +11,16 @@
* @param {Array} lines
* @returns {Array}
*/
-export default (lines = []) =>
+export const logLinesParser = (lines = [], lineNumberStart) =>
lines.reduce((acc, line, index) => {
+ const lineNumber = lineNumberStart ? lineNumberStart + index : index;
if (line.section_header) {
acc.push({
isClosed: true,
isHeader: true,
line: {
...line,
- lineNumber: index,
+ lineNumber,
},
lines: [],
@@ -27,14 +28,59 @@ export default (lines = []) =>
} else if (acc.length && acc[acc.length - 1].isHeader) {
acc[acc.length - 1].lines.push({
...line,
- lineNumber: index,
+ lineNumber,
});
} else {
acc.push({
...line,
- lineNumber: index,
+ lineNumber,
});
}
return acc;
}, []);
+
+/**
+ * When the trace is not complete, backend may send the last received line
+ * in the new response.
+ *
+ * We need to check if that is the case by looking for the offset property
+ * before parsing the incremental part
+ *
+ * @param array originalTrace
+ * @param array oldLog
+ * @param array newLog
+ */
+export const updateIncrementalTrace = (originalTrace = [], oldLog = [], newLog = []) => {
+ const firstLine = newLog[0];
+ const firstLineOffset = firstLine.offset;
+
+ // We are going to return a new array,
+ // let's make a shallow copy to make sure we
+ // are not updating the state outside of a mutation first.
+ const cloneOldLog = [...oldLog];
+
+ const lastIndex = cloneOldLog.length - 1;
+ const lastLine = cloneOldLog[lastIndex];
+
+ // The last line may be inside a collpasible section
+ // If it is, we use the not parsed saved log, remove the last element
+ // and parse the first received part togheter with the incremental log
+ if (
+ lastLine.isHeader &&
+ (lastLine.line.offset === firstLineOffset ||
+ (lastLine.lines.length &&
+ lastLine.lines[lastLine.lines.length - 1].offset === firstLineOffset))
+ ) {
+ const cloneOriginal = [...originalTrace];
+ cloneOriginal.splice(cloneOriginal.length - 1);
+ return logLinesParser(cloneOriginal.concat(newLog));
+ } else if (lastLine.offset === firstLineOffset) {
+ cloneOldLog.splice(lastIndex);
+ return cloneOldLog.concat(logLinesParser(newLog, cloneOldLog.length));
+ }
+ // there are no matches, let's parse the new log and return them together
+ return cloneOldLog.concat(logLinesParser(newLog, cloneOldLog.length));
+};
+
+export const isNewJobLogActive = () => gon && gon.features && gon.features.jobLogJson;
diff --git a/spec/frontend/jobs/store/mutations_spec.js b/spec/frontend/jobs/store/mutations_spec.js
index 343301b8716..8e5ab4b229a 100644
--- a/spec/frontend/jobs/store/mutations_spec.js
+++ b/spec/frontend/jobs/store/mutations_spec.js
@@ -97,6 +97,14 @@ describe('Jobs Store Mutations', () => {
});
});
+ describe('TOGGLE_COLLAPSIBLE_LINE', () => {
+ it('toggles the `isClosed` property of the provided object', () => {
+ const section = { isClosed: true };
+ mutations[types.TOGGLE_COLLAPSIBLE_LINE](stateCopy, section);
+ expect(section.isClosed).toEqual(false);
+ });
+ });
+
describe('REQUEST_JOB', () => {
it('sets isLoading to true', () => {
mutations[types.REQUEST_JOB](stateCopy);
diff --git a/spec/frontend/jobs/store/utils_spec.js b/spec/frontend/jobs/store/utils_spec.js
index 9e81558f8c2..1b96f95b5a5 100644
--- a/spec/frontend/jobs/store/utils_spec.js
+++ b/spec/frontend/jobs/store/utils_spec.js
@@ -1,60 +1,261 @@
-import linesParser from '~/jobs/store/utils';
+import { logLinesParser, updateIncrementalTrace } from '~/jobs/store/utils';
-describe('linesParser', () => {
- const mockData = [
- {
- offset: 1001,
- content: [{ text: ' on docker-auto-scale-com 8a6210b8' }],
- },
- {
- offset: 1002,
- content: [
- {
- text:
- 'Using Docker executor with image dev.gitlab.org:5005/gitlab/gitlab-build-images:ruby-2.6.3-golang-1.11-git-2.22-chrome-73.0-node-12.x-yarn-1.16-postgresql-9.6-graphicsmagick-1.3.33',
- },
- ],
- sections: ['prepare-executor'],
- section_header: true,
- },
- {
- offset: 1003,
- content: [{ text: 'Starting service postgres:9.6.14 ...' }],
- sections: ['prepare-executor'],
- },
- {
- offset: 1004,
- content: [{ text: 'Pulling docker image postgres:9.6.14 ...', style: 'term-fg-l-green' }],
- sections: ['prepare-executor'],
- },
- ];
+describe('Jobs Store Utils', () => {
+ describe('logLinesParser', () => {
+ const mockData = [
+ {
+ offset: 1001,
+ content: [{ text: ' on docker-auto-scale-com 8a6210b8' }],
+ },
+ {
+ offset: 1002,
+ content: [
+ {
+ text:
+ 'Using Docker executor with image dev.gitlab.org:5005/gitlab/gitlab-build-images:ruby-2.6.3-golang-1.11-git-2.22-chrome-73.0-node-12.x-yarn-1.16-postgresql-9.6-graphicsmagick-1.3.33',
+ },
+ ],
+ sections: ['prepare-executor'],
+ section_header: true,
+ },
+ {
+ offset: 1003,
+ content: [{ text: 'Starting service postgres:9.6.14 ...' }],
+ sections: ['prepare-executor'],
+ },
+ {
+ offset: 1004,
+ content: [{ text: 'Pulling docker image postgres:9.6.14 ...', style: 'term-fg-l-green' }],
+ sections: ['prepare-executor'],
+ },
+ ];
- let result;
+ let result;
- beforeEach(() => {
- result = linesParser(mockData);
- });
+ beforeEach(() => {
+ result = logLinesParser(mockData);
+ });
- describe('regular line', () => {
- it('adds a lineNumber property with correct index', () => {
- expect(result[0].lineNumber).toEqual(0);
- expect(result[1].line.lineNumber).toEqual(1);
+ describe('regular line', () => {
+ it('adds a lineNumber property with correct index', () => {
+ expect(result[0].lineNumber).toEqual(0);
+ expect(result[1].line.lineNumber).toEqual(1);
+ });
+ });
+
+ describe('collpasible section', () => {
+ it('adds a `isClosed` property', () => {
+ expect(result[1].isClosed).toEqual(true);
+ });
+
+ it('adds a `isHeader` property', () => {
+ expect(result[1].isHeader).toEqual(true);
+ });
+
+ it('creates a lines array property with the content of the collpasible section', () => {
+ expect(result[1].lines.length).toEqual(2);
+ expect(result[1].lines[0].content).toEqual(mockData[2].content);
+ expect(result[1].lines[1].content).toEqual(mockData[3].content);
+ });
});
});
- describe('collpasible section', () => {
- it('adds a `isClosed` property', () => {
- expect(result[1].isClosed).toEqual(true);
+ describe('updateIncrementalTrace', () => {
+ const originalTrace = [
+ {
+ offset: 1,
+ content: [
+ {
+ text: 'Downloading',
+ },
+ ],
+ },
+ ];
+
+ describe('without repeated section', () => {
+ it('concats and parses both arrays', () => {
+ const oldLog = logLinesParser(originalTrace);
+ const newLog = [
+ {
+ offset: 2,
+ content: [
+ {
+ text: 'log line',
+ },
+ ],
+ },
+ ];
+ const result = updateIncrementalTrace(originalTrace, oldLog, newLog);
+
+ expect(result).toEqual([
+ {
+ offset: 1,
+ content: [
+ {
+ text: 'Downloading',
+ },
+ ],
+ lineNumber: 0,
+ },
+ {
+ offset: 2,
+ content: [
+ {
+ text: 'log line',
+ },
+ ],
+ lineNumber: 1,
+ },
+ ]);
+ });
});
- it('adds a `isHeader` property', () => {
- expect(result[1].isHeader).toEqual(true);
+ describe('with regular line repeated offset', () => {
+ it('updates the last line and formats with the incremental part', () => {
+ const oldLog = logLinesParser(originalTrace);
+ const newLog = [
+ {
+ offset: 1,
+ content: [
+ {
+ text: 'log line',
+ },
+ ],
+ },
+ ];
+ const result = updateIncrementalTrace(originalTrace, oldLog, newLog);
+
+ expect(result).toEqual([
+ {
+ offset: 1,
+ content: [
+ {
+ text: 'log line',
+ },
+ ],
+ lineNumber: 0,
+ },
+ ]);
+ });
});
- it('creates a lines array property with the content of the collpasible section', () => {
- expect(result[1].lines.length).toEqual(2);
- expect(result[1].lines[0].content).toEqual(mockData[2].content);
- expect(result[1].lines[1].content).toEqual(mockData[3].content);
+ describe('with header line repeated', () => {
+ it('updates the header line and formats with the incremental part', () => {
+ const headerTrace = [
+ {
+ offset: 1,
+ section_header: true,
+ content: [
+ {
+ text: 'log line',
+ },
+ ],
+ sections: ['section'],
+ },
+ ];
+ const oldLog = logLinesParser(headerTrace);
+ const newLog = [
+ {
+ offset: 1,
+ section_header: true,
+ content: [
+ {
+ text: 'updated log line',
+ },
+ ],
+ sections: ['section'],
+ },
+ ];
+ const result = updateIncrementalTrace(headerTrace, oldLog, newLog);
+
+ expect(result).toEqual([
+ {
+ isClosed: true,
+ isHeader: true,
+ line: {
+ offset: 1,
+ section_header: true,
+ content: [
+ {
+ text: 'updated log line',
+ },
+ ],
+ sections: ['section'],
+ lineNumber: 0,
+ },
+ lines: [],
+ },
+ ]);
+ });
+ });
+
+ describe('with collapsible line repeated', () => {
+ it('updates the collapsible line and formats with the incremental part', () => {
+ const collapsibleTrace = [
+ {
+ offset: 1,
+ section_header: true,
+ content: [
+ {
+ text: 'log line',
+ },
+ ],
+ sections: ['section'],
+ },
+ {
+ offset: 2,
+ content: [
+ {
+ text: 'log line',
+ },
+ ],
+ sections: ['section'],
+ },
+ ];
+ const oldLog = logLinesParser(collapsibleTrace);
+ const newLog = [
+ {
+ offset: 2,
+ content: [
+ {
+ text: 'updated log line',
+ },
+ ],
+ sections: ['section'],
+ },
+ ];
+ const result = updateIncrementalTrace(collapsibleTrace, oldLog, newLog);
+
+ expect(result).toEqual([
+ {
+ isClosed: true,
+ isHeader: true,
+ line: {
+ offset: 1,
+ section_header: true,
+ content: [
+ {
+ text: 'log line',
+ },
+ ],
+ sections: ['section'],
+ lineNumber: 0,
+ },
+ lines: [
+ {
+ offset: 2,
+ content: [
+ {
+ text: 'updated log line',
+ },
+ ],
+ sections: ['section'],
+ lineNumber: 1,
+ },
+ ],
+ },
+ ]);
+ });
});
});
});
diff --git a/spec/javascripts/jobs/components/log/log_spec.js b/spec/javascripts/jobs/components/log/log_spec.js
new file mode 100644
index 00000000000..469bbf6714d
--- /dev/null
+++ b/spec/javascripts/jobs/components/log/log_spec.js
@@ -0,0 +1,77 @@
+import { mount, createLocalVue } from '@vue/test-utils';
+import Vuex from 'vuex';
+import { logLinesParser } from '~/jobs/store/utils';
+import Log from '~/jobs/components/log/log.vue';
+import { jobLog } from './mock_data';
+
+describe('Job Log', () => {
+ let wrapper;
+ let actions;
+ let state;
+ let store;
+
+ const localVue = createLocalVue();
+ localVue.use(Vuex);
+
+ const createComponent = () => {
+ wrapper = mount(Log, {
+ sync: false,
+ localVue,
+ store,
+ });
+ };
+
+ beforeEach(() => {
+ actions = {
+ toggleCollapsibleLine: () => {},
+ };
+
+ state = {
+ trace: logLinesParser(jobLog),
+ traceEndpoint: 'jobs/id',
+ };
+
+ store = new Vuex.Store({
+ actions,
+ state,
+ });
+
+ createComponent();
+ });
+
+ afterEach(() => {
+ wrapper.destroy();
+ });
+
+ describe('line numbers', () => {
+ it('renders a line number for each open line', () => {
+ expect(wrapper.find('#L1').text()).toBe('1');
+ expect(wrapper.find('#L2').text()).toBe('2');
+ expect(wrapper.find('#L3').text()).toBe('3');
+ });
+
+ it('links to the provided path and correct line number', () => {
+ expect(wrapper.find('#L1').attributes('href')).toBe(`${state.traceEndpoint}#L1`);
+ });
+ });
+
+ describe('collapsible sections', () => {
+ it('renders a clickable header section', () => {
+ expect(wrapper.find('.collapsible-line').attributes('role')).toBe('button');
+ });
+
+ it('renders an icon with the closed state', () => {
+ expect(wrapper.find('.collapsible-line svg').classes()).toContain('ic-angle-right');
+ });
+
+ describe('on click header section', () => {
+ it('calls toggleCollapsibleLine', () => {
+ spyOn(wrapper.vm, 'toggleCollapsibleLine').and.callThrough();
+
+ wrapper.find('.collapsible-line').trigger('click');
+
+ expect(wrapper.vm.toggleCollapsibleLine).toHaveBeenCalled();
+ });
+ });
+ });
+});
diff --git a/spec/javascripts/jobs/components/log/mock_data.js b/spec/javascripts/jobs/components/log/mock_data.js
new file mode 100644
index 00000000000..54a6d31b278
--- /dev/null
+++ b/spec/javascripts/jobs/components/log/mock_data.js
@@ -0,0 +1,26 @@
+// eslint-disable-next-line import/prefer-default-export
+export const jobLog = [
+ {
+ offset: 1000,
+ content: [{ text: 'Running with gitlab-runner 12.1.0 (de7731dd)' }],
+ },
+ {
+ offset: 1001,
+ content: [{ text: ' on docker-auto-scale-com 8a6210b8' }],
+ },
+ {
+ offset: 1002,
+ content: [
+ {
+ text: 'Using Docker executor with image dev.gitlab.org3',
+ },
+ ],
+ sections: ['prepare-executor'],
+ section_header: true,
+ },
+ {
+ offset: 1003,
+ content: [{ text: 'Starting service postgres:9.6.14 ...', style: 'text-green' }],
+ sections: ['prepare-executor'],
+ },
+];
diff --git a/spec/javascripts/jobs/store/actions_spec.js b/spec/javascripts/jobs/store/actions_spec.js
index 7b96df85b82..91d1942bdbf 100644
--- a/spec/javascripts/jobs/store/actions_spec.js
+++ b/spec/javascripts/jobs/store/actions_spec.js
@@ -16,6 +16,7 @@ import {
stopPollingTrace,
receiveTraceSuccess,
receiveTraceError,
+ toggleCollapsibleLine,
requestJobsForStage,
fetchJobsForStage,
receiveJobsForStageSuccess,
@@ -303,6 +304,19 @@ describe('Job State actions', () => {
});
});
+ describe('toggleCollapsibleLine', () => {
+ it('should commit TOGGLE_COLLAPSIBLE_LINE mutation ', done => {
+ testAction(
+ toggleCollapsibleLine,
+ { isClosed: true },
+ mockedState,
+ [{ type: types.TOGGLE_COLLAPSIBLE_LINE, payload: { isClosed: true } }],
+ [],
+ done,
+ );
+ });
+ });
+
describe('requestJobsForStage', () => {
it('should commit REQUEST_JOBS_FOR_STAGE mutation ', done => {
testAction(