Merge branch '31161-incremental-and-line-number' into 'master'

Updates the incremental trace function

See merge request gitlab-org/gitlab!17765

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

@@ -26,8 +26,7 @@ export default {
 
     if (log.append) {
       if (isNewJobLogActive()) {
-        state.originalTrace = state.originalTrace.concat(log.trace);
-        state.trace = updateIncrementalTrace(state.originalTrace, state.trace, log.lines);
+        state.trace = updateIncrementalTrace(log.lines, state.trace);
       } else {
         state.trace += log.html;
       }
@@ -38,7 +37,6 @@ export default {
       // html or size. We keep the old value otherwise these
       // will be set to `undefined`
       if (isNewJobLogActive()) {
-        state.originalTrace = log.lines || state.trace;
         state.trace = logLinesParser(log.lines) || state.trace;
       } else {
         state.trace = log.html || state.trace;

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

@@ -19,7 +19,6 @@ export default () => ({
   isScrolledToBottomBeforeReceivingTrace: true,
 
   trace: isNewJobLogActive() ? [] : '',
-  originalTrace: [],
   isTraceComplete: false,
   traceSize: 0,
   isTraceSizeVisible: false,

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

@@ -63,6 +63,30 @@ export const isCollapsibleSection = (acc = [], last = {}, section = {}) =>
   !section.section_duration &&
   section.section === last.line.section;
 
+/**
+ * Returns the lineNumber of the last line in
+ * a parsed log
+ *
+ * @param Array acc
+ * @returns Number
+ */
+export const getIncrementalLineNumber = acc => {
+  let lineNumberValue;
+  const lastIndex = acc.length - 1;
+  const lastElement = acc[lastIndex];
+  const nestedLines = lastElement.lines;
+
+  if (lastElement.isHeader && !nestedLines.length && lastElement.line) {
+    lineNumberValue = lastElement.line.lineNumber;
+  } else if (lastElement.isHeader && nestedLines.length) {
+    lineNumberValue = nestedLines[nestedLines.length - 1].lineNumber;
+  } else {
+    lineNumberValue = lastElement.lineNumber;
+  }
+
+  return lineNumberValue === 0 ? 1 : lineNumberValue + 1;
+};
+
 /**
  * Parses the job log content into a structure usable by the template
  *
@@ -75,32 +99,35 @@ export const isCollapsibleSection = (acc = [], last = {}, section = {}) =>
  *    - adds the index as lineNumber
  *
  * @param Array lines
- * @param Number lineNumberStart
  * @param Array accumulator
  * @returns Array parsed log lines
  */
-export const logLinesParser = (lines = [], lineNumberStart, accumulator = []) =>
-  lines.reduce((acc, line, index) => {
-    const lineNumber = lineNumberStart ? lineNumberStart + index : index;
-    const last = acc[acc.length - 1];
-
-    // If the object is an header, we parse it into another structure
-    if (line.section_header) {
-      acc.push(parseHeaderLine(line, lineNumber));
-    } else if (isCollapsibleSection(acc, last, line)) {
-      // if the object belongs to a nested section, we append it to the new `lines` array of the
-      // previously formated header
-      last.lines.push(parseLine(line, lineNumber));
-    } else if (line.section_duration) {
-      // if the line has section_duration, we look for the correct header to add it
-      addDurationToHeader(acc, line);
-    } else {
-      // otherwise it's a regular line
-      acc.push(parseLine(line, lineNumber));
-    }
+export const logLinesParser = (lines = [], accumulator = []) =>
+  lines.reduce(
+    (acc, line, index) => {
+      const lineNumber = accumulator.length > 0 ? getIncrementalLineNumber(acc) : index;
+
+      const last = acc[acc.length - 1];
+
+      // If the object is an header, we parse it into another structure
+      if (line.section_header) {
+        acc.push(parseHeaderLine(line, lineNumber));
+      } else if (isCollapsibleSection(acc, last, line)) {
+        // if the object belongs to a nested section, we append it to the new `lines` array of the
+        // previously formated header
+        last.lines.push(parseLine(line, lineNumber));
+      } else if (line.section_duration) {
+        // if the line has section_duration, we look for the correct header to add it
+        addDurationToHeader(acc, line);
+      } else {
+        // otherwise it's a regular line
+        acc.push(parseLine(line, lineNumber));
+      }
 
-    return acc;
-  }, accumulator);
+      return acc;
+    },
+    [...accumulator],
+  );
 
 /**
  * Finds the repeated offset, removes the old one
@@ -113,7 +140,7 @@ export const logLinesParser = (lines = [], lineNumberStart, accumulator = []) =>
  * @returns Array
  *
  */
-export const findOffsetAndRemove = (newLog, oldParsed) => {
+export const findOffsetAndRemove = (newLog = [], oldParsed = []) => {
   const cloneOldLog = [...oldParsed];
   const lastIndex = cloneOldLog.length - 1;
   const last = cloneOldLog[lastIndex];
@@ -140,40 +167,13 @@ export const findOffsetAndRemove = (newLog, oldParsed) => {
  * 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;
+export const updateIncrementalTrace = (newLog, oldParsed = []) => {
+  const parsedLog = findOffsetAndRemove(newLog, oldParsed);
 
-  // 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));
+  return logLinesParser(newLog, parsedLog);
 };
 
 export const isNewJobLogActive = () => gon && gon.features && gon.features.jobLogJson;

spec/frontend/jobs/store/mutations_spec.js

@@ -73,6 +73,7 @@ describe('Jobs Store Mutations', () => {
         html,
         size: 511846,
         complete: true,
+        lines: [],
       });
 
       expect(stateCopy.trace).toEqual(html);

spec/frontend/jobs/store/utils_spec.js

@@ -6,6 +6,7 @@ import {
   addDurationToHeader,
   isCollapsibleSection,
   findOffsetAndRemove,
+  getIncrementalLineNumber,
 } from '~/jobs/store/utils';
 import {
   utilsMockData,
@@ -292,11 +293,91 @@ describe('Jobs Store Utils', () => {
     });
   });
 
+  describe('getIncrementalLineNumber', () => {
+    describe('when last line is 0', () => {
+      it('returns 1', () => {
+        const log = [
+          {
+            content: [],
+            lineNumber: 0,
+          },
+        ];
+
+        expect(getIncrementalLineNumber(log)).toEqual(1);
+      });
+    });
+
+    describe('with unnested line', () => {
+      it('returns the lineNumber of the last item in the array', () => {
+        const log = [
+          {
+            content: [],
+            lineNumber: 10,
+          },
+          {
+            content: [],
+            lineNumber: 101,
+          },
+        ];
+
+        expect(getIncrementalLineNumber(log)).toEqual(102);
+      });
+    });
+
+    describe('when last line is the header section', () => {
+      it('returns the lineNumber of the last item in the array', () => {
+        const log = [
+          {
+            content: [],
+            lineNumber: 10,
+          },
+          {
+            isHeader: true,
+            line: {
+              lineNumber: 101,
+              content: [],
+            },
+            lines: [],
+          },
+        ];
+
+        expect(getIncrementalLineNumber(log)).toEqual(102);
+      });
+    });
+
+    describe('when last line is a nested line', () => {
+      it('returns the lineNumber of the last item in the nested array', () => {
+        const log = [
+          {
+            content: [],
+            lineNumber: 10,
+          },
+          {
+            isHeader: true,
+            line: {
+              lineNumber: 101,
+              content: [],
+            },
+            lines: [
+              {
+                lineNumber: 102,
+                content: [],
+              },
+              { lineNumber: 103, content: [] },
+            ],
+          },
+        ];
+
+        expect(getIncrementalLineNumber(log)).toEqual(104);
+      });
+    });
+  });
+
   describe('updateIncrementalTrace', () => {
     describe('without repeated section', () => {
       it('concats and parses both arrays', () => {
         const oldLog = logLinesParser(originalTrace);
-        const result = updateIncrementalTrace(originalTrace, oldLog, regularIncremental);
+        const result = updateIncrementalTrace(regularIncremental, oldLog);
 
         expect(result).toEqual([
           {
@@ -324,7 +405,7 @@ describe('Jobs Store Utils', () => {
     describe('with regular line repeated offset', () => {
       it('updates the last line and formats with the incremental part', () => {
         const oldLog = logLinesParser(originalTrace);
-        const result = updateIncrementalTrace(originalTrace, oldLog, regularIncrementalRepeated);
+        const result = updateIncrementalTrace(regularIncrementalRepeated, oldLog);
 
         expect(result).toEqual([
           {
@@ -343,7 +424,7 @@ describe('Jobs Store Utils', () => {
     describe('with header line repeated', () => {
       it('updates the header line and formats with the incremental part', () => {
         const oldLog = logLinesParser(headerTrace);
-        const result = updateIncrementalTrace(headerTrace, oldLog, headerTraceIncremental);
+        const result = updateIncrementalTrace(headerTraceIncremental, oldLog);
 
         expect(result).toEqual([
           {
@@ -369,11 +450,7 @@ describe('Jobs Store Utils', () => {
     describe('with collapsible line repeated', () => {
       it('updates the collapsible line and formats with the incremental part', () => {
         const oldLog = logLinesParser(collapsibleTrace);
-        const result = updateIncrementalTrace(
-          collapsibleTrace,
-          oldLog,
-          collapsibleTraceIncremental,
-        );
+        const result = updateIncrementalTrace(collapsibleTraceIncremental, oldLog);
 
         expect(result).toEqual([
           {