Automatic merge of gitlab-org/gitlab-ce master

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,

app/assets/javascripts/jobs/components/log/log.vue

+<script>
+import { mapState, mapActions } from 'vuex';
+import LogLine from './line.vue';
+import LogLineHeader from './line_header.vue';
+
+export default {
+  components: {
+    LogLine,
+    LogLineHeader,
+  },
+  computed: {
+    ...mapState(['traceEndpoint', 'trace']),
+  },
+  methods: {
+    ...mapActions(['toggleCollapsibleLine']),
+    handleOnClickCollapsibleLine(section) {
+      this.toggleCollapsibleLine(section);
+    },
+  },
+};
+</script>
+<template>
+  <code class="job-log">
+    <template v-for="(section, index) in trace">
+      <template v-if="section.isHeader">
+        <log-line-header
+          :key="`collapsible-${index}`"
+          :line="section.line"
+          :path="traceEndpoint"
+          :is-closed="section.isClosed"
+          @toggleLine="handleOnClickCollapsibleLine(section)"
+        />
+        <template v-if="!section.isClosed">
+          <log-line
+            v-for="line in section.lines"
+            :key="line.offset"
+            :line="line"
+            :path="traceEndpoint"
+          />
+        </template>
+      </template>
+      <log-line v-else :key="section.offset" :line="section" :path="traceEndpoint" />
+    </template>
+  </code>
+</template>

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

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';

app/assets/javascripts/jobs/store/mutations.js

+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;
   },

app/assets/javascripts/jobs/store/state.js

+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,

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;

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);

spec/frontend/jobs/store/utils_spec.js

-import linesParser 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'],
-    },
-  ];
-
-  let result;
-
-  beforeEach(() => {
-    result = linesParser(mockData);
-  });
+import { logLinesParser, updateIncrementalTrace } from '~/jobs/store/utils';
+
+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;
+
+    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('collpasible section', () => {
+      it('adds a `isClosed` property', () => {
+        expect(result[1].isClosed).toEqual(true);
+      });
 
-  describe('regular line', () => {
-    it('adds a lineNumber property with correct index', () => {
-      expect(result[0].lineNumber).toEqual(0);
-      expect(result[1].line.lineNumber).toEqual(1);
+      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,
+          },
+        ]);
+      });
+    });
+
+    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('adds a `isHeader` property', () => {
-      expect(result[1].isHeader).toEqual(true);
+    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: [],
+          },
+        ]);
+      });
     });
 
-    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 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,
+              },
+            ],
+          },
+        ]);
+      });
     });
   });
 });

spec/javascripts/jobs/components/log/log_spec.js

+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();
+      });
+    });
+  });
+});

spec/javascripts/jobs/components/log/mock_data.js

+// 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'],
+  },
+];

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(