Skip to content

CI: Break up Build & Test, Add Parallelization#11574

Merged
JasonGore merged 21 commits intomicrosoft:masterfrom
JasonGore:jg/build-and-test
Jan 7, 2020
Merged

CI: Break up Build & Test, Add Parallelization#11574
JasonGore merged 21 commits intomicrosoft:masterfrom
JasonGore:jg/build-and-test

Conversation

@JasonGore
Copy link
Copy Markdown
Contributor

@JasonGore JasonGore commented Dec 28, 2019

Pull request checklist

  • Addresses an existing issue: Fixes #0000
  • Include a change request file using $ yarn change

Description of changes

This PR builds off the work started in #11422 to speed up local and CI build times by breaking up the existing build task into build, test, lint and bundle and then using the new tasks in parallel in a new parallelization pipeline.

Command Changes

Monorepo Build Command Prereqs Important Notes Description
build Now TypeScript only. No longer runs bundle, lint or test. No longer production build by default. Generates output using TypeScript.
build:prod New. Production version of build command.
buildci No change. This command approximates PR build steps and is useful for validating your build locally. Reproduction of PR build steps: build, lint, test, bundle.
buildci:prod New Production version of buildci command.
buildto Now TypeScript only. No longer runs bundle, lint or test. No longer production build by default. Builds to specified target package.
buildfast Removed (redundant with 'build' and 'buildto')
bundle build New Runs webpack. Webpack configs use lib output as entry point, requiring build as prereq.
bundle:prod build New Production version of bundle command.
clean New Cleans package output.
lint build New Runs lint and lint-imports tasks. lint-imports requires build artifacts to detect that physical file exist for imports from 'lib'.
test build New (replaces 'validate') Runs jest. Build artifacts required for dependencies of packages under test.
validate Removed (replaced by 'test')

Pipeline Strategy

Based on build times and prerequisites, this is the parallelization and job strategy defined by the pipeline:

Job Prereq Job Runs Description
Build TypeScript compliation only
Validate Build Test (jest) and lint Requires build artifacts for internal package deps.
Deploy Build Bundle (webpack) Requires build artifacts since webpack configs are set to use lib entry points.
PerfTest Build Perf test script Requires build artifacts for bundling and running perf-test script.
VRTest Build Visual Regression tests (screener) Requires build artifacts for internal package deps.

TODO (before merge):

  • Finalize command structure.
  • Finish testing (command output, release pipeline.)
  • Resolve TODOs in PR code.
  • Communicate build command changes via email.
  • Document command changes in wiki.
  • Coordinate timing of merge to deal with any possible release pipeline and publishing issues.
  • Try lerna --parallel for all post-build jobs to increase performance.
  • Fix PerfTest (currently needs Deploy as prereq which mostly negates build time savings)
  • Rename azure-pipelines.parallel.yml pipeline to azure-pipelines.yml.
Microsoft Reviewers: Open in CodeFlow

@msft-github-bot
Copy link
Copy Markdown
Contributor

msft-github-bot commented Dec 28, 2019

Component Perf Analysis

No significant results to display.

All results

Scenario Master Ticks PR Ticks Status
BaseButton 833 857
BaseButton (experiments) 1065 1043
DefaultButton 1132 1101
DefaultButton (experiments) 2112 2089
DetailsRow 3690 3615
DetailsRow (fast icons) 3621 3682
DetailsRow without styles 3342 3324
DocumentCardTitle with truncation 1608 1625
MenuButton 1443 1523
MenuButton (experiments) 3826 3706
PrimaryButton 1326 1274
PrimaryButton (experiments) 2195 2228
SplitButton 3095 3187
SplitButton (experiments) 7508 7582
Stack 517 489
Stack with Intrinsic children 1178 1178
Stack with Text children 4695 4750
Text 397 394
Toggle 930 931
Toggle (experiments) 2447 2509
button 74 67

Comment thread package.json Outdated
"start-exp": "yarn workspace @uifabric/experiments start",
"build": "lerna run build --stream -- --production --lint",
"buildci": "lerna run build --stream -- --lint",
"build": "lerna run build --stream --",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is -- necessary here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is in here for now because I'm not sure how else to pass the --production flag to build. It implicitly worked with buildci before because the arg comes after -- there as well.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've removed stray instances of trailing -- and added a few :prod convenience commands. Funny enough I found in testing that yarn build -- --production does not work, but yarn build -- -- --production DOES. I've decided to add the :prod as convenience commands, which are only slightly less awkward.

Comment thread azure-pipelines.parallel.yml Outdated
Comment thread azure-pipelines.tools.yml
versionSpec: '10.x'
displayName: 'Install Node.js'

- task: geeklearningio.gl-vsts-tasks-yarn.yarn-installer-task.YarnInstaller@3
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this dependency? Can we just directly install it with a shell command instead?

Copy link
Copy Markdown
Contributor Author

@JasonGore JasonGore Jan 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is needed to install a newer version of yarn since this pipeline added its use. Ken advised me on this approach. I'm open to other methods, as well.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jdhuntington - we could install it through shell, I've not tested how fast that is. This is just what we've been using for the other tasks, seems to "install" yarn pretty quickly and made available nicely without having to worry about installing something globally on a CI box. That's the only difference to me...

Comment thread azure-pipelines.parallel.yml Outdated
Copy link
Copy Markdown
Contributor

@kenotron kenotron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some minor nits - but it's pretty close!

Comment thread scripts/package.json Outdated
@JasonGore JasonGore requested a review from aftab-hassan January 2, 2020 21:03
Comment thread apps/pr-deploy-site/just.config.js Outdated
Copy link
Copy Markdown
Member

@ecraig12345 ecraig12345 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! A few questions/comments.

Comment thread apps/perf-test/tasks/perf-test.js
Comment thread apps/pr-deploy-site/just.config.js
Comment thread apps/ssr-tests/just.config.js Outdated
Comment thread azure-pipelines.tools.yml
Comment thread packages/tsx-editor/just.config.js
Comment thread change/@uifabric-api-docs-2019-12-27-17-16-21-jg-build-and-test.json Outdated
@kenotron
Copy link
Copy Markdown
Contributor

kenotron commented Jan 4, 2020

Please ignore that draft build... this build is green!

@JasonGore JasonGore marked this pull request as ready for review January 7, 2020 21:36
Copy link
Copy Markdown
Contributor

@kenotron kenotron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks so much for being so methodical about this change.

@JasonGore JasonGore merged commit f0beb26 into microsoft:master Jan 7, 2020
@micahgodbolt
Copy link
Copy Markdown
Member

micahgodbolt commented Jan 7, 2020

giphy

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants