GitHub CI integration for macos #3117
Conversation
.github/workflows/test.yml Outdated
| version: ${{matrix.emacs_version}} | ||
| | ||
| - name: Install Eldev | ||
| run: curl -fsSL https://raw.github.com/doublep/eldev/master/webinstall/github-eldev | sh |
There was a problem hiding this comment.
I'd suggest using an immutable link here, reproducibility matters in CI
There was a problem hiding this comment.
This is the same setup we use in CircleCI for the other architectures. I understand the concerns, this is the recommended Eldev way. On the positive side, we get constant test improvements.
There was a problem hiding this comment.
Note that one can use tagged uris e.g. https://raw.githubusercontent.com/doublep/eldev/0.10.3/webinstall/github-eldev
I think we should not be exposed to arbitrary breaking changes. It really is counterproductive to have releases botched on the last minute because CI started failing for reasons outside of our control.
It happens.
I understand the concerns, this is the recommended Eldev way.
This is an overstatement tbh, a specific instruction doesn't mean they have considered other alternatives, or that those alternatives are actively discouraged.
There was a problem hiding this comment.
Fixed Eldev version to 0.10.3 as requested, all tests pass as expected.
.github/workflows/test.yml Outdated
| strategy: | ||
| matrix: | ||
| os: [macos-latest] | ||
| emacs_version: ['27.2'] |
There was a problem hiding this comment.
Seems a good idea to exercise the same versions we do on Circle
There was a problem hiding this comment.
I'd rather not waste resources on testing anything other the previous version on macos. Functionality wise, we get coverage of the older versions with the Gnu/Linux builds.
We also only test the latest version with MS-Widnows on CircleCI.
There was a problem hiding this comment.
This seems to couple things - reasoning about one build shoudn't depend on another build.
And also you say:
GitHub currently appears to be quite generous with resources, which I find it absurd in the positive way. I never hit a barrier yet and have been running some serious jobs in the past.
There was a problem hiding this comment.
This seems to couple things - reasoning about one build shoudn't depend on another build.
I agree.
And also you say:
GitHub currently appears to be quite generous with resources, which I find it absurd in the positive way. I never hit a barrier yet and have been running some serious jobs in the past.
... true for Linux and Window boxes that I have most experience with (the intention of the above was meant to be a statement showing the benefits of GitHub over CircleCI btw). I've also noticed that for macos they might reclaim the box while it is running. I think they macos resources are not as abundant as with the other platforms.
Ideally yes, we should be testing the whole version matrix across all architectures, but I don't fill comfortable doing so for macos (or anything else other than Linux, i.e. having at least some very good coverage about the other versions).
Would you like me to proceed with configuring the other versions for macos as well?
There was a problem hiding this comment.
Different macos versions don't matter as much because most likely we dont have business logic that depends on macOS intricacies.
But Emacs different versions certainly matter - just today we caught something:
https://github.com/clojure-emacs/enrich-classpath uses an extensive GH Actions matrix and its size has never been a problem per se:
There was a problem hiding this comment.
Different macos versions don't matter as much because most likely we dont have business logic that depends on macOS intricacies.
But Emacs different versions certainly matter - just today we caught something:
I agree.
https://github.com/clojure-emacs/enrich-classpath uses an extensive GH Actions matrix and its size has never been a problem per se:
This is a good demonstration we can have multiple macos jobs setup running, though I would add it is also sensitive on the relative commit frequency. I've added 26.3 to to the list alongside 27.2. I suppose the intention here is to include all Emacs versions supported by Emacs? Is there another version you would like me to add? Thanks,
26.3 on macos failed as expected similarly to the Linux build:
... [00:14.805] Debugger entered--Lisp error: (void-function package-get-version) ... There was a problem hiding this comment.
Is there another version you would like me to add?
I think that being on par with Circle suffice. Perhaps we can add comments in both config files, to remind us keeping the values in sync.
26.3 on macos failed as expected similarly to the Linux build:
Perfect, thanks!
| From a promotional email, Circle recently said macOS for the Free tier was "coming soon". Let's see when that actually happens, no issue in the meantime |
forked and deliberately forced a test to fail, macos run log showing the failure https://pipelines.actions.githubusercontent.com/gHVz7qA3HTDgSJiU6GmuUtGcUCzIGO2SADtrOJAVfQAYQZJJpD/_apis/pipelines/1/runs/2/signedlogcontent/2?urlExpires=2021-12-28T19%3A33%3A03.5638764Z&urlSigningMethod=HMACV1&urlSignature=%2BsRO2QQhtNG%2FnRO8c3G%2FZx0%2B0kRG5TqeBoEwZTJ260U%3D also the macos job page https://github.com/ikappaki/cider/runs/4652948439?check_suite_focus=true |
Noticed in the past |
ikappaki force-pushed the feature/ci-macos branch from 89cc3ee to 1a6d8e2 Compare 
| You merged this before I could comment, but I really think that if we go in this direction we should move everything to GitHub Actions, as the CI becomes a bit messy right now. One small example - how many CI badges do we need to put in the README? :-) I certainly don't want to deal with both GitHub Actions and CircleCI indefinitely. Also some general notes:
|
Probably I tend to agree but who knows which crazy problem might be caught sometime.
Speaking of them, apparently macOS is coming to free Circle soon. That would simplify badges/proliferation. Assuming Win and Mac builds are not too different in terms of credits, macOS would keep working as long as Win builds keep working? |
| A related topic is that currently CI builds don't even matter much, we often see red stuff on master. Perhaps we can refine that such that master is never red? Btw finally I'm appreciating the frequent commits on master to be melpa-snapshotted, they certainly yields useful user feedback. But if the build was red to begin with, then we're just using people for doing CI's job 😃 So as a middle ground perhaps we could have a |
Perhaps, I've just rarely seen projects test on macOS specifically unless they are some macOS apps. I guess part of the reason is that it's usually expensive to do so, but I assume another part is that macOS specific issues are quite unlikely. On the other hand, each extra build means more time waiting for the builds to pass.
Yeah, I guess so.
This has been proposed a few times in the past, but the real problem is that the test suite is pretty basic and rarely finds meaningful problems. (e.g. it didn't catch the problem with |
| Btw, even now I'm getting different results when running the tests locally and on the CI... Two tests fail locally (probably because I'm on Emacs 29), but the test that fails on Emacs 27 passes for me. And the byte-compilation error about the unused return value from |
3 participants
Hi,
could you please consider adding a new GitHub workflow to run the cider tests on the macos platform, as to have wider test coverage, as described in #3116.
It does not appear to be possible to have a free macos executor after having a quick look at this page https://circleci.com/docs/2.0/hello-world-macos/ :
thus the switch for this particular platform to GitHub worfklow, which is free.
All tests pass on macos-latest on Emacs 27.2.
I understand this fragments the ci admin files a little, but it adds quite a lot of value.
Before submitting the PR make sure the following things have been done (and denote this
by checking the relevant checkboxes):
eldev test)eldev lint) which is based onelisp-lintand includescheckdoc, check-declare, packaging metadata, indentation, and trailing whitespace checks.Thanks!