gfredericks / schpec

A utility library for clojure.spec
Eclipse Public License 1.0
45 stars 6 forks source link

instrument for :fn and :ret specs #17

Open madstap opened 7 years ago

madstap commented 7 years ago

This seems to be really popular wish.

Rich Hickey argues that instrument should only be concerned with a function being called correctly, and that :fn and :ret should only be checked by test/check.

I sometimes want feedback faster than that though, and I can think of several situations where it would be nice to have this option.

So it would be nice to have something like schpec/overinstrument that behaves just like test/instrument, but for :ret and :fn specs.

test/instrument used to check everything, but was changed in this commit, so it shouldn't be too hard to make a overinstrument function based on that.

gfredericks commented 7 years ago

sounds great to me; the less code it takes, the better.

We'll need to figure out if this requires a corresponding overunstrument as well.

gfredericks commented 7 years ago

I just read through the relevant clojure.spec.test code and realized that there are some subtleties here, around implementation and usage.

In particular:

I don't think there's a clean way to implement this without pasting a lot of code (>100 lines) from clojure.spec.test, as otherwise we would have to call a lot of private functions, and maybe do something dirty like with-redefs to change how the instrumentation works.

But if we paste a lot of code, then we have to decide if that will include the pair of atoms that spec uses to track the instrumentation state, which would probably be bad because then a user that uses spec/instrument and schpec/overinstrument together would get weird results. But those atoms are private, so using them would require the dirty tricks.

So since we already have to do dirty tricks, maybe with-redefs isn't so bad :/. I suppose we could document that dirty tricks are happening.

madstap commented 7 years ago

Seems like someone has done this already. It's a little different, as they don't have other functions, like overinstrument, but replace instrument. I haven't had time to take a good look at it, but I thought it was worth mentioning here.