cli(asset-saver): end devtoolsLog with a newline (#13566)

lighthouse-core/lib/asset-saver.js

@@ -19,6 +19,8 @@ const LHError = require('../lib/lh-error.js');
 // TODO(esmodules): Rollup does not support `promisfy` or `stream.pipeline`. Bundled files
 // don't need anything in this file except for `stringifyReplacer`, so a check for
 // truthiness before using is enough.
+// TODO: Can remove promisify(pipeline) in Node 15.
+// https://nodejs.org/api/stream.html#streams-promises-api
 const pipeline = promisify && promisify(stream.pipeline);
 
 const artifactsFilename = 'artifacts.json';
@@ -238,8 +240,6 @@ async function saveTrace(traceData, traceFilename) {
   const traceIter = traceJsonGenerator(traceData);
   const writeStream = fs.createWriteStream(traceFilename);
 
-  // TODO: Can remove promisify(pipeline) in Node 15.
-  // https://nodejs.org/api/stream.html#stream_stream_pipeline_streams_callback
   return pipeline(traceIter, writeStream);
 }
 
@@ -250,10 +250,12 @@ async function saveTrace(traceData, traceFilename) {
  * @return {Promise<void>}
  */
 function saveDevtoolsLog(devtoolsLog, devtoolLogFilename) {
-  const logIter = arrayOfObjectsJsonGenerator(devtoolsLog);
   const writeStream = fs.createWriteStream(devtoolLogFilename);
 
-  return pipeline(logIter, writeStream);
+  return pipeline(function* () {
+    yield* arrayOfObjectsJsonGenerator(devtoolsLog);
+    yield '\n';
+  }, writeStream);
 }
 
 /**

lighthouse-core/scripts/update-report-fixtures.js

@@ -5,22 +5,19 @@
  */
 'use strict';
 
-import fs from 'fs';
-
 import * as cli from '../../lighthouse-cli/run.js';
 import * as cliFlags from '../../lighthouse-cli/cli-flags.js';
 import assetSaver from '../lib/asset-saver.js';
 import {server} from '../../lighthouse-cli/test/fixtures/static-server.js';
 import budgetedConfig from '../test/results/sample-config.js';
-import {LH_ROOT} from '../../root.js';
 
 const artifactPath = 'lighthouse-core/test/results/artifacts';
 // All artifacts must have resources from a consistent port, to ensure reproducibility.
 // https://github.com/GoogleChrome/lighthouse/issues/11776
 const MAGIC_SERVER_PORT = 10200;
 
 /**
- * Update the report artifacts. If artifactName is set only that artifact will be updated.
+ * Update the report artifacts. If artifactName is set, only that artifact will be updated.
  * @param {keyof LH.Artifacts=} artifactName
  */
 async function update(artifactName) {
@@ -37,32 +34,30 @@ async function update(artifactName) {
   await cli.runLighthouse(url, flags, budgetedConfig);
   await server.close();
 
+  let newArtifacts = assetSaver.loadArtifacts(artifactPath);
+
+  // Normalize some data so it doesn't change on every update.
+  for (const timing of newArtifacts.Timing) {
+    // @ts-expect-error: Value actually is writeable at this point.
+    timing.startTime = 0;
+    // @ts-expect-error: Value actually is writeable at this point.
+    timing.duration = 1;
+  }
+
   if (artifactName) {
     // Revert everything except the one artifact
-    const newArtifacts = assetSaver.loadArtifacts(artifactPath);
     if (!(artifactName in newArtifacts) && !(artifactName in oldArtifacts)) {
       throw Error('Unknown artifact name: ' + artifactName);
     }
-    const finalArtifacts = oldArtifacts;
-    const newArtifact = newArtifacts[artifactName];
-    // @ts-expect-error tsc can't yet express that artifactName is only a single type in each iteration, not a union of types.
-    finalArtifacts[artifactName] = newArtifact;
-    await assetSaver.saveArtifacts(finalArtifacts, artifactPath);
-  }
 
-  // Normalize some data.
-  const artifactsFile = `${LH_ROOT}/${artifactPath}/artifacts.json`;
-  /** @type {LH.Artifacts} */
-  const artifacts = JSON.parse(fs.readFileSync(artifactsFile, 'utf-8'));
+    const newArtifactToKeep = newArtifacts[artifactName];
 
-  for (const timing of artifacts.Timing) {
-    // @ts-expect-error: Value actually is writeable at this point.
-    timing.startTime = 0;
-    // @ts-expect-error: Value actually is writeable at this point.
-    timing.duration = 1;
+    newArtifacts = oldArtifacts;
+    // @ts-expect-error tsc can't yet express that artifactName is only a single type in each iteration, not a union of types.
+    newArtifacts[artifactName] = newArtifactToKeep;
   }
 
-  fs.writeFileSync(artifactsFile, JSON.stringify(artifacts, null, 2));
+  await assetSaver.saveArtifacts(newArtifacts, artifactPath);
 }
 
 update(/** @type {keyof LH.Artifacts | undefined} */ (process.argv[2]));

lighthouse-core/test/lib/asset-saver-test.js

@@ -96,6 +96,7 @@ describe('asset-saver helper', () => {
           {args: {IsMainFrame: true}, cat: 'v8', pid: 1, ts: 5},
           {args: {data: {encodedDataLength: 20, requestId: '1.22'}}, pid: 1, ts: 6},
         ],
+        metadata: {'cpu-model': 9001, 'network-type': 'Unknown'},
       };
       await assetSaver.saveTrace(trace, traceFilename);
 
@@ -107,7 +108,11 @@ describe('asset-saver helper', () => {
   {"args":{},"cat":"v8","pid":1,"ts":3},
   {"args":{"IsMainFrame":true},"cat":"v8","pid":1,"ts":5},
   {"args":{"data":{"encodedDataLength":20,"requestId":"1.22"}},"pid":1,"ts":6}
-]}
+],
+"metadata": {
+  "cpu-model": 9001,
+  "network-type": "Unknown"
+}}
 `);
     });
 
@@ -202,7 +207,8 @@ describe('asset-saver helper', () => {
 `[
   {"method":"Network.requestServedFromCache","params":{"requestId":"1.22"}},
   {"method":"Network.responseReceived","params":{"status":301,"headers":{":method":"POST"}}}
-]`);
+]
+`);
     });
   });
 
@@ -298,6 +304,27 @@ describe('asset-saver helper', () => {
       expect(roundTripArtifacts.ScriptElements.friendlyMessage)
         .toBeDisplayString(/\(Method: Page\.getFastness\)/);
     });
+
+    it('saves artifacts in files concluding with a newline', async () => {
+      const artifacts = {
+        devtoolsLogs: {
+          [Audit.DEFAULT_PASS]: [{method: 'first'}, {method: 'second'}],
+        },
+        traces: {
+          [Audit.DEFAULT_PASS]: {traceEvents: traceEvents.slice(0, 100)},
+        },
+        RobotsTxt: {status: 404, content: null},
+      };
+      await assetSaver.saveArtifacts(artifacts, outputPath);
+
+      const artifactFilenames = fs.readdirSync(outputPath);
+      expect(artifactFilenames.length).toBeGreaterThanOrEqual(3);
+      for (const artifactFilename of artifactFilenames) {
+        expect(artifactFilename).toMatch(/\.json$/);
+        const contents = fs.readFileSync(`${outputPath}/${artifactFilename}`, 'utf8');
+        expect(contents).toMatch(/\n$/);
+      }
+    });
   });
 
   describe('saveLanternNetworkData', () => {

lighthouse-core/test/results/artifacts/artifacts.json

@@ -13061,4 +13061,4 @@
     "versions": [],
     "registrations": []
   }
-}
+}

lighthouse-core/test/results/artifacts/defaultPass.devtoolslog.json

@@ -222,4 +222,4 @@
   {"method":"Network.dataReceived","params":{"requestId":"31161.49","timestamp":8710.380969,"dataLength":0,"encodedDataLength":45}},
   {"method":"Network.loadingFinished","params":{"requestId":"31161.49","timestamp":8710.377607,"encodedDataLength":255,"shouldReportCorbBlocking":false}},
   {"method":"Page.lifecycleEvent","params":{"frameId":"537BC44B4EE044D67F9FFE7A76173AB1","loaderId":"6913DB840A4B952A23AAEB21C42F130A","name":"networkIdle","timestamp":8710.380991}}
-]
+]

lighthouse-core/test/results/artifacts/offlinePass.devtoolslog.json

@@ -199,4 +199,4 @@
   {"method":"Page.loadEventFired","params":{"timestamp":8731.287917}},
   {"method":"Page.lifecycleEvent","params":{"frameId":"537BC44B4EE044D67F9FFE7A76173AB1","loaderId":"46B5FB75F319C4D59DA48DEC4DBF052A","name":"load","timestamp":8731.287917}},
   {"method":"Page.frameStoppedLoading","params":{"frameId":"537BC44B4EE044D67F9FFE7A76173AB1"}}
-]
+]

lighthouse-core/test/results/artifacts/redirectPass.devtoolslog.json

@@ -107,4 +107,4 @@
   {"method":"Page.frameStoppedLoading","params":{"frameId":"537BC44B4EE044D67F9FFE7A76173AB1"}},
   {"method":"Network.requestWillBeSent","params":{"requestId":"31161.165","loaderId":"AD6C6061F72746B87EC3A2FC441DC450","documentURL":"http://localhost:10200/dobetterweb/dbw_tester.html","request":{"url":"filesystem:http://localhost:10200/temporary/empty-0.2581056797220984.png","method":"GET","headers":{"Referer":"http://localhost:10200/dobetterweb/dbw_tester.html"},"mixedContentType":"none","initialPriority":"Low","referrerPolicy":"strict-origin-when-cross-origin","isSameSite":true},"timestamp":8736.680819,"wallTime":1631045513.006942,"initiator":{"type":"script","stack":{"callFrames":[],"parent":{"description":"Image","callFrames":[{"functionName":"","scriptId":"146","url":"http://localhost:10200/dobetterweb/dbw_tester.html","lineNumber":473,"columnNumber":14}]}}},"type":"Image","frameId":"537BC44B4EE044D67F9FFE7A76173AB1","hasUserGesture":false}},
   {"method":"Network.loadingFailed","params":{"requestId":"31161.165","timestamp":8736.68087,"type":"Image","errorText":"","canceled":false,"blockedReason":"inspector"}}
-]
+]