From 1697bb30cd2109e9690292d1adfaa1769664e279 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Tue, 28 Apr 2020 09:50:21 -0300 Subject: [PATCH] Fix page benchmark result display (#8438) The page tested by the benchmark was changed from `notification` to `home` in #8358, but the announce script was still expecting the `notification` page to be in the results. It does collect results for all pages, but the `notification` page was hard-coded to be used for the benchmark summary. The announce script now correctly looks for the `home` page results for the benchmark summary. Variable names have been updated to make it more clear what's going on here as well. --- development/metamaskbot-build-announce.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/development/metamaskbot-build-announce.js b/development/metamaskbot-build-announce.js index dc0dab936..788929fb3 100755 --- a/development/metamaskbot-build-announce.js +++ b/development/metamaskbot-build-announce.js @@ -78,15 +78,17 @@ async function start () { } } + const summaryPlatform = 'chrome' + const summaryPage = 'home' let commentBody - if (!benchmarkResults.chrome) { - console.log(`No results for Chrome found; skipping benchmark`) + if (!benchmarkResults[summaryPlatform]) { + console.log(`No results for ${summaryPlatform} found; skipping benchmark`) commentBody = artifactsBody } else { try { - const chromePageLoad = Math.round(parseFloat(benchmarkResults.chrome.notification.average.load)) - const chromePageLoadMarginOfError = Math.round(parseFloat(benchmarkResults.chrome.notification.marginOfError.load)) - const benchmarkSummary = `Page Load Metrics (${chromePageLoad} ± ${chromePageLoadMarginOfError} ms)` + const summaryPageLoad = Math.round(parseFloat(benchmarkResults[summaryPlatform][summaryPage].average.load)) + const summaryPageLoadMarginOfError = Math.round(parseFloat(benchmarkResults[summaryPlatform][summaryPage].marginOfError.load)) + const benchmarkSummary = `Page Load Metrics (${summaryPageLoad} ± ${summaryPageLoadMarginOfError} ms)` const allPlatforms = new Set() const allPages = new Set()