Self-host tests not runnable

Description

If you run script/test-self-host, you will get an error rooted in "No such namespace: goog.json.HybridJsonProcessor".

This is an easy fix, related to churn in Google Closure (the same was revised in ClojureScript here https://github.com/clojure/clojurescript/commit/92433701acc6f86665b81349dc8c9eb4048ec464#diff-f3636bf6a57757156bc27de61a49fe31L91)

Fixing that, you will see there are other things to address for self-hosting for the defmethod for cljs.test/assert-expr 'clojure.test.check.clojure-test/check? in the (clojure.test.check.clojure-test.assertions and clojure.test.check.clojure-test.assertions.cljs namespaces).

If these can be fixed, we can get the self host tests to be runnable again. (They may not yet all pass, but they can then be executed.)

Environment

None

Activity

Show:
Mike Fikes
December 7, 2017, 6:54 PM

Hey Gary, the attached TCHECK-137-2.patch fixes the remaining failing unit tests.

The root cause was that the defmethod calls at the bottom of clojure.test.check.clojure-test were being run twice in self-hosted ClojureScript (once when the namespace was being compiled as a runtime namespace and once when being compiled as a macros namespace). A consequence of the resulting collision is that the "last one wins" (which was evidently the macros version) and this would cause dynamic Vars like report-trials to resolve to the copies of the Vars that end up in the macros namespace. (In other words, you could make the unit tests pass by qualifying them like clojure.test.check.clojure-test$macros/report-trials.)

The solution is to ensure that these defmethod calls are only run when being compiled as a runtime namespace by guarding them with an enclosing when, where the test detects the right time. It employs essentially the same test used by Christophe Grand's macrovich library https://github.com/cgrand/macrovich/blob/fe478682cab3b69496769f1425a45e036a0318a3/src/net/cgrand/macrovich.cljc#L20

gfredericks
December 8, 2017, 12:16 AM

So the test seems brittle in that it's depending on what I assume is an implementation detail of clojurescript, but my analysis is that if the detail it depends on ever changed such that that expression started returning false, the only impact would be the regression of the bug you just fixed, and traditional clojurescript uses would be unaffected; does that sound correct?

gfredericks
December 8, 2017, 12:31 AM

I'm seeing the following output from lein do clean, cljsbuild once node-dev:

Mike Fikes
December 8, 2017, 8:25 PM

Hi Gary,

Yes the test is brittle in that it relies on an internal detail where ClojureScript currently implements macros namespaces by suffixing with $macros. To have this not ever affect regular JVM ClojureScript users, we want it to lean towards always returning true (and only returning false in the special circumstance that it detects it is being compiled in a self-hosted macros namespace).

Given the inherent brittleness, I didn't want to try to innovate, and just copied what Christophe has had out in the wild for a while. But, it looks like you immediately found another way for it to break regular JVM ClojureScript users: If it throws.

The attached TCHECK-137-2b.patch puts one additional guard in place to try to prevent it from derailing if ns is not set.

gfredericks
December 9, 2017, 12:40 AM

Applied on master; thanks!

Completed

Assignee

gfredericks

Reporter

Mike Fikes

Labels

None

Approval

None

Patch

Code and Test

Priority

Minor