I’ve been able to isolate attempts to write non-supported values into the Transit analysis cache as being related to :parallel-build true, at least for me (I'm hoping this will lead to a minimal repro so we can arrive at a definitive explanation): https://gist.github.com/mfikes/e3313ec059b604674f60d326e701a7f1

@mfikes interesting, haven’t seen that before

@dnolen Emperically, if I place a course grained mutex around the transit write call, it works fine. A theory is that transit has a thread-safety bug, perhaps around the WriteHandlerMap stuff down in transit-java. I’ve locally upgraded the compiler to the latest transit-clj to check and the defect still occurs.

(The WriteHandlerMap is mutative.)

@mfikes huh, but we construct a new writer/reader every time

@dnolen Good point. I added a a tiny bit of logic to try to capture the number of concurrent threads writing to see if it is correlated with the failures (essentially (util/debug-prn 'starting (swap! write-concurrency inc))), and it every time it occurs, the there are 3 writers in flight.

@mfikes are you saying they share handlers?

@dnolen No… just a desperate and quick attempt to try to find a root cause. Not good analysis.

@dnolen All, I know is: 1) It fails only if parallel builds enabled 2) When so, it fails, maybe 30 to 50% of the time 3) If I put a coarse-grained mutex around the write calls, it succeeds 4) It appears to occur under heave write load (the 3 above)

@mfikes ok so patch for 3) is acceptable to me - would like to get that in this coming release if you have it

OK… will do 🙂

When testing the patch for http://dev.clojure.org/jira/browse/CLJS-1759 I encountered a self-host error which I'm assuming is unrelated:

ERROR in (test-cljs-1757) (TypeError:NaN:NaN)
expected: (s/exercise-fn (quote cljs.spec-test/cljs-1757-x))
  actual: #object[TypeError TypeError: Cannot read property 'call' of undefined]

Confirmed the above is unrelated; we’re good to go. Captured it as http://dev.clojure.org/jira/browse/CLJS-1760

@mfikes working on fixing CLJS-1760

not a valid excuse, but I assumed self-parity tests were passing and didn’t bother to run them because CLJS-1756 wasn’t there yet 🙂

Cool. The cljs.spec$macros/gen warning is really odd if it is referring to the gen symbol on this line: https://github.com/clojure/clojurescript/blob/master/src/main/cljs/cljs/spec.cljc#L77

@anmonteiro For what it’s worth, I quickly tried with http://dev.clojure.org/jira/browse/TCHECK-105 and that didn’t fix anything

@mfikes it seems that my patch didn’t introduce the bug, it just happens that exercise-fn had no tests before 🙂

Cool

attached patch to CLJS-1760

probably something you missed when you added a patch to qualify symbols & keywords

@anmonteiro You are right; you can see gen sitting there right at the bottom of https://github.com/clojure/clojurescript/commit/20b08fe442a24c46526c812f6c52ad8c7d3f6c9b

I’ve got a docker container running Clojurescript tests on Shippable. Some of the tests are failing for some reason. But atleast its a start. My Dockerfile and the config file for Shippable are here: https://github.com/ducky427/clojurescript

@rohit if you apply CLJS-1756 (or rebase on current master) failures should go away

oh cool. let me do that.

@anmonteiro: it looks much better now though some tests are still failing.

FAIL in (test-reader) (at builds/out-adv/core-advanced-test.js:1106:263)
Character Literals
expected: (= ["\t" "\r" "\n" " " "\b" "\f" "?"] (reader/read-string "[\\tab \\return \\newline \\space \\backspace \\formfeed \\u1234]"))
  actual: (not (= ["\t" "\r" "\n" " " "\b" "\f" "?"] ["\t" "\r" "\n" " " "\b" "\f" "ሴ"]))

some charset related issue I am guessing.

Here’s another one:

FAIL in (test-819) (at builds/out-adv/core-advanced-test.js:1106:263)
Testing reading, CLJS-819
expected: (= m " ?")
  actual: (not (= nil " ?"))

I’ll investigate these.

@rohit btw there’s 1 more test to run

lein test is also a thing

@anmonteiro: oh cool. I didn’t realise that. I need to install lein in that image in that case. cheers!

There is also script/test-simple. (But those may not be passing right now.)

I’ve added this to the list of tests as well. Its a good starting point.

@rohit the self parity test has just been fixed on master 🙂

so I suppose only encoding issues remain

@anmonteiro: thats awesome! 🙂

Looks like script/test-simple is passing. 🙂

@mfikes: neat. just added lein and triggered a new build.

@mfikes probably worth tweaking script/test to compile with :parallel-build enabled

re: your issue with transit & parallel build

maybe we could spot regressions that way

Right… otherwise we really don’t have anything testing :parallel-build I’d guess

@rohit hah! we can see logs here https://app.shippable.com/runs/57b72c6a1419e20f001a11b5/1/console

Thats awesome!

thought it might be private

lein test - all a-ok!

I am guessing public logs for public repos.

right

Update: thanks to @anmonteiro all tests are now passing!

ClojureScript 1.9.225 released https://groups.google.com/d/msg/clojurescript/MF8QfYkIDOs/HsIu7iKEDQAJ

Thanks everyone!

I wonder if there is some way to do something akin to https://github.com/clojure/clojurescript/releases/latest that could avoid having to PR like https://github.com/clojure/clojurescript-site/pull/22 for releases. (The front page of http://clojurescript.org seems to have a dynamically-updated link.)

@favila With respect to CLJS-1761 I was reproducing it via stuff in https://clojurians.slack.com/archives/cljs-dev/p1471581041001784

@favila Unfortunately, I failed to create a minimal repro 😞

@mfikes If my hunch is correct, then if transit-write-opts and transit-read-opts :handlers map are wrapped in transit-clj/write-handler-map call, issue will be solved without locking

@mfikes the http://clojurescript.org page is doing git describe --abbrev=0 --tags | sed "s/^r//“ during the build

read-handler-map and write-handler-map pre-merge the default handlers and bypass all the handler caching

(where the races are)

@mfikes I tried to make that something that could make a variable for the quick start page but that turned out to be annoyingly difficult. could be done with just some hacking on the files though. If you could file an issue on clojurescript-site I’ll get to it

Thanks @alexmiller https://github.com/clojure/clojurescript-site/issues/23

@favila: cool. Since I can easily repro, willing to try things. Will see if I can test your idea. :)

@mfikes thx