Skip to content

Switch to CircleCI #2603

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

Merged
merged 2 commits into from
Mar 10, 2019
Merged

Conversation

danielcompton
Copy link
Contributor

@danielcompton danielcompton commented Mar 5, 2019

Opening early for feedback. One thing that is tricky is that CircleCI doesn't have the concept of allowable failures like Travis does. At the moment Emacs master is failing on cider-jack-in-clj&cljs (timing out?), but 25 and 26 are running successfully: https://circleci.com/workflow-run/dff69ef0-ec14-4c24-8f19-457e4168af37.

Still needed:

  • Update hacking_on_cider doc to show how to do the same thing with CircleCI
  • Remove/convert the Travis Docker scripts to CircleCI scripts

Before submitting the PR make sure the following things have been done (and denote this
by checking the relevant checkboxes):

  • The commits are consistent with our contribution guidelines
  • You've added tests (if possible) to cover your change(s)
  • All tests are passing (make test)
  • All code passes the linter (make lint) which is based on elisp-lint and includes
  • You've updated the changelog (if adding/changing user-visible functionality)
  • You've updated the user manual (if adding/changing user-visible functionality)

See also clojure-emacs/clomacs#16, clojure-emacs/clojure-mode#514, and clojure-emacs/inf-clojure#159.

# Default actions to perform on each Emacs version
default: &default-steps
steps:
- checkout
Copy link
Member

Choose a reason for hiding this comment

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

Do we need Lein at all?

Copy link
Contributor Author

@danielcompton danielcompton Mar 7, 2019

Choose a reason for hiding this comment

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

cider-jack-in-clj&cljs requires lein to be around. See the test failure at https://circleci.com/gh/danielcompton/cider/11 when removing Leiningen from the build.

Copy link
Member

Choose a reason for hiding this comment

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

I'll take a look at the test. We don't do any real jack-ins anywhere in the tests, so probably something just needs to be stubbed.

- run: apt-get update && apt-get install make leiningen=2.8.1-6 -y
- run: make elpa
- run: emacs --version
- run: make test
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should have separate steps for the testing and linting, so that we would get nicer reports at the end?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The issue with this is that the majority of CI time is spent getting the image booted and into the right state to run the tests. The lint and test phases only take 6s and 3s. Given this is a free service, I wanted to try and minimise resource usage where practicable. If we wanted better errors from CircleCI we could look at producing JUnit XML reports which they can render.

Copy link
Member

Choose a reason for hiding this comment

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

Fair enough. My only issue is that the linting produces a lot of (hard to read) output and this makes it hard to find the tests at least for me.

@danielcompton
Copy link
Contributor Author

danielcompton commented Mar 7, 2019

I've migrated the docs for running locally, I think this is now ready to be reviewed. Still not quite sure what to do about failures from emacs-master though. There is an open feature request for this, but not sure when it will be addressed.

Replacing the docs for how to run Travis CI tests locally.
@bbatsov
Copy link
Member

bbatsov commented Mar 9, 2019

Well, perhaps we can just remove this build or fix the failure there? I don't see any other options. :-)

@bbatsov
Copy link
Member

bbatsov commented Mar 9, 2019

Actually checking the report you've linked it seems to me that output is not bad even now. I'm still a bit confused when navigating Circle's UI, as I was pretty used to Travis's.

I'll see what I can do about that failing test. Weirdly I don't see anything in the report and I also don't see anything special in the test.

@bbatsov bbatsov merged commit 9a6438d into clojure-emacs:master Mar 10, 2019
@bbatsov
Copy link
Member

bbatsov commented Mar 10, 2019

Thanks for tackling this, @danielcompton! Much appreciated!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants