speculative 2018-10-20

@borkdude has joined the channel

@mfikes has joined the channel

@slipset has joined the channel

@slipset I’m still trying to merge, even after you closed the PR… damn 🙂

Hopefully it’s all in now.

now I’ve got a conflict on the README. wait a minute 😉

no, I made a fix for the merge test so it worked on cljs as well

Chaotic times in the start of the project. Lots of stuff moving atm

and I added instruction to README, but I’m fixing this conflic tnow

BTW, I discussed this a bit with @mfikes already, but I’m sort of wanting to offer whatever becomes of speculative to clojure/core. That is, I would like speculative to be in a state for clojure/core to take what ever they want from it.

I hope that’s ok with you @borkdude.

I’ve already sent an email to Alex asking what he needs from the project in terms of license/CA and stuff for them to be able to take code from speculative, if they ever wanted to do so.

OK, PR ready. I’m sorry for the merge commits, but this is me trying to keep up 😓

They can take whatever they want for core, no problem.

You know with “Squash + Merge” in github, the merge-commits don’t matter 🙂

ah GH can do that? nice.

@mfikes The tests now run with planck. See the README

hmm, the test for merge is failing under cljs 😕

Not on my machine

$ plk -A:test -e "(require '[clojure.test :as t])" -e "(require '[speculative.core-test])" -e "(t/run-tests 'speculative.core-test)"

Testing speculative.core-test

Ran 2 tests containing 8 assertions.
0 failures, 0 errors.

jeeeez this is moving to fast for me 🙂

we could add those test expressions for plk into a script, I’m not aware of anything more concise. is this something you can put into deps.edn?

I just did like that,

Could probably be improved to return correct return-status so it could be used in the build pipeline.

@mfikes have you used circle with plk, as in any examples on the config.yml that would be needed to make sure plk is available?

@slipset we could also use cljs.main with some other JS runtime, but this was just the easiest to get going initially

I guess the most important thing is to get a critical amount of specs in 🙂

@slipset I made a PR using https://github.com/Olical/cljs-test-runner. Maybe that’s easier on Circle. It supports various engines.

will have a look

default is node

Even if Alex doesn't take anything and the core team writes their own stuff, much of it would have to be ported to ClojureScript. So, ensuring speculative contributors have signed the Clojure CA would make that easier.

So long as Circle is running Ubuntu 16 then it should be relatively easy to have it be set up for plk

sudo add-apt-repository ppa:mfikes/planck
sudo apt-get update
sudo apt-get install planck

I'll see if I can get plk running; it would be good to have both JVM and self-hosted coverage for ClojureScript.

We could use https://lispcast.com/100-most-used-clojure-expressions/ (download: https://lispcast.com/wp-content/uploads/2015/11/clojure-analysis-results.csv) as a list of popular Clojure core functions and work our way down. Make an issue for each function. How does that sound?

That does make sense

I can also imagine that a lot of functions have a lot of common spec. Like keep/filter/map

That’s a very good idea. I was just spec’ing fns that came to mind.

ok, I’ll filter this list on core functions that are not macros/special forms that already have some spec

you can f/def a macro right? I’m not sure how a useful fdef for -> would look like

Oh, wow. That CircleCI box is running an Ubuntu that was just released a couple of days ago (18.10, cosmic)

I'm making a release of Planck for Ubuntu 18.10, which will probably take a few hours to churn through the system.

Not sure what’s going on with #10… I haven’t even instrumented anything yet

Maybe CIDER is doing (stest/instrument) and then our fdef trips some error inside cider

so we have a case to improve our reduce fdef, it’s not working on this expression: https://github.com/clojure-emacs/orchard/blob/master/src/orchard/misc.clj#L36

it’s the same issue again with seqable? vs sequential?

I think the spec might need to be something like that in https://github.com/slipset/speculative/issues/5

I might just let you file a PR for #5 @mfikes 🙂

It’s a bit strange writing tests for str and =

Yeah, I'll file a PR for #5... it might be possible to use sequable? in there under Clojure, but at least the proposal in #5 would get things closer to right

closer to right what we’re aiming for.

So maybe it really is seqable?

I'll try a cleaner patch and see what you all think.

How do you spec the return value based on the args? Not possible currently in spec I think? E.g. ret val of map should be transducer in case of two args. Else it’s a sequential?.

so (s/def ::seq-or-transducer seq?) => (s/def ::seq-or-transducer (s/or :seq :seq? :transducer ifn?))

or maybe even (s/def ::transducer ifn?) so we can improve upon that one later

why ifn? instead of fn? again?

right:

boot.user=> (fn? #{:a})
false
boot.user=> (ifn? #{:a})
true

For :ret you can say (s/or :fn fn? :seq seq?) but if you want to then make statements that depend on the number of args you'd use :fn as described here https://clojure.org/guides/spec#_spec_ing_functions

(I think that, and the :ret spec, really only come into play when you are testing the implementations of the functions.)

I take that back, :ret does matter for doc

yes. I think it would be more correct if we went that way for map, etc.

Also, in other news, stuff is published directly onto clojars now.

@mfikes: I’m seeing that the tests are becoming rather repetitive, if this were a pure clojure project, I’d write a (with-instrumentation ...) macro in speculative/core-test

But since we’re also supporting the land of selfhosted clojurescript as well, I need some help on that.

one note about the tests: I’m not sure if we should call unstrument, because before the tests someone may have instrumented it already.. but we don’t expect people to run these tests, so probably OK

I guess the help I need is just understanding/being reminded of where to put the macro.

we could use macrovich for this and define the macro in the .cljc file

If that with-instrumentation macro were is some utility namespace, then it would be a little easier. It would go in a clj file, but there would also be a cljs file that does :require-macros on it

Macrovich works too

so test_utils.clj(s) it is

utility namespace is better probably

eh self hosted…

macrovich works 😉

so a test_utils.cljc file then?

I think we might not even need that macro. Just instrument all and run the tests.

It’s mostly that I wanted to prevent this weird CIDER issue that I called unstrument

but we just have to fix it

You can have a test_utils.cljs and test_utils.clj file and it will work in self hosted

cool, didn’t know

In other words, self-hosted also loads macros from clj files

if you want to do it correctly you have to record which functions are instrumented before testing and then restore after, but I wouldn’t bother @slipset. Just remove the unstruments

Hrm. @slipset I wonder if you have things deploying to clojars in CI. In my personal CI build it is failing when trying to do this.

Hmm, bummer. I guess I need to set up a script that checks what repo we’re building from, and only deploy when building from mine.

btw: (map 'lol [1 2 3]) is valid usage of map

oh it’s already ifn? in the code, the README is behind

Right 'lol can be used to look up things

Heh, also, if nothing else, participating in this project is a great way to explore the corners of the language 🙂

yeah, it’s fun already 😉

@slipset did you read my comments on unstrument btw? 🙂

I only added it because of the CIDER issue I had, not really necessary anymore imho

and what if I instrumented map and then run the tests? suddenly it’s unstrumented 😉

so what I would do is not bother with this macro at all, just call (st/instrument) and be done

That's kind-of what I had in mind. (st/instrument) near the top of the test namespace

one more thought: could we order the specs alphabetically so they’re at least easy to find?

I agree with that

Dang. Erik is on fire.

typo: (is (= "") (str nil))

Ok, I’ll revert the test-util stuff tomorrow.

One simplification that could be done is

`map
can be used instead of
`clojure.core/map

Fighting a bit with circle to only make it deploy from master and on my thing.

you could leave the util ns if we’re not using the macro, it’ll probably come in handy later

thanks Erik!

Hopefully latest master only deploys from my circle

Oh, well. There’s a day tomorrow as well. Thanks for all the help!

btw, do we need the :binary/:trinary thing any more?

(s/fdef clojure.core/reduce
  :args (s/or :binary  (s/cat :f fn? :coll ::reduceable-coll)
              :trinary (s/cat :f fn? :val any? :coll ::reduceable-coll)))

The error is a bit more verbose than it needs to:

speculative.core> (reduce 'lol 'lol)
Evaluation error - invalid arguments to clojure.core/reduce at (NO_SOURCE_FILE:44).
lol - failed: fn? at: [:binary :f]
lol - failed: fn? at: [:trinary :f]
speculative.core>