hi
Howdy
@slipset I’m still trying to merge, even after you closed the PR… damn 🙂
Sorry 🙂
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
Ah, ok.
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.
Merged.
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?
https://github.com/slipset/speculative/commit/689e974dcfb9a7f6f1d0e8e76fd47fdce8492573
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
probably
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)
Yeah, you can do macro defs @borkdude Here is an example https://github.com/clojure/clojurescript/blob/master/src/main/cljs/cljs/core/specs/alpha.cljc#L68
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.
Actually, CollReduce has this verbiage: https://github.com/clojure/clojure/blob/master/src/clj/clojure/core/protocols.clj#L15
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.
Agreed
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
.clj
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
exactly
one more thought: could we order the specs alphabetically so they’re at least easy to find?
I agree with that
goodnight
🌔
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>