Moving build to CircleCI #102

shen-tian · 2019-02-25T19:14:50Z

First stab at moving the linter(s) and builds over to CircleCI. Notes

Builds against the default Java8 JDK (Oracle?), and OpenJDK 8 and 11, per travis. Does not test against early access, but that would be the matter of finding the right build image.
Tests against clojure 1.8 1.9, 1.10 and master
Tests against nrepl 0.4 0.5 and 0.6.

Does not do the codecov side, as I imagine you don't want both travis and CircleCI doing that? But that should be relatively easy to get.

Does not user Orbs, as I suspect that's better left to the stage when we have a lot of projects that need to share configs, and it's clear what's needed.

Edit: link on where my fork is building: https://circleci.com/gh/shen-tian/workflows/piggieback/tree/circleci

cichli · 2019-02-25T19:39:10Z

Thank you for looking at this! 🙌

Does not do the codecov side, as I imagine you don't want both travis and CircleCI doing that? But that should be relatively easy to get.

It can probably handle it okay. It merges multiple builds from the same CI provider – see here – it should do the same for multiple providers. The hit counts will of course be inflated but the coverage should still be measured correctly.

Does not user Orbs, as I suspect that's better left to the stage when we have a lot of projects that need to share configs, and it's clear what's needed.

Agreed.

@bbatsov you will need to activate CircleCI for this repo here.

.circleci/config.yml

cichli

Looks great overall. Thanks again for looking at this!

.circleci/config.yml

shen-tian · 2019-02-26T06:03:48Z

A few other notes:

I don't think CircleCI supports anything equivalent to Travis's fast_finish/ allow_failures. Not sure what we want to do with the Clojure master/JDK earlyacess builds?
I've had a look at @plexus, progress on cider-nrepl builds. It uses some orbs I can't find the source to, so not sure what images he's using?

codecov-io · 2019-02-26T06:33:22Z

Codecov Report

Merging #102 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #102   +/-   ##
=======================================
  Coverage   85.78%   85.78%           
=======================================
  Files           1        1           
  Lines         190      190           
  Branches       11       11           
=======================================
  Hits          163      163           
  Misses         18       18           
  Partials        9        9

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7bcda97...cb6e6c8. Read the comment docs.

shen-tian · 2019-02-26T06:45:48Z

Added Cloverage/Codecov job. See results here: https://codecov.io/gh/shen-tian/piggieback/commits

plexus · 2019-02-26T16:02:05Z

The source for the orbs I've used is here: https://github.com/lambdaisland/meta/tree/master/circleci

The docker images used are created like this: https://nextjournal.com/plexus/openjdk11-%2B-clojure

shen-tian · 2019-02-26T17:32:19Z

The source for the orbs I've used is here: https://github.com/lambdaisland/meta/tree/master/circleci

The docker images used are created like this: https://nextjournal.com/plexus/openjdk11-%2B-clojure

Thanks! Will simplify a few things. @plexus: I haven't looked into it, but what does it take for an orb to be in the CircleCI repo?

@cichli : other than the CircleCI setup in the nrepl org, what do you feel we still need to do here?

shen-tian · 2019-02-27T06:29:30Z

I did some tidying up, with executors so that we don't have weird hanging - at end of jdk_versions. I didn't end up using @plexus's orbs, (after trying for a while), because there would have been a few fiddly bits to align on (running as root v.s. circleci etc.), which is not essential to this PR.

cichli

Everything looks great to me. Fantastic work, thank you! 🙌

cichli · 2019-02-27T10:42:58Z

I will merge once we have a confirmed build on this repo. cc @bbatsov :-)

bbatsov · 2019-03-09T09:04:04Z

Sorry for the delay. I'll take a look over the weekend.

bbatsov · 2019-03-09T12:02:28Z

.circleci/config.yml

+      - checkout_code
+      - lint_code:
+          name: Code Linting
+          requires:


What's with all those requires? Isn't there some way to avoid this repetition?

The gist of this is that we check out the code + download deps once, and test n times. Thus each of the tests require the checkout_code step.

Alternatively, we can just run n tests, and have each job do its own checkout etc. I'm not sure either is better than the other given that everything runs fast enough.

Answering the next question: in CircleCI, each you you commit, you trigger a workflow, which is comprised of many jobs. You can set up dependency between the jobs, so they run in order while allowing for parallel jobs etc.

bbatsov · 2019-03-09T12:04:33Z

.circleci/config.yml

+
+workflows:
+  version: 2.1
+  build_and_test:


Not sure what a workflow is exactly in CircleCI, but as we don't really do any explicit "build", I think the name is a bit confusing. I guess the real stages of the build process are "Linting" and "Running tests".

The build_and_test refers to the overall process/workflow (i.e. linting + testing). Happy to take suggestions on the right name.

bbatsov · 2019-03-09T12:06:14Z

.circleci/config.yml

+          requires:
+            - checkout_code
+      - test_code:
+          name: java8, clj1.9 tests


I guess "Test Code" appears somewhere in the output, so for name I'd rather have "Java 8, Clojure 1.9"

danielcompton · 2019-03-10T23:08:42Z

@shen-tian there are lots of small jobs and workflows here. In my experience, most of the time spent running tests is constant factors of pulling images, downloading dependencies, starting up containers, e.t.c. I wonder if you could get away with a smaller number of workflows, each doing more work in a single job?

The pattern I was imagining was to have one job per JDK version, and have each job do all of the work in one go, rather than splitting it up over multiple jobs.

Btw, I'm super impressed with what you've done here, very nice usage of the new CircleCI features, I especially liked the JDK version selection.

danielcompton · 2019-03-10T23:57:43Z

I've made a different branch which has two jobs:

https://circleci.com/gh/danielcompton/workflows/piggieback/tree/use-circle-ci-dc
https://github.com/danielcompton/piggieback/blob/use-circle-ci-dc/.circleci/config.yml

The main benefit of these changes is that the CI config might be a little bit easier to follow. The downside is that the wall-clock time is a bit slower (4 mins instead of 3 mins), and some things are done multiple times unnecessarily, like eastwood and clfmt.

Edit: Generating combinatorial expansion of Lein test profiles brings the test time down to roughly 3-4 minutes for both jobs, although some of my perf improvements would also apply to this PR and could speed it up further.

.circleci/config.yml

shen-tian · 2019-03-11T06:33:12Z

@shen-tian there are lots of small jobs and workflows here. In my experience, most of the time spent running tests is constant factors of pulling images, downloading dependencies, starting up containers, e.t.c. I wonder if you could get away with a smaller number of workflows, each doing more work in a single job?

The pattern I was imagining was to have one job per JDK version, and have each job do all of the work in one go, rather than splitting it up over multiple jobs.

This actually came down less to how to get all the tests to run, but more showing, at a glance, which jobs in the build matrix failed. This is probably less important for piggieback, and more so for the likes on nRepl itself. Even here, we are testing across the combination of JDK versions, Clojure versions, and nRepl versions. It's thus useful to see, e.g. the build broke on JDK11 + Clojure 1.9 and nRepl 0.6, as an example. Given the assumption that's a desirable thing (is it? I'm inferring this from how the Travis build is set up) the maintainers would know), having high granularity at the jobs level would be useful.

Re optimising CircleCI jobs for runtime, I've definitely found, as you point out, that pulling images takes up 20s or more, and penalises otherwise clever looking many-small-job workflows. The current setup is definitely not optimising for run time.

I realise we are testing all nRepl versions via the makefiles, so we are not actually splitting those out into the separate jobs. I've got a separate question about having some of the project complexity spread across project.clj Makefile and config.yml. It feels like more of what's in Makefile can be done in CircleCI than in Travis? And given the similarity between projects in the org, the CircleCI common patterns could be moved into Orbs. Then, each project would mostly declare what needs to be tested, rather than repeat the how part. But that's something that can be dealt with later if we like what we end up with here.

Steps forward: more input on the above welcome? Making any of the changes discussed should be quick/easy enough.

danielcompton · 2019-03-11T06:39:27Z

more showing, at a glance, which jobs in the build matrix failed.

Yep, this is a pretty good argument. I don't feel strongly about this, just wanted to test out an alternative, but it is nice being able to see the specific cases that fail at a glance.

bbatsov · 2019-03-11T07:41:17Z

Yeah, I agree. It's nice to be able to quickly figure out what exactly failed.

I realise we are testing all nRepl versions via the makefiles, so we are not actually splitting those out into the separate jobs. I've got a separate question about having some of the project complexity spread across project.clj Makefile and config.yml. It feels like more of what's in Makefile can be done in CircleCI than in Travis? And given the similarity between projects in the org, the CircleCI common patterns could be moved into Orbs. Then, each project would mostly declare what needs to be tested, rather than repeat the how part. But that's something that can be dealt with later if we like what we end up with here.

I think we've added the makefile mostly for the benefit of end users, who'd run some tasks manually, and for readability (as some commands had pretty long lein commands associated with them).

I don't even know what an orb is so I can't comment on this, but in general I can imagine that most projects would have similar builds.

shen-tian · 2019-03-11T19:00:21Z

Updated this to be in line with nrepl/nrepl#131. Managed to reuse a lot of config, so we are looking good for some Orbs later on.

Updated the badge and removed travis completely.

Might still need the project enabled on CircleCI

bbatsov · 2019-03-11T22:05:54Z

Thanks!

shen-tian added 11 commits February 23, 2019 13:47

First stab at this

42cfa2b

Using custom command

825c648

Adding openjdk-8 tests

0dae97f

Adding openjdk-11 tests

bbf92d1

Parameterised jobs

5d6eb9a

Running tests for all versions of clojure

2080999

Don't wait for lintters to finish before tests. Adding clj master

8e06c65

Using the makefile

e222155

Install make?

bfa460e

Use make for the linters too

d02bf1a

Break jdk x clojure combos into separate jobs

de3ae6d

cichli self-requested a review February 25, 2019 19:18

cichli reviewed Feb 25, 2019

View reviewed changes

.circleci/config.yml Outdated Show resolved Hide resolved

cichli reviewed Feb 25, 2019

View reviewed changes

.circleci/config.yml Outdated Show resolved Hide resolved

cichli reviewed Feb 25, 2019

View reviewed changes

.circleci/config.yml Outdated Show resolved Hide resolved

.circleci/config.yml Outdated Show resolved Hide resolved

Adding cloverage

a468b22

Fixing the make statement

7ba3708

shen-tian force-pushed the circleci branch from 4428620 to 7ba3708 Compare February 27, 2019 06:13

Using executors

38c90bd

cichli approved these changes Feb 27, 2019

View reviewed changes

shen-tian mentioned this pull request Mar 7, 2019

CircleCI nrepl/nrepl#131

Merged

bbatsov reviewed Mar 9, 2019

View reviewed changes

danielcompton reviewed Mar 11, 2019

View reviewed changes

.circleci/config.yml Show resolved Hide resolved

shen-tian added 2 commits March 11, 2019 20:52

Updated based on the nrepl config

ed7eb61

Adding CircleCI badge, remove travis config

cb6e6c8

bbatsov merged commit 123b1b3 into nrepl:master Mar 11, 2019

shen-tian deleted the circleci branch March 11, 2019 22:20

shen-tian mentioned this pull request Mar 12, 2019

Moving CI to CircleCI #101

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Moving build to CircleCI #102

Moving build to CircleCI #102

shen-tian commented Feb 25, 2019 •

edited

Loading

cichli commented Feb 25, 2019

cichli left a comment

shen-tian commented Feb 26, 2019

codecov-io commented Feb 26, 2019 •

edited

Loading

shen-tian commented Feb 26, 2019

plexus commented Feb 26, 2019

shen-tian commented Feb 26, 2019

shen-tian commented Feb 27, 2019

cichli left a comment

cichli commented Feb 27, 2019

bbatsov commented Mar 9, 2019

bbatsov Mar 9, 2019

shen-tian Mar 10, 2019

bbatsov Mar 9, 2019

shen-tian Mar 10, 2019

bbatsov Mar 9, 2019

shen-tian Mar 10, 2019

danielcompton commented Mar 10, 2019 •

edited

Loading

danielcompton commented Mar 10, 2019 •

edited

Loading

shen-tian commented Mar 11, 2019 •

edited

Loading

danielcompton commented Mar 11, 2019

bbatsov commented Mar 11, 2019 •

edited

Loading

shen-tian commented Mar 11, 2019

bbatsov commented Mar 11, 2019

Moving build to CircleCI #102

Moving build to CircleCI #102

Conversation

shen-tian commented Feb 25, 2019 • edited Loading

cichli commented Feb 25, 2019

cichli left a comment

Choose a reason for hiding this comment

shen-tian commented Feb 26, 2019

codecov-io commented Feb 26, 2019 • edited Loading

Codecov Report

shen-tian commented Feb 26, 2019

plexus commented Feb 26, 2019

shen-tian commented Feb 26, 2019

shen-tian commented Feb 27, 2019

cichli left a comment

Choose a reason for hiding this comment

cichli commented Feb 27, 2019

bbatsov commented Mar 9, 2019

bbatsov Mar 9, 2019

Choose a reason for hiding this comment

shen-tian Mar 10, 2019

Choose a reason for hiding this comment

bbatsov Mar 9, 2019

Choose a reason for hiding this comment

shen-tian Mar 10, 2019

Choose a reason for hiding this comment

bbatsov Mar 9, 2019

Choose a reason for hiding this comment

shen-tian Mar 10, 2019

Choose a reason for hiding this comment

danielcompton commented Mar 10, 2019 • edited Loading

danielcompton commented Mar 10, 2019 • edited Loading

shen-tian commented Mar 11, 2019 • edited Loading

danielcompton commented Mar 11, 2019

bbatsov commented Mar 11, 2019 • edited Loading

shen-tian commented Mar 11, 2019

bbatsov commented Mar 11, 2019

shen-tian commented Feb 25, 2019 •

edited

Loading

codecov-io commented Feb 26, 2019 •

edited

Loading

danielcompton commented Mar 10, 2019 •

edited

Loading

danielcompton commented Mar 10, 2019 •

edited

Loading

shen-tian commented Mar 11, 2019 •

edited

Loading

bbatsov commented Mar 11, 2019 •

edited

Loading