test.check

Self-host tests not runnable

Details

  • Type: Defect Defect
  • Status: Resolved Resolved
  • Priority: Minor Minor
  • Resolution: Completed
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
  • Patch:
    Code and Test

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.)

  1. TCHECK-137.patch
    06/Dec/17 9:08 PM
    3 kB
    Mike Fikes
  2. TCHECK-137-2.patch
    07/Dec/17 12:54 PM
    4 kB
    Mike Fikes
  3. TCHECK-137-2b.patch
    08/Dec/17 2:25 PM
    4 kB
    Mike Fikes

Activity

Hide
Mike Fikes added a comment -

The attached gets the self-host tests running again.

All tests pass apart from 5 in self-host mode:
tcheck-116-debug-prn-should-be-optional
can-report-trials-with-dots
can-report-trials-periodically
can-report-shrinking
can-report-shrunk

Show
Mike Fikes added a comment - The attached gets the self-host tests running again. All tests pass apart from 5 in self-host mode: tcheck-116-debug-prn-should-be-optional can-report-trials-with-dots can-report-trials-periodically can-report-shrinking can-report-shrunk
Hide
Gary Fredericks added a comment -

I applied the patch. I suppose I'll leave the ticket open since the tests aren't all passing yet?

I also noticed that they fail, with or without the patch, using java 9. It complains about javax.xml.bind.DatatypeConverter not existing, via cljs/util.cljc:1:1.

Show
Gary Fredericks added a comment - I applied the patch. I suppose I'll leave the ticket open since the tests aren't all passing yet? I also noticed that they fail, with or without the patch, using java 9. It complains about javax.xml.bind.DatatypeConverter not existing, via cljs/util.cljc:1:1.
Hide
Mike Fikes added a comment -

Thanks Gary! I'll see if I can produce a follow-up patch for the other failing tests.

The Java 9 issue is fixed in ClojureScript master (CLJS-2377).

Show
Mike Fikes added a comment - Thanks Gary! I'll see if I can produce a follow-up patch for the other failing tests. The Java 9 issue is fixed in ClojureScript master (CLJS-2377).
Hide
Mike Fikes added a comment -

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

Show
Mike Fikes added a comment - 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
Hide
Gary Fredericks added a comment -

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?

Show
Gary Fredericks added a comment - 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?
Hide
Gary Fredericks added a comment -

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

Compiling ClojureScript...
Compiling "target/cljs/node_dev/tests.js" from ["src/main/clojure" "src/test/clojure" "src/target/cljs/node"]...
WARNING: Use of undeclared Var clojure.test.check.clojure-test/*err* at line 171 /home/gary/dev/oss/test.check/src/main/clojure/clojure/test/check/clojure_test.cljc
WARNING: No such namespace: cljs.test$macros, could not locate cljs/test$macros.cljs, cljs/test$macros.cljc, or JavaScript source providing "" at line 14 src/main/clojure/clojure/test/check/clojure_test/assertions/cljs.cljc
WARNING: Use of undeclared Var cljs.test$macros/assert-expr at line 14 src/main/clojure/clojure/test/check/clojure_test/assertions/cljs.cljc
/home/gary/dev/oss/test.check/target/cljs/node_dev/out/cljs/core.js:35956
return ns_obj.name;
             ^

TypeError: Cannot read property 'name' of null
    at Object.cljs$core$ns_name [as ns_name] (/home/gary/dev/oss/test.check/target/cljs/node_dev/out/cljs/core.js:35956:14)
    at Object.<anonymous> (/home/gary/dev/oss/test.check/target/cljs/node_dev/out/clojure/test/check/clojure_test.js:308:77)
    at Module._compile (module.js:570:32)
    at Object.Module._extensions..js (module.js:579:10)
    at Module.load (module.js:487:32)
    at tryModuleLoad (module.js:446:12)
    at Function.Module._load (module.js:438:3)
    at Module.require (module.js:497:17)
    at require (internal/module.js:20:19)
    at global.CLOSURE_IMPORT_SCRIPT (/home/gary/dev/oss/test.check/target/cljs/node_dev/out/goog/bootstrap/nodejs.js:75:5)
Successfully compiled "target/cljs/node_dev/tests.js" in 7.372 seconds.
Show
Gary Fredericks added a comment - I'm seeing the following output from lein do clean, cljsbuild once node-dev:
Compiling ClojureScript...
Compiling "target/cljs/node_dev/tests.js" from ["src/main/clojure" "src/test/clojure" "src/target/cljs/node"]...
WARNING: Use of undeclared Var clojure.test.check.clojure-test/*err* at line 171 /home/gary/dev/oss/test.check/src/main/clojure/clojure/test/check/clojure_test.cljc
WARNING: No such namespace: cljs.test$macros, could not locate cljs/test$macros.cljs, cljs/test$macros.cljc, or JavaScript source providing "" at line 14 src/main/clojure/clojure/test/check/clojure_test/assertions/cljs.cljc
WARNING: Use of undeclared Var cljs.test$macros/assert-expr at line 14 src/main/clojure/clojure/test/check/clojure_test/assertions/cljs.cljc
/home/gary/dev/oss/test.check/target/cljs/node_dev/out/cljs/core.js:35956
return ns_obj.name;
             ^

TypeError: Cannot read property 'name' of null
    at Object.cljs$core$ns_name [as ns_name] (/home/gary/dev/oss/test.check/target/cljs/node_dev/out/cljs/core.js:35956:14)
    at Object.<anonymous> (/home/gary/dev/oss/test.check/target/cljs/node_dev/out/clojure/test/check/clojure_test.js:308:77)
    at Module._compile (module.js:570:32)
    at Object.Module._extensions..js (module.js:579:10)
    at Module.load (module.js:487:32)
    at tryModuleLoad (module.js:446:12)
    at Function.Module._load (module.js:438:3)
    at Module.require (module.js:497:17)
    at require (internal/module.js:20:19)
    at global.CLOSURE_IMPORT_SCRIPT (/home/gary/dev/oss/test.check/target/cljs/node_dev/out/goog/bootstrap/nodejs.js:75:5)
Successfully compiled "target/cljs/node_dev/tests.js" in 7.372 seconds.
Hide
Mike Fikes added a comment -

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.

Show
Mike Fikes added a comment - 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.
Hide
Gary Fredericks added a comment -

Applied on master; thanks!

Show
Gary Fredericks added a comment - Applied on master; thanks!

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: