-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Setup for Circle 2 #4149
Setup for Circle 2 #4149
Conversation
you can probably share some of the values using steps: &steps
- checkout
- ...
test-node-6:
<<: *steps |
Yup, we need a docker container with chrome. |
i love that we have a separate build for each version! |
.circleci/config.yml
Outdated
docker: | ||
- image: circleci/node:8.1.4 | ||
steps: | ||
<<: *checkout |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, i don't think you can merge sequences.
ok, got to the point where only node 4 fails. |
No, it's important that we verify Jest runs on travis. One of the primary use cases of Jest is to run on CI, and Travis is one of the most popular CI systems. |
At least we'll have faster Circle feedback ;) |
@cpojer that makes sense! i'll try to get travis fixed in master then. |
@aaronabramov Node 4 fails, because local packages are not linked (because they're installed with npm) |
oh. so that's a bigger issue.. |
This was intentional, I'd like to know for sure if Jest can be safely installed and used on Node 4, because npm 2 produces different node_modules structure, to prevent bugs like we had when releasing jest 20 |
Looks like we have eslint plugin import installed, I just need to add no-extraneous-dependencies rule so we prevent bugs caused by using transitive deps |
Codecov Report
@@ Coverage Diff @@
## master #4149 +/- ##
=======================================
Coverage 60.55% 60.55%
=======================================
Files 196 196
Lines 6777 6777
Branches 6 6
=======================================
Hits 4104 4104
Misses 2670 2670
Partials 3 3
Continue to review full report at Codecov.
|
@cpojer are we good to merge it? |
Looks good to me. Can we make it so the website is only deployed when the tests run successfully at least in one version of node? |
Should be possible. I'll play around with that someday |
|
||
- &yarn-install | ||
run: | | ||
sudo npm i -g yarn@^0.27.5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Node.js Docker image already has Yarn installed, so you shouldn't need to do this. Also, we discourage installing Yarn via npm 😃
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isn't it a super old version of yarn though?
i remember i had to manually install it on travis
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Travis doesn't use the Node.js Docker image. I think the Node.js image has the latest Yarn.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This docker image was using yarn 0.24, that's why I had to install it manually. Will check if that was updated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we do want to upgrade Yarn, we should do that in a Docker image that's layered on top of another one 😃
We should at least be installing Yarn using the Debian package if the image is Debian, based, too :D
Feel free to reach out to me if you need help with it!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would appreciate some help here, I'm not really experienced with Docker images 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's docs on CircleCI's site: https://circleci.com/docs/2.0/custom-images/
I was going to submit a PR for this, but then I realised it's actually using a bunch of Docker images for various Node.js versions 😛 Initially I was going to do something like this:
# Dockerfile for building and testing Jest
FROM markhobson/node-chrome
MAINTAINER Daniel Lo Nigro <daniel@dan.cx>
# Yarn repo
ADD https://dl.yarnpkg.com/debian/pubkey.gpg /tmp/yarn-pubkey.gpg
RUN apt-key add /tmp/yarn-pubkey.gpg && rm /tmp/yarn-pubkey.gpg
RUN echo "deb http://dl.yarnpkg.com/debian/ stable main" > /etc/apt/sources.list.d/yarn.list
# Crowdin repo
ADD https://artifacts.crowdin.com/repo/GPG-KEY-crowdin /tmp/crowdin-pubkey.gpg
RUN apt-key add /tmp/crowdin-pubkey.gpg && rm /tmp/crowdin-pubkey.gpg
RUN echo "deb https://artifacts.crowdin.com/repo/deb/ /" > /etc/apt/sources.list.d/crowdin.list
RUN apt-get -y update && \
apt-get install -y --no-install-recommends \
yarn \
default-jre \
crowdin \
&& \
apt-get clean && \
rm -rf /var/lib/apt/lists/*
# crowdin install start | ||
sudo apt-get install default-jre | ||
wget https://artifacts.crowdin.com/repo/deb/crowdin.deb -O crowdin.deb | ||
sudo dpkg -i crowdin.deb |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally all required software should be installed in a Docker image so that you don't incur the cost every single time the build runs (instead it's just a once-off cost whenever you build the Docker container). That's one of the main advantages of CircleCI 2.0 :) I might submit a PR to do that, if I get a chance.
Basically, there should be some Jest dev Docker image that extends whichever other Docker images you're using, and has this stuff installed.
* Setup for Circle 2 * Try to reuse config * Fix aliases and add browser build * Adjust config * Fix save-cache alias * Add website/node_modules to alias cache * Change browser docker * Add working_directory * add yarn-install-no-sudo * Remove environment * Actually skip react-native example on Node 4 * Declare transitive deps in corresponding package.json * Install node 4 deps with yarn * Setup import/no-extraneous-dependencies rule
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Summary
This is experimental PR to switch to Circle 2.
The main advantage of v2 is "Workflows" feature, enabling easier setup for parallel builds.
There's a lot of repetitive code, but I'm not sure if we can do anything about it.
Test plan
CI pass