Clojurians Log v2

@slipset that’s useful if you want to do something like this: https://clojurians.slack.com/archives/CDJGJ3QVA/p1540070302000100

Maybe we should make a decision that we’re focusing only on :args specs to be correct so errors can be caught during dev and not :ret and :fn? In that case you don’t need the :binary, :trinary, but if we think this might be useful to core one day, then it’s probably better to write full specs.

Ahh, interesting, I guess you could make the :val field optional in this regex: (s/cat :f fn? :val any? :coll ::reduceable-coll)

Maybe that spec could be

(s/cat :f fn? :val (s/? any?) :coll ::reduceable-coll)

I'll put together a PR with that simplification.

I fixed the tests in clj + cljs in this branch: https://github.com/slipset/speculative/pull/18

and got rid of some boilerplate using macrovich. It turned out the with-instrumentation macro was useful after all, because (s/instrument) doesn’t work properly in JS it seems

so the JS tests didn’t seem to do anything when the specs were offended 😉

By the way, I nearly have CI sorted for ClojureScript and self-hosted ClojureScript (with Planck). I'm still fighting with an issue where the CI ubuntu box is different than the stock 18.10, thus causing a shared library issue with PPA builds.

Hmm, (merge-with +) is just stupid isn’t it?

What’s the purpose of that? I agree that speculative should allow for it, since clojure.core does, but it still doesn’t make sense.

Another fun thing with speculative is that it creates a tool for generatively testing clojure.core functions.

I was just thinking about wether speculative should include generative tests for clojure.core or if that is another lib.

I would imagine it’s another lib.

That’s what I was getting at with https://clojurians.slack.com/archives/CDJGJ3QVA/p1540111252000100. We either make specs for error assertions, or for testing core. Maybe the first is something people would like, the second is more for the core team to use.

Testing core is not something Joe the clojure developer does on a daily basis 😉

I’d be aming for error assertions, but that doesn’t stop us from using speculative to exercise the functions in core.

About the PR: it works on self hosted, normal cljs and clj now.

Nice. merging

@slipset yeah, but then I’m all for having separate cases for arities, so you can leverage that inside :fn. E.g. map should only return a transducer when called with one arg, in all other cases, it’s a sequential.

so when map would return a transducer when called with two args, it’s an error. now it would just be ok

gotta go

Thanks for the help! This has been a lot of fun!

Got a challenge for you 🙂

https://github.com/slipset/speculative/tree/spec-for-division

I must be doing something wrong here since this spec fails for (/ 1 0)

One issue @slipset is that it is legal to divide by zero in ClojureScript.

In fact, it is legal to divide by negative zero as well. 😎

(/ (/ ##-Inf))

Oh, I see you are currently focusing on the Clojure test... in that case you might need to use

(apply / [1 0])

(I found you need to do the same for the count tests.)

And with ClojureScript some things are macro-functions.

Aha!

But that would mean that a Spec for / wouldnt have much value?

For ClojureScript, we could spec the macro and the function. Perhaps if we are clever we could avoid duplication.

Cleaned up the test utils into one .cljc file now

Thanks for merging. @mfikes I’m not sure why, but to make plk work with the macros in test-utils, I had to add this: https://github.com/slipset/speculative/blob/master/test/speculative/core_test.cljc#L9 although it worked without in “normal” cljs.

That seems to match the sample here, but I'm not familiar with the subtleties https://github.com/cgrand/macrovich#sample

yeah, that sample doesn’t use the macro in a different namespace though. but it works. I think it’s because I use the macros/case which is needed at runtime in plk because of self hosting

@mfikes: the direct linking stuff (which is an area of great ignorance for me), if the test needs to be written as (apply / [1 0]), would a spec for / have any value for Joe Enduser?

maybe if I would fully qualify it in the macro, then it would work without the require

yup, that works

@slipset hope you’re not getting tired of the PRs 😉

I’m loving it 🙂

Going to try speculative on a big project I’m working on…. hope it doesn’t explode

ah it did 😉

hmm, I think it didn’t pick up the latest snapshot, installing locally

exploded with:

clojure.lang.ExceptionInfo: Call to #'clojure.core/merge did not conform to spec.
           clojure.spec.alpha/args: nil
        clojure.spec.alpha/failure: :instrument
             clojure.spec.alpha/fn: clojure.core/merge
       clojure.spec.alpha/problems: [{:path [], :pred clojure.core/coll?, :val nil, :via [], :in []}]
           clojure.spec.alpha/spec: #object[clojure.spec.alpha$every_impl$reify__2254 0x74f0ba3e "clojure.spec.alpha$every_impl$reify__2254@74f0ba3e"]
          clojure.spec.alpha/value: nil
    clojure.spec.test.alpha/caller: {:file "util.clj", :line 238, :var-scope yada.util/eval47062}
clojure.lang.ExceptionInfo: Call to #'clojure.core/merge did not conform to spec.
           clojure.spec.alpha/args: nil
        clojure.spec.alpha/failure: :instrument
             clojure.spec.alpha/fn: clojure.core/merge
       clojure.spec.alpha/problems: [{:path [], :pred clojure.core/coll?, :val nil, :via [], :in []}]
           clojure.spec.alpha/spec: #object[clojure.spec.alpha$every_impl$reify__2254 0x74f0ba3e "clojure.spec.alpha$every_impl$reify__2254@74f0ba3e"]
          clojure.spec.alpha/value: nil
    clojure.spec.test.alpha/caller: {:file "util.clj", :line 238, :var-scope yada.util/eval47062}
                              line: 238

going to look at this later, cooking right now

ah, it fails on (merge) which is valid in clojure

https://github.com/juxt/yada/blob/master/src/yada/util.clj#L238

also map? doesn’t seem right to me, it should be associative probably

Right. We could copy the bit from merge-with

although it’s a bit non-sensical: (merge [0 1 3] []) works

associative? would allow vectors. But perhaps records are ok?

But, reading the yada code. Does that actually make sense? To have a call to (merge) just like that?

although this errors: (merge {0 :a} [[0 :b]]). Not sure what is the right answer. cooking again

It happens in the wild and it doesn’t error, so we should allow it

I guess

True, but I guess a pr against yada would make sense 🙂

😄

we could try to run our specs against some public code bases. maybe there’s 4clojure answers or something?

You could be applying merge to a list of maps which happens to be empty

yes.

That’s actually a valid point. I’ve seen a Clojure ticket on that topic.

Some fns allow this, while others dont.

yes, I was just thinking the same, (apply merge nil)

https://github.com/juxt/yada/pull/246

Nice... the first true fruit of the speculative project. 🙂

(Even though (merge) is legit, speculative's current spec caught that.)

Running speculative through 4clojure should be trivial with https://github.com/mfikes/coal-mine

You tend to see some bizarre code in 4clojure, given it is written by people learning the language 🙂

I'll put together a couple of aliases that we could use to drive speculative through coal-mine, to see if you like them.

We need not even commit them, clj is probably flexible enough to compose speculative and coal-mine right on the command line.

Here’s another challenge for you Mike:

19:21 $ ./run_tests.sh

Running tests in #{"test"}

Testing speculative.core-test

Ran 16 tests containing 56 assertions.
0 failures, 0 errors.

Testing speculative.core-test

ERROR in (merge-test) (RangeError:NaN:NaN)
expected: (nil? (merge))
  actual: #object[RangeError RangeError: Maximum call stack size exceeded]

Ran 16 tests containing 55 assertions.
0 failures, 1 errors.

Testing speculative.core-test
git pu
ERROR in (merge-test) (cljs$core$IFn$_invoke$arity$2@cljs/core.js:336:219)
expected: (nil? (merge))
  actual: #object[RangeError RangeError: Maximum call stack size exceeded.]
s
Ran 16 tests containing 55 assertions.
0 failures, 1 errors.

given the code in https://github.com/slipset/speculative/tree/26_allow_for_merge

So basically, it works as expected in Clojure, but for some reason bombs in Clojurescript

Smells like a bug in ClojureScript... might be worth changing the ClojureScript dep in deps.edn temporarily to point at ClojureScript master's git SHA

BTW, what’s happened to Clojurescript? Is it abandoned? Latest commit 26 days ago!?!?!

It is currently in a quiet mode for release. There is one last change that David has been wanting to do.

oh, the fact that deps.edn doesn’t handle git:.. urls 😕

still fails with “6eedd0a08c49f7b0d4dcb30977b2fb38c90577bd”

deps.edn handles git: urls, but perhaps that causes extra authentication to occur? Hrm.

yes, it was something with the authentication.

Anyway, if broken on master, perhaps a minimal repro is possible in JIRA

I tried a minimal repro without speculative, and of course it passes 😕

Interesting, also wrote a missing test for (merge-with +) which also fails…

{:message "Call to #'cljs.core/merge-with did not conform to spec.",
 :data {:cljs.spec.alpha/problems [{:path [],
                                    :pred (cljs.core/fn [%] (cljs.core/or (cljs.core/nil? %)
                                                                          (cljs.core/sequential? %))),
                                    :val #object[cljs$core$_PLUS_],
                                    :via [], :in []}],
        :cljs.spec.alpha/spec #object[cljs.spec.alpha.t_cljs$spec$alpha19571],
        :cljs.spec.alpha/value #object[cljs$core$_PLUS_],
        :cljs.spec.alpha/fn cljs.core/merge-with,
        :cljs.spec.alpha/args #object[cljs$core$_PLUS_],
        :cljs.spec.alpha/failure :instrument}}

for

(s/fdef clojure.core/merge-with
  :args (s/cat :f ifn? :maps (s/* (s/nilable map?)))
  :ret (s/nilable map?))

It's a bit verbose, but we can run speculative through coal-mine (4clojure) via something like this https://github.com/mfikes/speculative/commit/3606e5d15627483ffde1da2e3aed76a453342533

nice!

PR welcome 😉

I'll let it run on my larger box and see what happens. (You have to be prepared to sacrifice a kilowatt.)

@slipset I wonder if that (merge-with +) spec issue above is related to https://dev.clojure.org/jira/browse/CLJS-2793

interesting

It is probably worth its own JIRA just in case it is a different root cause

I can’t seem to create a minimal repro though.

oh, getting there now:

(ns fail-test
  (:require  [clojure.test :as t :refer [deftest is testing]]
             [clojure.spec.test.alpha :as stest]
             [clojure.spec.alpha :as s]))

(s/fdef clojure.core/merge-with
  :args (s/cat :f ifn?)
  :ret (s/nilable map?))

(stest/instrument `clojure.core/merge-with)

(deftest foo
  (is (nil? (merge-with identity))))

fails with

20:18 $ clj -A:cljstests
WARNING: Use of undeclared Var test.runner/fail-test at line 30 cljs-test-runner-out/gen/test-runner.cljs

Testing fail-test

ERROR in (foo) (Error:NaN:NaN)
expected: (nil? (merge-with +))
  actual: #error {:message "Call to #'cljs.core/merge-with did not conform to spec.", :data {:cljs.spec.alpha/problems [{:path [], :pred (cljs.core/fn [%] (cljs.core/or (cljs.core/nil? %) (cljs.core/sequential? %))), :val #object[cljs$core$_PLUS_], :via [], :in []}], :cljs.spec.alpha/spec #object[cljs.spec.alpha.t_cljs$spec$alpha12632], :cljs.spec.alpha/value #object[cljs$core$_PLUS_], :cljs.spec.alpha/fn cljs.core/merge-with, :cljs.spec.alpha/args #object[cljs$core$_PLUS_], :cljs.spec.alpha/failure :instrument}}

/Users/erik/Documents/github.com/fail/cljs-test-runner-out/cljs/spec/test/alpha.js:133
throw cljs.core.ex_info.call(null,["Call to ",cljs.core.str.cljs$core$IFn$_invoke$arity$1(v__$1)," did not conform to spec."].join(''),ed);
^
Error: Call to #'cljs.core/merge-with did not conform to spec.
    at new cljs$core$ExceptionInfo (/Users/erik/Documents/github.com/fail/cljs-test-runner-out/cljs/core.js:36762:10)
    at Function.cljs.core.ex_info.cljs$core$IFn$_invoke$arity$3 (/Users/erik/Documents/github.com/fail/cljs-test-runner-out/cljs/core.js:36823:9)
    at Function.cljs.core.ex_info.cljs$core$IFn$_invoke$arity$2 (/Users/erik/Documents/github.com/fail/cljs-test-runner-out/cljs/core.js:36819:26)
    at cljs$core$ex_info (/Users/erik/Documents/github.com/fail/cljs-test-runner-out/cljs/core.js:36805:26)
    at /Users/erik/Documents/github.com/fail/cljs-test-runner-out/cljs/spec/test/alpha.js:133:25
    at G__12915__delegate (/Users/erik/Documents/github.com/fail/cljs-test-runner-out/cljs/spec/test/alpha.js:190:15)
    at G__12915 (/Users/erik/Documents/github.com/fail/cljs-test-runner-out/cljs/spec/test/alpha.js:212:27)
    at Function.cljs.core.MetaFn.cljs$core$IFn$_invoke$arity$3 (/Users/erik/Documents/github.com/fail/cljs-test-runner-out/cljs/core.js:6860:113)
    at G__12995__2 (/Users/erik/Documents/github.com/fail/cljs-test-runner-out/cljs/core.js:14945:45)
    at G__12995 (/Users/erik/Documents/github.com/fail/cljs-test-runner-out/cljs/core.js:14984:20)

Oh, I thought it was trivial to repro at the REPL. (I repro'd it in a Planck REPL; let me try a ClojureScript node REPL)

hang on, I’m calling this with identity

and the stacktrace says +

But I guess the (merge) thing is more interesting, as it ends up in a out of memory exception.

it could be some other code triggering this, for example the test script that sums the failed and succeeded tests 😉

Jepps

I got a minimal repro for the (merge-with +) issue; I'll drop it in a JIRA.

https://dev.clojure.org/jira/browse/CLJS-2942

I don’t even get the error that spec reports there

@mfikes: I got an even better one 🙂

attaching to the jira

It’s a much more fun repro with (merge)

got rid of the cljs test runner in favor of the simple script that’s also used for planck https://github.com/slipset/speculative/pull/27/commits/35d5d9805983364b96df8f6aaf8d2591e4fb0e00

and also forced less deps on our consumers

I suspect that Planck could be made to use that test runner as well

merged

Cool. Let me see if I can, via a little conditional code in that namespace, completely eliminate the Planck stuff.

@borkdude that b0rk circle 😕

Oh

#!/bin/bash -eo pipefail
clojure -R:test:runner:cljstests -Spath
Error building classpath. Specified aliases are undeclared: [:runner]
Exited with code 1

Probably makes sense since we removed :runner as an alias.

I bet we could have a single test runner that spans all 3 targets.

I’ve always wanted to work on a remote team with devs in US, Europe, and Asia.

With good devs, you could run a 24/7 operation.

Problem is (as I’m experiencing now) is that its really easy to sit up too late.

I don't think that exiting Node is working with that last commit

For me, it hangs, and there is also this https://dev.clojure.org/jira/browse/CLJS-2637

WIth my latest commit (which fixed the build) or with @borkdude latest?

With the Michiel's latest. But perhaps I have a different environmental issue as it seems to work in CI.

It hangs if you actually have a test failure. You won't see it if all tests pass.

oh yes,

(when-not false #_(cljs.test/successful? m)
    (exit 1))

That is real odd. Hrm. Tach also exits Node that way https://github.com/mfikes/tach/blob/master/src/leiningen/tach.clj#L49

reproducable:

$ clj -A:test -m cljs.main -re node
ClojureScript 1.10.339
cljs.user=> (.exit js/process 1)

If I had to guess, Node may in fact be exiting, but the JVM side doesn't know that this occurred.

Speaking of that, it would also need to somehow affect the JVM exit code.

https://dev.clojure.org/jira/browse/CLJS-2637?page=com.atlassian.jira.plugin.system.issuetabpanels:changehistory-tabpanel ?

Right

Maybe we can use Nashorn

Trying an experiment with that now...

maybe we shouldn’t be using -re but -t?

Switching to Nashorn would do it https://github.com/slipset/speculative/pull/28/commits/d56d88cc2ec1de26bba695cfe3a1b5da49e9fb45

With respect to -t, -re node implies -t node (or -t nodejs, I can't recall)

ok thanks for the fix!

@slipset I’m from Europe btw. Only @mfikes is from the USA. I guess he can do the other 12 hours of the day 😉

I now have this in my build.boot file for the “big” project:

(require '[speculative.core])
(require '[clojure.spec.test.alpha :as stest])
(stest/instrument)
(stest/unstrument `merge)

I’m in Virginia (East Coast)

but it runs now 🙂

I’m surprised it really works now 😄 if I call (juxt 1) I get an error. but my laptop’s fan is spinning, it’s really an extra toll on performance

can we find a way of only checking direct usage of core functions and not indirect? this is what I hoped would happen with clojure being compiled with direct linking, but apparently not

Yeah, for every core fn call, it will do a boatload if extra checking

is clojure itself AOT-compiled when you consume it via a jar?

Right, you’d want spec checking to be disabled “inside” core

@borkdude I know you’re from Europe. My point was just that working with people in EU/US is tiring (because it’s too easy to always be on). Imagine what it’d be like if we had a fourth person in, say Singapore or soemthing.

I guess it makes sense to only turn on instrumentation when your program has finished starting

yeah

I had a similar problem working with people on the West Coast... they keep working once I was ready to stop, have dinner, family time, etc

And they arrive so late at work!

I’m working for a US company, but luckily my team is in Europe

yeah, instrumenting as late as possible just before you’re ready to hit the REPL is what you want with speculative

I’m using boot. It’s all running in one JVM. So when I instrument, my clojurescript will probably be compiled with instrumented clojure, which also slows things down

Anyways, thanks a lot for all the efforts. I’m off to bed.

good night!

yikes, (fn*) is valid in RC1

oh it was valid in 1.8.0 as well phew

probably better to ignore things not in this list anyway: https://clojure.github.io/clojure/clojure.core-api.html