feat: use markers config to decide if end at lcp

TracerBench · Aug 2, 2022 · 3602a59 · 3602a59
1 parent 7f645b7
commit 3602a59
Show file tree

Hide file tree

Showing 19 changed files with 275 additions and 208 deletions.
diff --git a/packages/cli/markdown/compare.md b/packages/cli/markdown/compare.md
@@ -60,9 +60,6 @@ OPTIONS
   --isCIEnv=isCIEnv
       Provides a drastically slimmed down stdout report for CI workflows. However does NOT hide analysis.
 
-  --lcpRegex=lcpRegex
-      The regex pattern to identify the LCP candidate, if undefined, we use the first LCP candidate
-
   --markers=markers
       (required) [default: domComplete] User Timing Markers
 
@@ -89,12 +86,9 @@ OPTIONS
 
   --tbResultsFolder=tbResultsFolder
       (required) [default: ./tracerbench-results] The output folder path for all tracerbench results
-
-  --traceEndAtLcp
-      Overrides the default loadEventEnd as trace end or the last marker in markers array as tracerbench
 ```
 
-_See code: [dist/src/commands/compare/index.ts](https://github.com/TracerBench/tracerbench/blob/v6.1.2/dist/src/commands/compare/index.ts)_
+_See code: [dist/src/commands/compare/index.ts](https://github.com/TracerBench/tracerbench/blob/v7.0.0/dist/src/commands/compare/index.ts)_
 
 ## `tracerbench compare:analyze RESULTSFILE`
 
@@ -124,7 +118,7 @@ OPTIONS
                                                          threshold runs against.
 ```
 
-_See code: [dist/src/commands/compare/analyze.ts](https://github.com/TracerBench/tracerbench/blob/v6.1.2/dist/src/commands/compare/analyze.ts)_
+_See code: [dist/src/commands/compare/analyze.ts](https://github.com/TracerBench/tracerbench/blob/v7.0.0/dist/src/commands/compare/analyze.ts)_
 
 ## `tracerbench compare:report`
 
@@ -150,4 +144,4 @@ ALIASES
   $ tracerbench report
 ```
 
-_See code: [dist/src/commands/compare/report.ts](https://github.com/TracerBench/tracerbench/blob/v6.1.2/dist/src/commands/compare/report.ts)_
+_See code: [dist/src/commands/compare/report.ts](https://github.com/TracerBench/tracerbench/blob/v7.0.0/dist/src/commands/compare/report.ts)_
diff --git a/packages/cli/markdown/record-har.md b/packages/cli/markdown/record-har.md
@@ -39,7 +39,7 @@ OPTIONS
   --url=url                          (required) URL to visit for record-har, auth, timings & trace commands
 ```
 
-_See code: [dist/src/commands/record-har/index.ts](https://github.com/TracerBench/tracerbench/blob/v6.1.2/dist/src/commands/record-har/index.ts)_
+_See code: [dist/src/commands/record-har/index.ts](https://github.com/TracerBench/tracerbench/blob/v7.0.0/dist/src/commands/record-har/index.ts)_
 
 ## `tracerbench record-har:auth`
 
@@ -73,4 +73,4 @@ OPTIONS
   --username=username                (required) The username to login to the form
 ```
 
-_See code: [dist/src/commands/record-har/auth.ts](https://github.com/TracerBench/tracerbench/blob/v6.1.2/dist/src/commands/record-har/auth.ts)_
+_See code: [dist/src/commands/record-har/auth.ts](https://github.com/TracerBench/tracerbench/blob/v7.0.0/dist/src/commands/record-har/auth.ts)_
diff --git a/packages/cli/oclif.manifest.json b/packages/cli/oclif.manifest.json
diff --git a/packages/cli/src/command-config/tb-config.ts b/packages/cli/src/command-config/tb-config.ts
@@ -45,11 +45,6 @@ export interface ITBConfig {
   isCIEnv?: boolean | string;
   marker?: string;
   regressionThresholdStat?: RegressionThresholdStat;
-  //default to false, if it's true
-  //it overrides the default loadEventEnd or the last marker in markers array as trace end
-  traceEndAtLcp?: boolean;
-  //the regex pattern to the LCP candidate element user want to measure, if undefined, we use the first LCP candidate
-  lcpRegex?: string;
 }
 
 export interface IHARServer {

diff --git a/packages/cli/src/commands/compare/index.ts b/packages/cli/src/commands/compare/index.ts
@@ -54,8 +54,6 @@ import {
   sampleTimeout,
   socksPorts,
   tbResultsFolder,
-  traceEndAtLcp,
-  lcpRegex,
 } from "../../helpers/flags";
 import {
   chalkScheme,
@@ -92,8 +90,6 @@ export interface ICompareFlags {
   report?: boolean;
   isCIEnv?: boolean;
   regressionThresholdStat: RegressionThresholdStat;
-  traceEndAtLcp?: boolean;
-  lcpRegex?: string;
 }
 
 export default class Compare extends TBBaseCommand {
@@ -124,8 +120,6 @@ export default class Compare extends TBBaseCommand {
     headless,
     isCIEnv: isCIEnv(),
     regressionThresholdStat,
-    traceEndAtLcp,
-    lcpRegex: lcpRegex(),
   };
   public compareFlags: ICompareFlags;
   public parsedConfig: ITBConfig = defaultFlagArgs;
@@ -446,8 +440,6 @@ export default class Compare extends TBBaseCommand {
           captureV8RuntimeStats: this.compareFlags.runtimeStats,
           saveTraceAs: (group, i) =>
             `${this.compareFlags.tbResultsFolder}/traces/${group}${i}.json`,
-          traceEndAtLcp: this.compareFlags.traceEndAtLcp,
-          lcpRegex: this.compareFlags.lcpRegex,
         },
       },
     ];
@@ -488,8 +480,6 @@ export default class Compare extends TBBaseCommand {
           captureV8RuntimeStats: this.compareFlags.runtimeStats,
           saveTraceAs: (group, i) =>
             `${this.compareFlags.tbResultsFolder}/traces/${group}${i}.json`,
-          traceEndAtLcp: this.compareFlags.traceEndAtLcp,
-          lcpRegex: this.compareFlags.lcpRegex,
         },
       },
     ];

diff --git a/packages/cli/src/helpers/flags.ts b/packages/cli/src/helpers/flags.ts
@@ -277,13 +277,3 @@ export const jsonReport = oclifFlags.boolean({
   description: `Include a JSON file from the stdout report`,
   default: false,
 });
-
-export const traceEndAtLcp = oclifFlags.boolean({
-  description: `Overrides the default loadEventEnd as trace end or the last marker in markers array as tracerbench`,
-  default: false,
-});
-
-export const lcpRegex = oclifFlags.build({
-  description: `The regex pattern to identify the LCP candidate, if undefined, we use the first LCP candidate`,
-  required: false,
-});
diff --git a/packages/cli/src/helpers/utils.ts b/packages/cli/src/helpers/utils.ts
@@ -88,7 +88,7 @@ export function parseMarkers(m: string | string[]): Marker[] {
     m = m.split(",");
   }
 
-  for (let i = 1; i < m.length; i++) {
+  for (let i = 1; i < m.length; i += 2) {
     a.push({
       start: m[i - 1],
       label: m[i],

diff --git a/packages/cli/tb-schema.json b/packages/cli/tb-schema.json
@@ -1,11 +1,9 @@
 {
     "$schema": "http://json-schema.org/draft-07/schema#",
-    "additionalProperties": {
-    },
+    "additionalProperties": {},
     "definitions": {
         "IBenchmarkEnvironmentOverride": {
-            "additionalProperties": {
-            },
+            "additionalProperties": {},
             "properties": {
                 "cpuThrottle": {
                     "type": "number"
@@ -142,9 +140,6 @@
                 "boolean"
             ]
         },
-        "lcpRegex": {
-            "type": "string"
-        },
         "locations": {
             "type": "string"
         },
@@ -193,6 +188,7 @@
                             "responseEnd",
                             "responseStart",
                             "secureConnectionStart",
+                            "serverTiming",
                             "startTime",
                             "toJSON",
                             "transferSize",
@@ -269,9 +265,6 @@
         "tbResultsFolder": {
             "type": "string"
         },
-        "traceEndAtLcp": {
-            "type": "boolean"
-        },
         "traceFrame": {
             "type": "string"
         },

diff --git a/packages/cli/test/commands/compare.test.ts b/packages/cli/test/commands/compare.test.ts
@@ -11,7 +11,8 @@ const fidelityLow = "10";
 const emulateDevice = "iphone-4";
 const regressionThreshold = "50";
 const network = "FIOS";
-const lcpRegexPattern = "h1";
+const markers = 'navigationStart,load,jqueryLoaded,jquery,emberLoaded,application,startRouting,routing,willTransition,transition,largestContentfulPaint';
+
 describe("compare fixture: A/A", () => {
   test
     .stdout()
@@ -82,65 +83,11 @@ describe("compare fixture: A/A CI", () => {
     );
 });
 
-describe("compare regression: fixture: A/B trace end at specific LCP candidate", () => {
-  test
-    .stdout()
-    .it(
-      `runs compare --controlURL ${FIXTURE_APP.control} --experimentURL ${FIXTURE_APP.regression} --fidelity ${fidelityLow} --tbResultsFolder ${TB_RESULTS_FOLDER} --config ${FIXTURE_APP.regressionConfig} --regressionThreshold ${regressionThreshold} --headless --traceEndAtLcp --lcpRegex ${lcpRegexPattern}`,
-      async (ctx) => {
-        const results = await Compare.run([
-          "--controlURL",
-          FIXTURE_APP.control,
-          "--experimentURL",
-          FIXTURE_APP.regression,
-          "--fidelity",
-          fidelityLow,
-          "--tbResultsFolder",
-          TB_RESULTS_FOLDER,
-          "--config",
-          FIXTURE_APP.regressionConfig,
-          "--regressionThreshold",
-          regressionThreshold,
-          "--lcpRegex",
-          lcpRegexPattern,
-          "--headless",
-          "--traceEndAtLcp",
-        ]);
-
-        const resultsJSON: ICompareJSONResults = await JSON.parse(results);
-        expect(ctx.stdout).to.contain(
-          `    SUCCESS     ${fidelityLow} test samples took`
-        );
-        // confirm with headless flag is logging the trace stream
-        expect(ctx.stdout).to.contain(`duration phase estimated regression +`);
-        expect(ctx.stdout).to.contain(
-          `    ! ALERT     Regression found exceeding the set regression threshold of ${regressionThreshold} ms`
-        );
-        assert.isAbove(
-          parseInt(resultsJSON.benchmarkTableData[0].estimatorDelta, 10),
-          500
-        );
-        assert.isAbove(
-          parseInt(resultsJSON.benchmarkTableData[0].confidenceInterval[0], 10),
-          500
-        );
-        assert.isAbove(
-          parseInt(resultsJSON.benchmarkTableData[0].confidenceInterval[1], 10),
-          500
-        );
-        // results are json and are significant
-        assert.isTrue(resultsJSON.areResultsSignificant);
-        // regression is over the threshold
-        assert.isFalse(resultsJSON.isBelowRegressionThreshold);
-      }
-    );
-});
-
-describe("compare regression: fixture: A/B trace end at first LCP candidate", () => {
+describe("compare regression: fixture: A/B trace end at LCP candidate after a marker", () => {
   test
     .stdout()
     .it(
-      `runs compare --controlURL ${FIXTURE_APP.control} --experimentURL ${FIXTURE_APP.regression} --fidelity ${fidelityLow} --tbResultsFolder ${TB_RESULTS_FOLDER} --config ${FIXTURE_APP.regressionConfig} --regressionThreshold ${regressionThreshold} --headless --traceEndAtLcp`,
+      `runs compare --controlURL ${FIXTURE_APP.control} --experimentURL ${FIXTURE_APP.regression} --fidelity ${fidelityLow} --tbResultsFolder ${TB_RESULTS_FOLDER} --config ${FIXTURE_APP.regressionConfig} --regressionThreshold ${regressionThreshold} --markers ${markers} --headless`,
       async (ctx) => {
         const results = await Compare.run([
           "--controlURL",
@@ -155,8 +102,9 @@ describe("compare regression: fixture: A/B trace end at first LCP candidate", ()
           FIXTURE_APP.regressionConfig,
           "--regressionThreshold",
           regressionThreshold,
-          "--headless",
-          "--traceEndAtLcp",
+          "--markers",
+          markers,
+          "--headless"
         ]);
 
         const resultsJSON: ICompareJSONResults = await JSON.parse(results);

diff --git a/packages/core/package.json b/packages/core/package.json
@@ -32,7 +32,7 @@
     "lint": "eslint -c .eslintrc.js --ext .ts .",
     "prepare": "yarn build",
     "report": "Rscript bin/report.R test/results/results.json",
-    "test": "yarn lint && yarn mocha",
+    "test": "yarn lint && yarn mocha \"test/test.ts\" \"test/trace/*.ts\"",
     "watch": "tsc -b -w"
   },
   "dependencies": {

diff --git a/packages/core/src/create-trace-benchmark.ts b/packages/core/src/create-trace-benchmark.ts
@@ -67,8 +67,6 @@ export interface TraceOptions {
   additionalCategories: string[];
   additionalTrialCategories: string[];
   saveTraceAs: SaveTraceAsFn;
-  traceEndAtLcp?: boolean;
-  lcpRegex?: string;
 }
 
 export type TraceFn = (

diff --git a/packages/core/src/create-trace-navigation-benchmark.ts b/packages/core/src/create-trace-navigation-benchmark.ts
@@ -6,13 +6,18 @@ import extractNavigationSample, {
   NavigationSample
 } from './metrics/extract-navigation-sample';
 import type { Benchmark } from './run';
-import type { WaitForLCP, WaitForMark } from './util/inject-mark-observer';
+import type { WaitForMarkOrLCP } from './util/inject-mark-observer';
 import injectMarkObserver, {
   injectLCPObserver
 } from './util/inject-mark-observer';
 import navigate from './util/navigate';
 import type { PageSetupOptions } from './util/setup-page';
 import setupPage from './util/setup-page';
+import {
+  LCP_EVENT_NAME,
+  uniformLCPEventName,
+  isTraceEndAtLCP
+} from './trace/utils';
 
 export interface Marker {
   start: string;
@@ -36,7 +41,7 @@ export default function createTraceNavigationBenchmark(
         label: 'load'
       },
       {
-        start: 'loadEventEnd',
+        start: LCP_EVENT_NAME,
         label: 'paint'
       }
     );
@@ -46,34 +51,32 @@ export default function createTraceNavigationBenchmark(
     async (page, _i, _isTrial, raceCancel, trace) => {
       setupPage(page, raceCancel, options.pageSetupOptions);
 
-      let traceEndAtLcp = false;
-      let lcpRegex: string | undefined;
-      if (options.traceOptions) {
-        traceEndAtLcp = options.traceOptions.traceEndAtLcp ?? false;
-        lcpRegex = options.traceOptions.lcpRegex;
+      const markerList = uniformLCPEventName(markers);
+      const mLength = markerList.length;
+
+      const { start: lastMarker } = markerList[mLength - 1];
+      const traceEndAtLcp = isTraceEndAtLCP(markerList);
+      let priorMarker = 'navigationStart';
+      if (traceEndAtLcp && mLength >= 2) {
+        priorMarker = markerList[mLength - 2].start;
       }
 
-      let waitForLcp: WaitForLCP;
-      let waitForMark: WaitForMark;
-      const { start: mark } = markers[markers.length - 1];
+      let waitTraceEnd: WaitForMarkOrLCP;
+
       if (traceEndAtLcp) {
-        waitForLcp = await injectLCPObserver(page, lcpRegex);
+        waitTraceEnd = await injectLCPObserver(page, priorMarker);
       } else {
-        waitForMark = await injectMarkObserver(page, mark);
+        waitTraceEnd = await injectMarkObserver(page, lastMarker);
       }
       return extractNavigationSample(
         buildModel(
           await trace(async (raceCancel) => {
             // do navigation
             await navigate(page, url, raceCancel);
-            if (traceEndAtLcp) {
-              await waitForLcp(raceCancel);
-            } else {
-              await waitForMark(raceCancel);
-            }
+            await waitTraceEnd(raceCancel);
           })
         ),
-        markers
+        markerList
       );
     },
     options

diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts
@@ -45,5 +45,10 @@ export {
   authClient,
   getNewTab,
   createBrowser,
-  getTab
+  getTab,
+  LCP_EVENT_NAME,
+  LCP_EVENT_NAME_ALIAS,
+  isLCPEvent,
+  isTraceEndAtLCP,
+  uniformLCPEventName
 } from './trace';