Skip to content

Commit

Permalink
[ci] Use dangerJS to report bundle size changes (mui#14587)
Browse files Browse the repository at this point in the history
* [core] Track size via build artifacts

* [core] Fix size snapshot failing on ci

tmp/ was not created on ci

* [ci] Fix size snapshot not being saved

* [ci] Decouple size snapshot from test pipeline

Use circle ci workspaces to persist build output. attach
that workspace in a size_snapshot job. dangerJS will later run in
that job and report size changes. An additional persist_size_snapshot
will later upload the size-snapshot permanently. CircleCI
artifacts are not supposed to be used permanently.
Only "around build time".

* [ci] Persist snapshot in s3 bucket

* [ci] Add dangerfile

* [ci] Fix pull requests not being recognized

* [ci] Proper dangerJS report

* [ci] Silence non existing snapshots

* [ci] Print commit range to CI

* [ci] Fix violation passed to danger js

* [docs] explain scripts folder structure

* [ci] Remove debug tasks

* [ci] Report even with no changes

* [ci] Fix danger js violation if no important change

* [ci] Fix no change being considered any change

* [ci] Improve review expierence

* [core] Remove rollup snapshot

createSizeSnapshot script takes care of this

* [ci] improve config docs

* [ci] Fix emoji in bundle changes details

* [ci] cleanup danger async runner

* [ci] reintriduce umd smoke test

was only removed to speed up CI

* [ci] Fix lint errors

* [ci] Refactor code

- improves code docs
- move some generic utils to utils module

* [ci] Use +-inf the mark added/removed snapshots

* [ci] Fix new bundles being considered as reducing size

* [ci] Consider non-existing as zero

allows for more useful comparison:
- add means current.size abs change, +inf rel change
- remove means -prev.size abs change, -100% rel change

* [ci] Place change emoji after percent

looks "weird" otherwise

* [ci] Fix increase being displayed like reduction (triangle down)

* [ci] Fix outdated code comments

* [ci] Use upercase snakecase for constants

* [core] Remove legacy scripts

* [ci] Improve destructuring semantics

* [ci] Exit with non-zero for any error in danger

This will leave a "fail" comment on the PR but it's better than having
the build show green.

* [scripts] Remove utils in favor of lodash
  • Loading branch information
eps1lon authored Feb 21, 2019
1 parent 6a36f8d commit f0cdca8
Show file tree
Hide file tree
Showing 12 changed files with 767 additions and 128 deletions.
52 changes: 45 additions & 7 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -179,13 +179,12 @@ jobs:
- run:
name: Can we generate the @material-ui/system build?
command: cd packages/material-ui-system && yarn build
# Netlify already do it for us but we need to check the size.
- run:
name: Can we build the docs?
command: yarn docs:build
- run:
name: Is the size acceptable?
command: yarn lerna bootstrap && yarn size
- persist_to_workspace:
root: .
paths:
- packages/*/build
# rollup snapshot
- packages/material-ui/size-snapshot.json
- run:
name: Install dependencies for Chrome Headless
# From https://github.com/GoogleChrome/puppeteer/blob/811415bc8c47f7882375629b57b3fe186ad61ed4/docs/troubleshooting.md#chrome-headless-doesnt-launch
Expand Down Expand Up @@ -232,6 +231,42 @@ jobs:
command: |
DOCKER_TEST_URL=http://$(ip addr show lo | grep "inet\b" | awk '{print $2}' | cut -d/ -f1):3090 yarn test:regressions
yarn argos
size_snapshot:
<<: *defaults
steps:
- checkout
- *restore_yarn_offline_mirror
- *restore_yarn_cache
- *install_js
- attach_workspace:
at: /tmp/workspace
- run:
name: Restore build
command: |
sudo apt install -y rsync
rsync -a /tmp/workspace/ .
# Netlify already do it for us but we need to check the size.
- run:
name: Can we build the docs?
command: yarn docs:build
- run:
name: Create a size snapshot
command: yarn size:snapshot
# downloaded by aws lambda to s3 bucket
# lambda allowes us to limit this to mui-org-branches-only while hiding credentials
# that allow write access to s3
- store_artifacts:
path: size-snapshot.json
- run:
name: Possibly persist size snapshot
command: |
if [ -z "$CI_PULL_REQUEST" ]; then
echo "no pull request; lets persist the size snapshot"
curl -X PUT --header "x-api-key: $CIRCLE_AWS_API_KEY" https://t6nulys5kl.execute-api.us-east-1.amazonaws.com/v1/persist-size-snapshot?build-id=$CIRCLE_BUILD_NUM
else
echo "pull request; let's run dangerJS"
yarn danger ci
fi
workflows:
version: 2
pipeline:
Expand All @@ -258,3 +293,6 @@ workflows:
- test_unit
- test_browser
- test_build
- size_snapshot:
requires:
- test_build
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,4 @@
build
node_modules
package-lock.json
size-snapshot.json
105 changes: 0 additions & 105 deletions .size-limit.js

This file was deleted.

190 changes: 190 additions & 0 deletions dangerfile.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,190 @@
/* eslint-disable no-console */
// inspire by reacts dangerfile
// danger has to be the first thing required!
const { danger, markdown } = require('danger');
const { exec } = require('child_process');
const { loadComparison } = require('./scripts/sizeSnapshot');

const parsedSizeChangeThreshold = 300;
const gzipSizeChangeThreshold = 100;

/**
* executes a git subcommand
* @param {any} args
*/
function git(args) {
return new Promise((resolve, reject) => {
exec(`git ${args}`, (err, stdout) => {
if (err) {
reject(err);
} else {
resolve(stdout.trim());
}
});
});
}

const UPSTREAM_REMOTE = 'danger-upstream';

/**
* This is mainly used for local development. It should be executed before the
* scripts exit to avoid adding internal remotes to the local machine. This is
* not an issue in CI.
*/
async function cleanup() {
await git(`remote remove ${UPSTREAM_REMOTE}`);
}

/**
* creates a callback for Object.entries(comparison).filter that excludes every
* entry that does not exceed the given threshold values for parsed and gzip size
*
* @param {number} parsedThreshold
* @param {number} gzipThreshold
*/
function createComparisonFilter(parsedThreshold, gzipThreshold) {
return comparisonEntry => {
const [, snapshot] = comparisonEntry;
return (
Math.abs(snapshot.parsed.absoluteDiff) >= parsedThreshold ||
Math.abs(snapshot.gzip.absoluteDiff) >= gzipThreshold
);
};
}

/**
* checks if the bundle is of a package e.b. `@material-ui/core` but not
* `@material-ui/core/Paper`
* @param {[string, any]} comparisonEntry
*/
function isPackageComparison(comparisonEntry) {
const [bundleKey] = comparisonEntry;
return /^@[\w-]+\/[\w-]+$/.test(bundleKey);
}

/**
* Generates a user-readable string from a percentage change
* @param {number} change
* @param {string} goodEmoji emoji on reduction
* @param {string} badEmooji emoji on increase
*/
function addPercent(change, goodEmoji = '', badEmooji = ':small_red_triangle:') {
const formatted = (change * 100).toFixed(2);
if (/^-|^0(?:\.0+)$/.test(formatted)) {
return `${formatted}%${goodEmoji}`;
}
return `+${formatted}%${badEmooji}`;
}

/**
* Generates a Markdown table
* @param {string[]} headers
* @param {string[][]} body
* @returns {string}
*/
function generateMDTable(headers, body) {
const tableHeaders = [headers.join(' | '), headers.map(() => ' --- ').join(' | ')];

const tablebody = body.map(r => r.join(' | '));
return `${tableHeaders.join('\n')}\n${tablebody.join('\n')}`;
}

function generateEmphasizedChange([bundle, { parsed, gzip }]) {
// increase might be a bug fix which is a nice thing. reductions are always nice
const changeParsed = addPercent(parsed.relativeDiff, ':heart_eyes:', '');
const changeGzip = addPercent(gzip.relativeDiff, ':heart_eyes:', '');

return `**${bundle}**: parsed: ${changeParsed}, gzip: ${changeGzip}`;
}

async function run() {
// Use git locally to grab the commit which represents the place
// where the branches differ
const upstreamRepo = danger.github.pr.base.repo.full_name;
const upstreamRef = danger.github.pr.base.ref;
try {
await git(`remote add ${UPSTREAM_REMOTE} https://github.com/${upstreamRepo}.git`);
} catch (err) {
// ignore if it already exist for local testing
}
await git(`fetch ${UPSTREAM_REMOTE}`);
const mergeBaseCommit = await git(`merge-base HEAD ${UPSTREAM_REMOTE}/${upstreamRef}`);

const commitRange = `${mergeBaseCommit}...${danger.github.pr.head.sha}`;

const comparison = await loadComparison(mergeBaseCommit, upstreamRef);
const results = Object.entries(comparison.bundles);
const anyResultsChanges = results.filter(createComparisonFilter(1, 1));

if (anyResultsChanges.length > 0) {
markdown('This PR introduced some changes to the bundle size.');

const importantChanges = results
.filter(createComparisonFilter(parsedSizeChangeThreshold, gzipSizeChangeThreshold))
.filter(isPackageComparison)
.map(generateEmphasizedChange);

// have to guard against empty strings
if (importantChanges.length > 0) {
markdown(importantChanges.join('\n'));
}

const detailsTable = generateMDTable(
[
'bundle',
'parsed diff',
'gzip diff',
'prev parsed',
'current parsed',
'prev gzip',
'current gzip',
],
results.map(([bundle, { parsed, gzip }]) => {
return [
bundle,
addPercent(parsed.relativeDiff),
addPercent(gzip.relativeDiff),
parsed.previous,
parsed.current,
gzip.previous,
gzip.current,
];
}),
);

const details = `
<details>
<summary>Details of bundle changes.</summary>
<p>Comparing: ${commitRange}</p>
${detailsTable}
</details>`;

markdown(details);
} else {
// this can later be removed to reduce PR noise. It is kept for now for debug
// purposes only. DangerJS will swallow console.logs if completes successfully
markdown(`No bundle size changes comparing ${commitRange}`);
}
}

(async () => {
let exitCode = 0;
try {
await run();
} catch (err) {
console.error(err);
exitCode = 1;
}

try {
await cleanup();
} catch (err) {
console.error(err);
exitCode = 1;
}

process.exit(exitCode);
})();
5 changes: 3 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,7 @@
"lint:fix": "eslint . --cache --fix && echo \"eslint: no lint errors\"",
"prettier": "yarn babel-node ./scripts/prettier.js",
"prettier:all": "yarn babel-node ./scripts/prettier.js write",
"size": "size-limit",
"size:overhead:why": "size-limit --why ./test/size/overhead.js",
"size:snapshot": "node scripts/createSizeSnapshot",
"size:why": "size-limit --why packages/material-ui/build/index.js",
"start": "yarn docs:dev",
"test": "yarn lint && yarn typescript && yarn test:coverage",
Expand Down Expand Up @@ -103,6 +102,7 @@
"cross-env": "^5.1.1",
"css-loader": "^2.0.0",
"css-mediaquery": "^0.1.2",
"danger": "^7.0.12",
"date-fns": "2.0.0-alpha.21",
"doctrine": "^3.0.0",
"downshift": "^3.0.0",
Expand Down Expand Up @@ -140,6 +140,7 @@
"karma-sourcemap-loader": "^0.3.7",
"karma-webpack": "^3.0.0",
"lerna": "^3.4.3",
"lodash": "^4.17.11",
"lz-string": "^1.4.4",
"markdown-to-jsx": "^6.8.3",
"material-ui-pickers": "^2.0.2",
Expand Down
Loading

0 comments on commit f0cdca8

Please sign in to comment.