From 18a3b2913d61502ae0bf57cdddc02b63fca7d76f Mon Sep 17 00:00:00 2001 From: Chris Thoburn Date: Wed, 6 Nov 2019 08:55:07 -0800 Subject: [PATCH] [CHORE] Improve output and fail test for Asset size Check (#6676) * [CHORE] Improve output and fail test for Asset size Check * when using tee we need to ensure failures are propagated --- .github/workflows/asset-size-check.yml | 21 ++++++- bin/asset-size-tracking/generate-diff.js | 75 +++++++++++++++++------- bin/asset-size-tracking/src/library.js | 2 +- 3 files changed, 73 insertions(+), 25 deletions(-) diff --git a/.github/workflows/asset-size-check.yml b/.github/workflows/asset-size-check.yml index 4590c2d1536..a87395a8e7e 100644 --- a/.github/workflows/asset-size-check.yml +++ b/.github/workflows/asset-size-check.yml @@ -14,6 +14,16 @@ jobs: with: node-version: 12.x repo-token: ${{ secrets.GITHUB_TOKEN }} + - name: Check SHA + run: | + sha=$(git rev-parse --short=8 HEAD) + echo "HEAD sha=$sha" + echo "GITHUB_SHA sha=$GITHUB_SHA" + mkdir -p tmp + echo $sha > tmp/sha-for-check.txt + originSha=$(git rev-parse HEAD^2) + echo $originSha > tmp/sha-for-commit.txt + git show --format=short --no-patch $originSha - name: Checkout master run: git checkout master - name: Install dependencies for master @@ -25,7 +35,9 @@ jobs: - name: Checkout ${{github.ref}} # We checkout the PR Branch before parsing the master vendor file # So that we are always using the current analysis tooling - run: git checkout --progress --force $GITHUB_SHA + run: | + sha=$(cat tmp/sha-for-check.txt) + git checkout --progress --force $sha - name: Install dependencies for ${{github.ref}} run: yarn install - name: Parse Master Assets @@ -45,6 +57,7 @@ jobs: run: | mkdir -p tmp mkdir -p tmp/asset-sizes + set -o pipefail node ./bin/asset-size-tracking/generate-diff.js | tee tmp/asset-sizes/diff.txt - name: Prepare Data For Report if: failure() || success() @@ -53,6 +66,7 @@ jobs: - name: Print Asset Size Report if: failure() || success() run: | + set -o pipefail node ./bin/asset-size-tracking/print-analysis.js | tee tmp/asset-sizes/commit-analysis.txt - name: Upload ${{github.ref}} Dist Artifacts uses: actions/upload-artifact@v1 @@ -69,7 +83,8 @@ jobs: env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | - COMMENT_MARKER="Asset Size Report for " - node ./bin/asset-size-tracking/src/create-comment-text.js $GITHUB_SHA > tmp/asset-sizes/comment.txt + COMMENT_MARKER="Asset Size Report for "= + sha=$(cat tmp/sha-for-commit.txt) + node ./bin/asset-size-tracking/src/create-comment-text.js $sha > tmp/asset-sizes/comment.txt COMMENT_TEXT="@./tmp/asset-sizes/comment.txt" source bin/asset-size-tracking/src/post-comment.sh diff --git a/bin/asset-size-tracking/generate-diff.js b/bin/asset-size-tracking/generate-diff.js index 215e7f87064..c3d5a9adeec 100644 --- a/bin/asset-size-tracking/generate-diff.js +++ b/bin/asset-size-tracking/generate-diff.js @@ -92,7 +92,7 @@ function analyzeDiff(diff) { if (diff.currentSize < diff.newSize) { let delta = diff.newSize - diff.currentSize; let compressedDelta = diff.newSizeCompressed - diff.currentSizeCompressed; - if (delta > library_failure_threshold) { + if (delta > library_failure_threshold && compressedDelta > 0) { failures.push( `The size of the library ${diff.name} has increased by ${formatBytes(delta)} (${formatBytes( compressedDelta @@ -114,6 +114,7 @@ function analyzeDiff(diff) { } function printDiff(diff) { + console.log('\n```\n'); printItem(diff); diff.packages.forEach(pkg => { printItem(pkg, 2); @@ -121,37 +122,57 @@ function printDiff(diff) { printItem(m, 4); }); }); + console.log('\n```\n'); } function printItem(item, indent = 0) { if (item.currentSize !== item.newSize) { const indentColor = indent >= 4 ? 'grey' : indent >= 2 ? 'yellow' : indent >= 0 ? 'magenta' : 'green'; console.log( - leftPad( - chalk[indentColor](item.name) + ' ' + chalk.white(formatBytes(item.newSizeCompressed)) + formatDelta(item), - indent * 2 - ) + leftPad(chalk[indentColor](item.name) + ' ' + formatSize(item, false) + ' ' + formatSize(item, true), indent * 2) ); } } -function formatDelta(item) { - if (item.currentSize === item.newSize) { - return ''; +function formatSize(item, isCompressed = false) { + let size = formatBytes(isCompressed ? item.newSizeCompressed : item.newSize); + let delta = formatDelta(item, isCompressed); + + return isCompressed ? chalk.grey(`(${chalk.white(size)} ${delta} compressed)`) : `${chalk.white(size)} ${delta}`; +} + +function formatDelta(item, isCompressed = false) { + let delta = isCompressed ? item.newSizeCompressed - item.currentSizeCompressed : item.newSize - item.currentSize; + + if (delta === 0) { + return chalk.black('±0 B'); } - if (item.currentSize > item.newSize) { - return chalk.green(` (- ${formatBytes(item.currentSizeCompressed - item.newSizeCompressed)})`); + + if (delta < 0) { + return chalk.green(`${formatBytes(delta)}`); } else { - return chalk.red(` (+ ${formatBytes(item.newSizeCompressed - item.currentSizeCompressed)})`); + return chalk.red(`+${formatBytes(delta)}`); + } +} + +function humanizeNumber(n) { + let s = n.toFixed(2); + if (s.charAt(s.length - 1) === '0') { + s = n.toFixed(1); + + if (s.charAt(s.length - 2) === '0') { + s = n.toFixed(0); + } } + return s; } function formatBytes(b) { let str; - if (b > 1024) { - str = (b / 1024).toFixed(2) + ' KB'; + if (b > 1024 || b < -1024) { + str = humanizeNumber(b / 1024) + ' KB'; } else { - str = b + ' B'; + str = humanizeNumber(b) + ' B'; } return str; @@ -164,7 +185,6 @@ function leftPad(str, len, char = ' ') { return str; } -printDiff(diff); const { failures, warnings } = analyzeDiff(diff); if (failures.length) { @@ -181,6 +201,8 @@ if (failures.length) { console.log(w); }); } + console.log('\n**Changeset**\n'); + printDiff(diff); console.log('\n'); process.exit(1); } else { @@ -189,16 +211,25 @@ if (failures.length) { console.log(`\n
\n ${diff.name} has not changed in size`); } else if (delta > 0) { console.log( - `\n\n ${diff.name} shrank by ${formatBytes(delta)} (${formatBytes( + `\n
\n ${diff.name} shrank by ${formatBytes(delta)} (${formatBytes( diff.currentSizeCompressed - diff.newSizeCompressed )} compressed)` ); } else { - console.log( - `\n${diff.name} increased by ${formatBytes(-1 * delta)} (${formatBytes( - diff.newSizeCompressed - diff.currentSizeCompressed - )} compressed) which is within the allowed tolerance of ${library_failure_threshold} bytes uncompressed` - ); + let compressedDelta = diff.newSizeCompressed - diff.currentSizeCompressed; + if (-1 * delta < library_failure_threshold) { + console.log( + `\n
\n ${diff.name} increased by ${formatBytes(-1 * delta)} (${formatBytes( + compressedDelta + )} compressed) which is within the allowed tolerance of ${library_failure_threshold} bytes uncompressed` + ); + } else { + console.log( + `\n
\n ${diff.name} increased by ${formatBytes( + -1 * delta + )} uncompressed but decreased by ${formatBytes(-1 * compressedDelta)} compressed` + ); + } } if (warnings.length) { console.log('\nWarnings\n-----------------------'); @@ -208,6 +239,8 @@ if (failures.length) { } else { console.log('\nIf any packages had changed sizes they would be listed here.'); } + console.log('\n**Changeset**\n'); + printDiff(diff); console.log('\n
'); process.exit(0); } diff --git a/bin/asset-size-tracking/src/library.js b/bin/asset-size-tracking/src/library.js index 8295e210c41..83b76e443d3 100644 --- a/bin/asset-size-tracking/src/library.js +++ b/bin/asset-size-tracking/src/library.js @@ -224,7 +224,7 @@ function formatBytes(b) { if (b > 1024) { str = (b / 1024).toFixed(2) + ' KB'; } else { - str = b + ' B'; + str = b.toFixed(2) + ' B'; } return str;