[CI] Migrate to Circle 2.0 #14955

hramos · 2017-07-11T22:17:55Z

Following the migration guide. Let's see what happens here.

ide

My guess is that it'll be hard to write a working YAML file the first time, might be easier to incrementally work on it in this PR.

ide · 2017-08-18T16:55:50Z

.circleci/config.yml

+version: 2
+jobs:
+  build:
+    machine: true


CircleCI stopped recommending the machine key and said they might charge for it. I don't think we need a machine executor since we're not using docker-compose. I think we want to use the Java 8 image (circleci/openjdk:8-jdk) and then install Node separately or add a new build to the workflow that uses the Node image to build the website in parallel.

We've been using workflows with separate images (in particular, testing on different Node versions, just like Travis does by default) and it's been working well. Not sure about performance disadvantages though.

ide · 2017-08-18T16:57:46Z

.circleci/config.yml

+    steps:
+      - checkout
+      - restore_cache:
+          key: react-native


How about splitting the cache up more granularly, one key for each of the four directories being cached? This way we can invalidate the cache in a more fine-grained way too.

ide · 2017-08-18T16:59:56Z

.circleci/config.yml

+            cd website && npm install
+
+      - save_cache:
+          key: react-native


Can you add a version to the cache keys (just prefix with "v1-") so we can invalidate them by hand easily? Also for node_modules and website/node_modules, how about separate caches where each key name contains {{ checksum "package.json" }}?

ide · 2017-08-18T17:01:07Z

.circleci/config.yml

+      - save_cache:
+          key: react-native
+          paths:
+            - "ReactAndroid/build/downloads"


If the list of RN downloads changes we'll want the key to invalidate itself. Perhaps we should include the checksum of one of the Gradle files in a cache key.

ide · 2017-08-18T17:02:29Z

.circleci/config.yml

+            #find . -type f -regex ".*/buck-out/gen/ReactAndroid/src/test/.*/.*xml" -exec cp {} $CIRCLE_TEST_REPORTS/junit/ \;
+
+      - deploy:
+          branch: [/.*-stable/, /master/]


I'm not sure if this syntax is supported anymore since the CircleCI docs say to check the branch name in the command section manually.

ide · 2017-08-18T17:10:19Z

.circleci/config.yml

+            # CIRCLE_NPM_TOKEN is in React Native project settings in Circle CI.
+            # It was generated for bestander user, easy to replace with anyone's else
+            echo "//registry.npmjs.org/:_authToken=${CIRCLE_NPM_TOKEN}" > ~/.npmrc
+            npm config set spin=false


I think we'll need to install Node before this, probably need to get nvm first

grabbou · 2017-08-21T09:42:26Z

Not sure it's worth it, but one small tweak that I've applied when migrating was to take advantage of .circleci folder and extract all commands into separate scripts. That way, I've reduced config size and made it more analysable at a glance (quite handy, when workflows are in place)

serima · 2017-09-05T01:45:49Z

I think that CI's speed-up will contribute to improving the progress of the project and I update my favorite projects to CircleCI 2.0. I want to work on this issue together. How do I proceed?

hramos · 2017-09-05T17:28:28Z

@serima I don't think I'll get back to this PR for some time as this month is pretty busy for me, but any PR opened against this repo should trigger a CI run. You can start fresh or use this PR as a base, and open a new PR with your changes. There's several things running on Circle, so a successful PR will need to ensure every test is covered, including website deployment and publishing to npm.

serima · 2017-09-05T22:11:46Z

Hi, @hramos, I see. Thank you.

hramos · 2017-09-05T22:44:31Z

Had some extra time between tasks today. Instead of migrating all of the old tests over, I'll handle them one by one.

Hopefully I'll have more time to dig into this tomorrow. My plate is quite full at the moment and I was not planning on working on this just yet, but it may be necessary in order to test multiple Node versions concurrently.

ide · 2017-09-05T23:08:14Z

.circleci/config.yml

+      steps:
+        - checkout
+        - restore-cache: *restore-cache
+        - npm install


Need this to be behind the run: key. Also might want to add --no-package-lock since npm 5 adds package-lock.json.

ide · 2017-09-05T23:09:07Z

.circleci/config.yml

+  test-node-8:
+      working_directory: ~/react-native
+      docker:
+        - image: circleci/node:8.1.4


What do you think of just making this be circleci/node:8 so we naturally test the latest version of 8.x (but give up some reproduceability)?

ide · 2017-09-05T23:09:35Z

.circleci/config.yml

+          npm run flow -- check
+          npm test -- --maxWorkers=1
+  test-node-4:
+    working_directory: ~/jest


Should this be ~/react-native like the others?

It should! I followed Jest's as a starting point, and I overlooked this line.

ide · 2017-09-05T23:11:33Z

.circleci/config.yml

+aliases:
+  - &restore-cache
+    keys:
+      - dependencies-{{ .Branch }}-{{ checksum "package.json" }}


Can you prefix all the keys with v1- so we can easily manually bust the cache if we need to?

I'm also concerned about sharing node_modules caches across npm versions. In particular, Node 4 comes with npm 2 which doesn't have the deduping functionality of npm 3+.

What if we didn't cache node_modules for react-native but continued to cache them for the website and danger (where we use just a single Node + npm version)?

serima · 2017-09-06T00:36:43Z

Thank you very much. The point of @ide's code review seems very appropriate. After that, it takes time to repeat trial and error.
Anyway, I appreciate your thoughtfulness.

…se Node 8, s/jest/react-native/

hramos · 2017-09-06T06:23:33Z

Yup, this has been helpful. I had to push out the previous commit earlier before I had to run. Taking another stab at doing some basic JS tests tonight.

hramos · 2017-09-06T06:28:14Z

Lints and basic tests are working on Node 6 and 8. npm install failed on 4 as expected. Having all the individual workflow results listed here on GitHub is very useful, and should help us get better signal even when some tests are broken.

hramos · 2017-09-07T20:57:26Z

Forgot to mention: caching is in place in various places, which should significantly speed up the build!

hramos · 2017-09-07T23:59:21Z

Failures are in the Animated library, I suspect it's because our AVD is launched with no hw accel.

hramos · 2017-09-08T00:03:17Z

Looks like starting the emulator like so should fix this, but I am getting a segfault:

$ LD_LIBRARY_PATH=${ANDROID_HOME}/emulator/lib64:${ANDROID_HOME}/emulator/lib64/qt/lib emulator64-arm -avd testAVD -no-audio -no-window -no-boot-anim -gpu on
Segmentation fault

This post on SO has more info. I’ll continue tomorrow.
https://stackoverflow.com/questions/45424110/how-to-run-android-ui-tests-properly-on-circleci-2-0

…l (unnecessary)

hramos · 2017-09-08T21:17:07Z

I'm afraid Docker images on Circle are unable to take advantage of hardware acceleration. If I understand correctly, the Animated tests require hw accel.

ide · 2017-09-08T21:22:41Z

This is getting out of my depth but FWIW CircleCI 1.0 ran its tests in Docker too, which makes me think that it's possible to run the Animated tests somehow. Mind if I clone this PR and try to poke around?

hramos · 2017-09-08T21:55:19Z

Yup, go for it. If you ssh into the machine, make sure to update your PATH in the same way as we do in https://github.com/facebook/react-native/pull/14955/files#diff-1d37e48f9ceff6d8030570cd36286a61R198 because $BASH_ENV points to a random file when this runs in the image. Once the ssh-enabled run has installed all deps, you should be able to use buck to run the tests manually.

The JS bundle won't be available to you when you re-run with SSH enabled though, as it's built in a different job and the attach_workspace step fails when the build is retried. I'll see if I can provide the bundle as an artifact to aid with debugging.

hramos · 2017-09-08T22:05:29Z

In case it helps, there's a difference in how we manage SDKs and the AVD in Circle 1.0 versus Circle 2.0. With Circle 1.0, we use the android CLI which is now deprecated, and we start the AVD using $ANDROID_HOME/tools/emulator -avd testAVD -no-skin -no-audio -no-window.

With Circle 2.0, I'm using sdkmanager instead to manage all SDK dependencies, and the AVD is started using emulator64-arm. I'm not very familiar with Android tooling, but it's possible that the emulator CLI used in Circle 1.0 supports hw accel. I've attempted to enable the GPU with emulator64-arm, but it only results in a Segmentation fault if the ARM image is used. I looked into using the x86 image, but after looking at the instructions to enable hw accel in linux, it looks like the Docker image is not running in a host that would allow us to use hw accel.

Ideally, I'd set up the Android SDK and AVD in the same manner as we do in Circle 1.0, but it looks like both the android-23 and android-26 images are already set up with recent versions of the Android SDK where android is already deprecated. I'll keep prodding here and see if I can get closer to the Circle 1.0 config. In any case, I'd still like to get this working with the latest tools, instead of relying on a deprecated cli.

CircleCI doesn't let us map objects under /dev/shm from within the Docker container so another directory instead when running in CircleCI. This change shouldn't impact FB's internal CI.

See if we can get a green run on CircleCI 2 by commenting out tests that we know don't pass on CircleCI 1

ide · 2017-09-20T00:57:30Z

@hramos I added a couple of commits at the end to fix Android unit tests (ended up being unrelated to GPU) and to comment out Android integration tests that were commented out in the CircleCI 1 config anyway. Tests look green now -- do you want to uncomment the Node 6 and Website jobs and then land this?

hramos · 2017-09-20T23:10:46Z

Uncommented tests. This should be good to go now.

facebook-github-bot · 2017-09-20T23:14:42Z

@hramos has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

facebook-github-bot · 2017-09-21T11:48:14Z

I tried to merge this pull request into the Facebook internal repo but some checks failed. To unblock yourself please check the following: Does this pull request pass all open source tests on GitHub? If not please fix those. Does the code still apply cleanly on top of GitHub master? If not can please rebase. In all other cases this means some internal test failed, for example a part of a fb app won't work with this pull request. I've added the Import Failed label to this pull request so it is easy for someone at fb to find the pull request and check what failed. If you don't see anyone comment in a few days feel free to comment mentioning one of the core contributors to the project so they get a notification.

Migrating to Circle 2

0dc5af0

facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. GH Review: review-needed labels Jul 11, 2017

hramos added 3 commits July 11, 2017 15:19

Add working_directory

0b6dde4

Update working directory

5df9b22

Use actual home dir

2880d0c

ide reviewed Aug 18, 2017

View reviewed changes

hramos self-assigned this Aug 24, 2017

Reverting to minimal configuration, running tests on Node.

12a7898

Add workflows

30cbad6

ide reviewed Sep 5, 2017

View reviewed changes

Prefix npm with run:, remove node_modules caching for react-native, u…

2c2e469

…se Node 8, s/jest/react-native/

hramos added 9 commits September 6, 2017 00:07

Prefix cache keys

7bee3e8

Android tests

6921bef

Fix parsing error

1bd2934

Remove filter

9c9e0d8

Fix indentation

1b66576

Remove colon

317d5ff

Remove colon

d5aee4b

Fix PATH env var

7fd9c66

Build JS bundle in Node image before running android tests

13a6dce

hramos added 4 commits September 8, 2017 10:49

Explicitly install various Android packages, remove NDK 32-bit instal…

5b13f5a

…l (unnecessary)

Bump docker image to API level 26

8867db7

Slight logging change

09375d6

Install NDK using sdkmanager

a938df0

hramos and others added 5 commits September 8, 2017 15:16

Provide js bundle as an artifact, try old emulator command

db5daf7

Restore 32-bit, 64-bit NDK manual installs

25c4b82

yaml typo

447601f

[ci] Don't set /dev/shm as the temp dir in CircleCI

99e5c8f

CircleCI doesn't let us map objects under /dev/shm from within the Docker container so another directory instead when running in CircleCI. This change shouldn't impact FB's internal CI.

[ci] Comment out Android tests we know are failing

666958c

See if we can get a green run on CircleCI 2 by commenting out tests that we know don't pass on CircleCI 1

hramos added 2 commits September 20, 2017 12:36

Uncomment tests

c4716ca

fix yaml

bc964ac

hramos changed the title ~~[WIP] Migrating to Circle 2~~ [CI] Migrate to Circle 2.0 Sep 20, 2017

facebook-github-bot added GH Review: accepted Import Started This pull request has been imported. This does not imply the PR has been approved. and removed GH Review: review-needed labels Sep 20, 2017

facebook-github-bot added Import Failed and removed Import Started This pull request has been imported. This does not imply the PR has been approved. labels Sep 21, 2017

facebook-github-bot closed this in 8fa9984 Sep 21, 2017

hramos deleted the circle2 branch September 23, 2017 20:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CI] Migrate to Circle 2.0 #14955

[CI] Migrate to Circle 2.0 #14955

hramos commented Jul 11, 2017

ide left a comment

ide Aug 18, 2017

grabbou Aug 21, 2017

ide Aug 18, 2017

ide Aug 18, 2017

ide Aug 18, 2017

ide Aug 18, 2017

ide Aug 18, 2017

grabbou commented Aug 21, 2017

serima commented Sep 5, 2017

hramos commented Sep 5, 2017

serima commented Sep 5, 2017

hramos commented Sep 5, 2017

ide Sep 5, 2017 •

edited

Loading

ide Sep 5, 2017

ide Sep 5, 2017

hramos Sep 6, 2017

ide Sep 5, 2017

serima commented Sep 6, 2017

hramos commented Sep 6, 2017

hramos commented Sep 6, 2017

hramos commented Sep 7, 2017

hramos commented Sep 7, 2017

hramos commented Sep 8, 2017 •

edited

Loading

hramos commented Sep 8, 2017

ide commented Sep 8, 2017

hramos commented Sep 8, 2017

hramos commented Sep 8, 2017 •

edited

Loading

ide commented Sep 20, 2017

hramos commented Sep 20, 2017

facebook-github-bot commented Sep 20, 2017

facebook-github-bot commented Sep 21, 2017

[CI] Migrate to Circle 2.0 #14955

[CI] Migrate to Circle 2.0 #14955

Conversation

hramos commented Jul 11, 2017

ide left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

grabbou commented Aug 21, 2017

serima commented Sep 5, 2017

hramos commented Sep 5, 2017

serima commented Sep 5, 2017

hramos commented Sep 5, 2017

ide Sep 5, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

serima commented Sep 6, 2017

hramos commented Sep 6, 2017

hramos commented Sep 6, 2017

hramos commented Sep 7, 2017

hramos commented Sep 7, 2017

hramos commented Sep 8, 2017 • edited Loading

hramos commented Sep 8, 2017

ide commented Sep 8, 2017

hramos commented Sep 8, 2017

hramos commented Sep 8, 2017 • edited Loading

ide commented Sep 20, 2017

hramos commented Sep 20, 2017

facebook-github-bot commented Sep 20, 2017

facebook-github-bot commented Sep 21, 2017

ide Sep 5, 2017 •

edited

Loading

hramos commented Sep 8, 2017 •

edited

Loading

hramos commented Sep 8, 2017 •

edited

Loading