refactor(ci): Bypass Lerna and repurpose the 'test-travis' run-script #129

spencerwilson-optimizely · 2018-06-28T16:47:21Z

Summary

Bypass Lerna things in CI
Don't install grunt. I presume it was installed because test-travis refers to it, but locally-installed executable modules are on the $PATH in run-scripts so global installation is unnecessary.
Rename test-travis to test-xbrowser and modify it such that it only runs cross-browser tests in BrowserStack (via grunt), rather than re-running unit tests headlessly+locally again with npm test (which is done as part of the prepare lifecycle hook that's invoked during npm install). The rename makes it clearer that the run-script could plausibly, usefully be run locally as well (say, if one was debugging a consistent failure in some browser).

Lerna seems to be correlated with some recent sporadic timeout failures:

This PR cd's the Travis CI pipeline directly into the optimizely-sdk package's dir, and does things there. Doing so seems to reduce the mean CI time (compute time on Travis machines, not wall-clock time) from about 21 mins to 15 mins.

Test plan

This PR's CI run: #288.

tylerbrandt

Like the refactor! I would argue that this commit ought to have a "refactor" type since it's not intended to change functionality and it doesn't add a test.

I'm not totally sure we need to bypass lerna (build issues seem orthogonal and in fact unreproducible now).

tylerbrandt · 2018-06-28T23:10:14Z

.travis.yml

@@ -15,12 +15,9 @@ env:
  - secure: WnyM4gMsi2n69O/YZUD/pYwHJXdKDcBv3Hwft2cCw52yYc+z75uuRgdaLKs4BPisckBtnR17dH7hKlPX3HWwjCoqQm1q5qNpbJrArWaEcbotWGF2YFy21ZZ4rKNQmJqdgRj6XFZhLHbncA8v2gQPK7F6GUJ0vsJF/kiTfxAUjefR23oorcKSQrh9BfOxNAYu2Ma92qlaaMmHYbBdlNDM45/EQE+LnPCfboCiJD/5zTYq4Q+XhKLPV01vUDU60pj9ckDNXyLj9X2BwMbAzGAPGE4qTAB/IrMndVsUXblsahtwKQ6yrsUrsdTASz8/3oNkImtY7fqU874jSeG3d7PNBfZs47zkXEVy73ZWNBgM9rzVS5cPaIU3wqpuBoXFntDJcdHQhNTWEYdxmtcTUmxKt5TdUzDhrrkcti2WVLabU3N52aOBeOM0XBpfLkbV+HT6oWi3bNUb+EDMHvCxOxsP4IoEDfFs9HMzNIO3mmC3+2DFbI7s2Mb2oacAut38MbJDYSDTOLL4smG8scA3E0RQO4r8+TNk4aRIMQc7vCKqz7PpbO7Aj9dXSpeHrDmIszSmEoQqmaaGsRBwbXRom2P8fB9FcTbd/wbsfgoFNEPz5DlbtCtCmt0pQMa+3myWveKH52WC5KlFijBSDjYOMUnXbLnj5fK5eKaWp+z6/qcNwU8=
  # BROWSER_STACK_ACCESS_KEY
  - secure: U0GGZw46rJowBtH9gVluIrerB40u2b3uZpH0HsOdLlsXCCaTVk4JXX/JPVPashWAFLC7Enk3UOE4ofeEpVd0wbG6CxtG9/gklc2U2tvkqsdPpFZKaRrXoUzCyyPOmHEC2mXDXctbrncmttM4APaceRfbdTBEZIIfyLJadomjWylA61szFE9IZjvJpiwJO2xa5HI9GVRu3yXJci+riJux+JsDmfJ1hNwv3waMeeg/scddUH0hfgq69ftGs8cpMlYiO20eh32S7uPF7/IJTH1fDJjVKYQZwpypkF6AeI+od5CFTY1ajb25eaBNXThLS0Bo9ZJE/8Sogvon21dEJkt/ClY6R341InbAFXZvz7jyQAisvh0I4zxcu0VUCfh7bEUl6GXMO8VJnyxHEfqB+AIT2RoMXckkhulwiNUsJYH1yJ8mjnLvZq85mWBCp4n4jg0K6Wf46lHpjnHOVpLyLyoFGfiPf90AQVL02AJ3/ia8RkMuj0Ax+AGtiTC/+wy7dsDQOif/VpBNJcx/RciQ24mYOGzAMh4GsUWnXaZ9vXSxliogVNrmIefK5invJ0omv9pIx8NZHTHYGaulh4w6JsliiEq2kH78SlyvSrcsFGTwCY97LLaxiLm/75/Zf+F7LajKC23Fbtnj/LQizitFZqGMJ09DnR52krBAeultqRq8QLM=
-before_script:
- npm install grunt-cli -g


not necessary anymore?

Never necessary, I think. Added an explanation to the PR summary.

👍 I wonder how much instability this added. I definitely saw on at least one build that the "hang" occurred while installing grunt-cli.

tylerbrandt

I have a mild preference to keep using lerna if we aren't sure it is actually the cause of the problem, just to avoid having so much unused stuff in this repo (alternatively, we could refactor the code to bring packages/optimizely-sdk to the top level). But as I mentioned before, the renamed scripts and removed unnecessary commands are a good thing.

tylerbrandt · 2018-06-29T16:29:11Z

.travis.yml

@@ -15,12 +15,9 @@ env:
  - secure: WnyM4gMsi2n69O/YZUD/pYwHJXdKDcBv3Hwft2cCw52yYc+z75uuRgdaLKs4BPisckBtnR17dH7hKlPX3HWwjCoqQm1q5qNpbJrArWaEcbotWGF2YFy21ZZ4rKNQmJqdgRj6XFZhLHbncA8v2gQPK7F6GUJ0vsJF/kiTfxAUjefR23oorcKSQrh9BfOxNAYu2Ma92qlaaMmHYbBdlNDM45/EQE+LnPCfboCiJD/5zTYq4Q+XhKLPV01vUDU60pj9ckDNXyLj9X2BwMbAzGAPGE4qTAB/IrMndVsUXblsahtwKQ6yrsUrsdTASz8/3oNkImtY7fqU874jSeG3d7PNBfZs47zkXEVy73ZWNBgM9rzVS5cPaIU3wqpuBoXFntDJcdHQhNTWEYdxmtcTUmxKt5TdUzDhrrkcti2WVLabU3N52aOBeOM0XBpfLkbV+HT6oWi3bNUb+EDMHvCxOxsP4IoEDfFs9HMzNIO3mmC3+2DFbI7s2Mb2oacAut38MbJDYSDTOLL4smG8scA3E0RQO4r8+TNk4aRIMQc7vCKqz7PpbO7Aj9dXSpeHrDmIszSmEoQqmaaGsRBwbXRom2P8fB9FcTbd/wbsfgoFNEPz5DlbtCtCmt0pQMa+3myWveKH52WC5KlFijBSDjYOMUnXbLnj5fK5eKaWp+z6/qcNwU8=
  # BROWSER_STACK_ACCESS_KEY
  - secure: U0GGZw46rJowBtH9gVluIrerB40u2b3uZpH0HsOdLlsXCCaTVk4JXX/JPVPashWAFLC7Enk3UOE4ofeEpVd0wbG6CxtG9/gklc2U2tvkqsdPpFZKaRrXoUzCyyPOmHEC2mXDXctbrncmttM4APaceRfbdTBEZIIfyLJadomjWylA61szFE9IZjvJpiwJO2xa5HI9GVRu3yXJci+riJux+JsDmfJ1hNwv3waMeeg/scddUH0hfgq69ftGs8cpMlYiO20eh32S7uPF7/IJTH1fDJjVKYQZwpypkF6AeI+od5CFTY1ajb25eaBNXThLS0Bo9ZJE/8Sogvon21dEJkt/ClY6R341InbAFXZvz7jyQAisvh0I4zxcu0VUCfh7bEUl6GXMO8VJnyxHEfqB+AIT2RoMXckkhulwiNUsJYH1yJ8mjnLvZq85mWBCp4n4jg0K6Wf46lHpjnHOVpLyLyoFGfiPf90AQVL02AJ3/ia8RkMuj0Ax+AGtiTC/+wy7dsDQOif/VpBNJcx/RciQ24mYOGzAMh4GsUWnXaZ9vXSxliogVNrmIefK5invJ0omv9pIx8NZHTHYGaulh4w6JsliiEq2kH78SlyvSrcsFGTwCY97LLaxiLm/75/Zf+F7LajKC23Fbtnj/LQizitFZqGMJ09DnR52krBAeultqRq8QLM=
-before_script:
- npm install grunt-cli -g


👍 I wonder how much instability this added. I definitely saw on at least one build that the "hang" occurred while installing grunt-cli.

tylerbrandt · 2018-06-29T16:30:24Z

packages/optimizely-sdk/package.json

@@ -6,7 +6,7 @@
  "browser": "lib/index.browser.js",
  "scripts": {
    "test": "mocha ./lib/*.tests.js ./lib/**/*.tests.js ./lib/**/**/*tests.js --recursive",
-    "test-travis": "npm run test && grunt",
+    "test-xbrowser": "grunt",


nit: It's a little weird that the "default" grunt command is one that runs xbrowser tests specifically; I might use the specific named grunt task here, if there is one.

I found it surprising too. It's defined as the default task in Gruntfile.js.

spencerwilson-optimizely · 2018-06-29T18:13:19Z

I have a mild preference to keep using lerna if we aren't sure it is actually the cause of the problem, just to avoid having so much unused stuff in this repo

I hear you. I'm still kind of swayed by how bypassing lerna seems to reduce build times. That's valuable to the extent that (at least as of 2015) open source build concurrency is limited to 5. 🤷‍♂️ Though that argument is weakened, I think, by the fact that when this repo moves to travis-ci.com, concurrency can be boosted because it'll be linked with our paid Travis CI account:

open source projects will be joining private projects on travis-ci.com ... This means you can ... allocate additional concurrency to either open source or private projects ...

(source)

## Summary - Convert `prepare` hook to a (not-equivalent) `prepublishOnly`, reducing the number of times unit tests are run in `prepare`-literate builds by 1. #129 introduced a regression where unit tests stopped being run for Node.js v4 and v6. Prior to that PR, unit tests were run - v4 and v6: once - v8, v9, v10, ... (versions whose npm understands the `prepare` hook): twice After that PR was merged, the number of times unit tests were run in each build went down by 1, _for every build_. That means v4 and v6 stopped running unit tests altogether. 2faa9c4 fixed the regression by unconditionally running unit tests on all matrix-based builds (and explicitly adding a build to run xbrowser tests). With this, Node.js 8+ builds resumed running unit tests twice ([example](https://travis-ci.org/optimizely/javascript-sdk/jobs/399704185)). Regarding the `prepare` hook: npm@4 added the [`prepare`](https://docs.npmjs.com/misc/scripts#prepublish-and-prepare) hook with that fires on prepublish and postinstall. This is strange. Why run tests when someone installs the package? They see the mocha output in their shell, which is likely to surprise. This PR stops the unconventional test runs on postinstall. ## Test plan CI.

## Summary - Re-enable CI against latest Node.js 10.x release It was disabled in #101, but from the [example failure](https://travis-ci.org/optimizely/javascript-sdk/jobs/380942404) given in that PR, I suspect the bug lies in lerna. No matter now, since lerna has been bypassed since #129. ## Test plan CI

test(ci): Bypass Lerna

98c392c

spencerwilson-optimizely added the wip label Jun 28, 2018

spencerwilson-optimizely added 4 commits June 28, 2018 10:54

change

c5d0ad2

Drop a compact disc

2ffdef6

Repurpose test-travis as test-xbrowser

85b2a0e

Remove things

a7006c1

spencerwilson-optimizely removed the wip label Jun 28, 2018

spencerwilson-optimizely requested review from tylerbrandt, mikeproeng37 and shihpatrick June 28, 2018 20:51

spencerwilson-optimizely changed the title ~~test(ci): Bypass Lerna~~ test(ci): Bypass Lerna and repurpose npm run test-travis Jun 28, 2018

spencerwilson-optimizely changed the title ~~test(ci): Bypass Lerna and repurpose npm run test-travis~~ test(ci): Bypass Lerna and the 'test-travis' run-script Jun 28, 2018

spencerwilson-optimizely changed the title ~~test(ci): Bypass Lerna and the 'test-travis' run-script~~ test(ci): Bypass Lerna and repurpose the 'test-travis' run-script Jun 28, 2018

tylerbrandt reviewed Jun 28, 2018

View reviewed changes

spencerwilson-optimizely changed the title ~~test(ci): Bypass Lerna and repurpose the 'test-travis' run-script~~ refactor(ci): Bypass Lerna and repurpose the 'test-travis' run-script Jun 29, 2018

tylerbrandt approved these changes Jun 29, 2018

View reviewed changes

spencerwilson-optimizely added 2 commits June 29, 2018 11:00

Merge remote-tracking branch 'origin/master' into sw/ci-bypass-lerna

4abd1f1

Explicitly name karma grunt task

8d5aa5b

spencerwilson-optimizely force-pushed the sw/ci-bypass-lerna branch from 3e9bf5c to 8d5aa5b Compare June 29, 2018 18:02

spencerwilson-optimizely merged commit 9d5d8ef into master Jun 29, 2018

spencerwilson-optimizely deleted the sw/ci-bypass-lerna branch June 29, 2018 18:14

spencerwilson-optimizely mentioned this pull request Jul 2, 2018

fix(publish): Change prepare to prepublishOnly #138

Merged

spencerwilson-optimizely mentioned this pull request Jul 3, 2018

chore(ci): Restore Node.js 10.x to Travis matrix #142

Merged

spencerwilson-optimizely mentioned this pull request Aug 21, 2018

CI: hang (sometimes) on lerna boostrap in travis-ci #131

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refactor(ci): Bypass Lerna and repurpose the 'test-travis' run-script #129

refactor(ci): Bypass Lerna and repurpose the 'test-travis' run-script #129

Uh oh!

spencerwilson-optimizely commented Jun 28, 2018 •

edited

Loading

Uh oh!

tylerbrandt left a comment

Uh oh!

tylerbrandt Jun 28, 2018

Uh oh!

spencerwilson-optimizely Jun 29, 2018

Uh oh!

tylerbrandt Jun 29, 2018

Uh oh!

tylerbrandt left a comment

Uh oh!

tylerbrandt Jun 29, 2018

Uh oh!

tylerbrandt Jun 29, 2018

Uh oh!

spencerwilson-optimizely Jun 29, 2018

Uh oh!

spencerwilson-optimizely commented Jun 29, 2018 •

edited

Loading

Uh oh!

Uh oh!

refactor(ci): Bypass Lerna and repurpose the 'test-travis' run-script #129

refactor(ci): Bypass Lerna and repurpose the 'test-travis' run-script #129

Uh oh!

Conversation

spencerwilson-optimizely commented Jun 28, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

tylerbrandt left a comment

Choose a reason for hiding this comment

Uh oh!

tylerbrandt Jun 28, 2018

Choose a reason for hiding this comment

Uh oh!

spencerwilson-optimizely Jun 29, 2018

Choose a reason for hiding this comment

Uh oh!

tylerbrandt Jun 29, 2018

Choose a reason for hiding this comment

Uh oh!

tylerbrandt left a comment

Choose a reason for hiding this comment

Uh oh!

tylerbrandt Jun 29, 2018

Choose a reason for hiding this comment

Uh oh!

tylerbrandt Jun 29, 2018

Choose a reason for hiding this comment

Uh oh!

spencerwilson-optimizely Jun 29, 2018

Choose a reason for hiding this comment

Uh oh!

spencerwilson-optimizely commented Jun 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

spencerwilson-optimizely commented Jun 28, 2018 •

edited

Loading

spencerwilson-optimizely commented Jun 29, 2018 •

edited

Loading