refactor: Transform coverage data to improve internal manipulation

src/components/diffviewer.js

@@ -80,7 +80,8 @@ const DiffViewer = ({ appError, coverage, node, parsedDiff, summary }) => (
         <DiffFile
           key={diffBlock.from}
           diffBlock={diffBlock}
-          coverage={coverage}
+          fileCoverageDiffs={(coverage) ?
+            coverage.diffs[diffBlock.from] : undefined}
         />
       ))}
   </div>
@@ -111,16 +112,7 @@ const CoverageMeta = ({ ccovBackend, codecov, coverage, gh, hgRev, pushlog, summ
 );
 
 /* A DiffLine contains all diff changes for a specific file */
-const DiffFile = ({ coverage, diffBlock }) => {
-  // We try to see if the file modified shows up in the code
-  // coverage data we have for this diff
-  let coverageInfo;
-  if (coverage) {
-    coverageInfo = (coverage.diffs) ?
-      coverage.diffs.find(info => info.name === diffBlock.from) : undefined;
-  }
-
-  return (
+const DiffFile = ({ fileCoverageDiffs, diffBlock }) => (
   <div className="diff-file">
     <div className="file-summary">
       <div className="file-path">{diffBlock.from}</div>
@@ -130,12 +122,11 @@ const DiffFile = ({ coverage, diffBlock }) => {
         key={block.content}
         filePath={diffBlock.from}
         block={block}
-          coverageInfo={coverageInfo}
+        fileDiffs={fileCoverageDiffs}
       />
     ))}
   </div>
-  );
-};
+);
 
 const uniqueLineId = (filePath, change) => {
   let lineNumber;
@@ -150,7 +141,7 @@ const uniqueLineId = (filePath, change) => {
 };
 
 /* A DiffBlock is *one* of the blocks changed for a specific file */
-const DiffBlock = ({ filePath, block, coverageInfo }) => (
+const DiffBlock = ({ filePath, block, fileDiffs }) => (
   <div>
     <div className="diff-line-at">{block.content}</div>
     <div className="diff-block">
@@ -162,7 +153,7 @@ const DiffBlock = ({ filePath, block, coverageInfo }) => (
               key={uid}
               id={uid}
               change={change}
-              coverageInfo={coverageInfo}
+              fileDiffs={fileDiffs}
             />);
           })}
         </tbody>
@@ -172,7 +163,7 @@ const DiffBlock = ({ filePath, block, coverageInfo }) => (
 );
 
 /* A DiffLine contains metadata about a line in a DiffBlock */
-const DiffLine = ({ change, coverageInfo, id }) => {
+const DiffLine = ({ change, fileDiffs, id }) => {
   const c = change; // Information about the line itself
   const changeType = change.type; // Added, deleted or unchanged line
   let rowClass = 'nolinechange'; // CSS tr and td classes
@@ -181,17 +172,15 @@ const DiffLine = ({ change, coverageInfo, id }) => {
 
   if (changeType === 'add') {
     // Added line - <blank> | <new line number>
-    if (coverageInfo) {
+    if (fileDiffs) {
       try {
-        const { coverage } = coverageInfo.changes.find(lineCovInfo =>
-          (lineCovInfo.line === c.ln));
-
+        const coverage = fileDiffs[c.ln];
         if (coverage === 'Y') {
           rowClass = 'hit';
         } else if (coverage === '?') {
           rowClass = 'nolinechange';
         } else {
-          rowClass = 'miss'; // Let's start assuming a miss
+          rowClass = 'miss';
         }
       } catch (e) {
         console.log(e);

src/utils/data.js

@@ -18,21 +18,19 @@ const coverageSummary = (coverage) => {
     addedLines: 0,
     coveredLines: 0,
   };
-  if (coverage.diffs.length > 0) {
-    coverage.diffs.forEach((diff) => {
-      diff.changes.forEach((change) => {
-        if (change.coverage === 'Y') {
+  Object.keys(coverage.diffs).forEach((filePath) => {
+    Object.keys(coverage.diffs[filePath]).forEach((lineNumber) => {
+      const lineCoverage = coverage.diffs[filePath][lineNumber];
+      if (lineCoverage === 'Y') {
         s.coveredLines += 1;
       }
-        if (change.coverage !== '?') {
+      if (lineCoverage !== '?') {
         s.addedLines += 1;
       }
     });
-      s.percentage = (s.addedLines === 0) ?
-        undefined :
-        100 * (s.coveredLines / s.addedLines);
   });
-  }
+  s.percentage = (s.addedLines === 0) ?
+    undefined : 100 * (s.coveredLines / s.addedLines);
   return s;
 };
 
@@ -55,6 +53,31 @@ export const coverageSummaryText = (coverage) => {
   return result;
 };
 
+// We transform the data
+export const transformCoverageData = (cov) => {
+  /* We only want to transform the diffs entry in the data:
+    "diffs": [{
+        "changes": [{ "coverage": "?", "line": 413 }, ... ]
+        "name": "browser/extensions/formautofill/FormAutofillParent.jsm"
+      }]
+    to
+    "diffs": {
+      "browser/extensions/formautofill/FormAutofillParent.jsm": {
+        "413": "?",
+      }
+   */
+  const newCov = Object.assign({}, cov);
+  newCov.diffs = {};
+  cov.diffs.forEach(({ changes, name }) => {
+    const lines = {};
+    changes.forEach(({ coverage, line }) => {
+      lines[line] = coverage;
+    });
+    newCov.diffs[name] = lines;
+  });
+  return newCov;
+};
+
 export const csetWithCcovData = async (cset) => {
   if (!cset.node) {
     throw Error(`No node for cset: ${cset}`);
@@ -71,7 +94,7 @@ export const csetWithCcovData = async (cset) => {
       // This is the only case when we poll again
       newCset.summary = PENDING;
     } else if (res.status === 200) {
-      const coverageData = await res.json();
+      const coverageData = transformCoverageData(await res.json());
 
       // XXX: Document in which cases we would not have overall_cur
       if (coverageData.overall_cur) {