Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

CI test fixes #580

Merged
merged 9 commits into from Dec 28, 2018
Merged

CI test fixes #580

merged 9 commits into from Dec 28, 2018

Conversation

plexus
Copy link
Contributor

@plexus plexus commented Dec 28, 2018

This branch contains the "real" fixes that came out of #569. It also adds Clojure 1.10 to the build matrix.

See individual commits for exact changes. Unless #578 gets in the way this should go green now 馃

Now that Clojure 1.10 is out, add an extra Leiningen profile to test with, and
switch the the `:master` profile to use the `1.11.0-master-SNAPSHOT` version.

Run CI on Clojure 1.8/1.9/1.10/master and oraclejdk8/openjdk11.
ClojureScript prior to 1.10 relied on the javax.xml.bind API. Since JDK 9 this
API is part of a Jigsaw module that needs to be included when booting the JVM,
from JDK 11 this API is no longer bundled with the JDK.

To prevent users from running into this issue, we always include the JAXB API as
a separate dependency. This can be dropped once we drop support for
ClojureScript < 1.10.
The `with-cljs-env` macro tried to grab the ClojureScript compiler env during
macroexpansion, at which point it may not be set yet. This was causing issues
where the ClojureScript compiler found an `*env*` of `nil`, causing a
NullPointerException, which was then getting swallowed by the test code.

Instead, grab the env at runtime.
ClojureScript now includes a var named "private-var", which was interfering with
the test. Instead use "my-private-var" for uniqueness.
@bbatsov
Copy link
Member

bbatsov commented Dec 28, 2018

See individual commits for exact changes. Unless #578 gets in the way this should go green now 馃

It seems indentation problems got in the way. :-)

@bbatsov
Copy link
Member

bbatsov commented Dec 28, 2018

All the changes look good to me.

Clojure 1.10 saw changes in how certain errors are reported, in particular
reader/compiler related errors that happen at the REPL. Since CIDER is based on
a REPL interface, this changes the errors that CIDER sees.

Split the tests in separate 1.9 / 1.10 branches where appropriate.
Because of the more complex Reflector in Clojure 1.10, there are extra
stacktraces of the `invoke` and `invokeStatic` methods. These do not have an
associated file. Update the stacktrace-test accordingly.
Maybe it's me but I had a really hard time parsing `(let)` inside `(->>)`.
This gets rid of a warning in the latest Leiningen.
@plexus
Copy link
Contributor Author

plexus commented Dec 28, 2018

Ah yes, let's try that again. Bozhidar I also added back the :implicits, seems it's actually mranderson which is triggering the warning.

@plexus
Copy link
Contributor Author

plexus commented Dec 28, 2018

So close! But still one failure I hadn't seen before in the debug test.

https://travis-ci.org/clojure-emacs/cider-nrepl/jobs/473008395

This is on Oracle jdk 8 and Clojure 1.9 only.

@bbatsov bbatsov merged commit bbcb8e5 into master Dec 28, 2018
@bbatsov bbatsov deleted the ci-test-fixes branch December 28, 2018 16:30
@bbatsov
Copy link
Member

bbatsov commented Dec 28, 2018

So close! But still one failure I hadn't seen before in the debug test.

I think there's often been flakiness in this test - I've definitely seen this failure before. I've long suspected it's some race condition.

I'm merging the PR regardless, as it gets us extremely close to the coveted destination and to the release of 0.19.

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.

None yet

2 participants