hyperfiddle / rcf

RCF – a REPL-first, async test macro for Clojure/Script
MIT License
278 stars 12 forks source link

`:=` macro equivalent? #46

Open xificurC opened 2 years ago

xificurC commented 2 years ago

The infix := operator, while looking nice, can be problematic. First of all it's impossible to align code correctly with it. Secondly there are edge cases like #40 and #42 where the expansion isn't working currently. Thirdly some people might prefer a more lisp-y style of defining the assertions.

Would it be complicated to allow asserting manually? Something like

(rcf/assert= (+ 2 3) 5)
ggeoffrey commented 2 years ago

We are considering compatibility with clojure.test/assert-expr. Then:

xificurC commented 2 years ago

you mean prefix, right?

dustingetz commented 2 years ago

First of all it's impossible to align code correctly with it

@xificurC can you give me an example of what you mean by this?

xificurC commented 2 years ago

Sure. The examples on the homepage like

{:a :b, :b [2 :b]} := {:a _, _ [2 _]}

look very nice, but once your test doesn't nicely fit on a single line, how do you align it to look well?

{:a :b, :b [2 :b]}
:=
{:a _, _ [2 _]}

This leaves a whole line for the poor :=, so lonely!

{:a :b, :b [2 :b]}
:= {:a _, _ [2 _]}

or

{:a :b, :b [2 :b]} :=
{:a _, _ [2 _]}

are just hard to read, especially when the code is bigger, one has to search for the :=, which is not good.

With a prefix operator this problem is solved

(assert= {:a :b, :b [2 :b]}
         {:a _, _ [2 _]}
dustingetz commented 2 years ago

Thank you! You're right, we do write trailing := a bit.

So many people are asking for this that we are possibly willing to reconsider. So if I can follow up:

How would you write repl-style code that uses bindings to *1 etc?

 (tests
    "REPL bindings work"
    (keyword "a") := :a
    (keyword "b") := :b
    (keyword "c") := :c
    *1 := :c
    *2 := :b
    *3 := :a
    *1 := :c                   ; inspecting history does not affect history

    (keyword "d") := :d
    *1 := :d
    *2 := :c
    *3 := :b
    (symbol *2) := 'c          ; this does affect history
    (symbol *2) := 'd)

Note you don't want 1 to be bound to the result of the assertion, you want it bound to the LHS. Should we change the rules of 1?

dustingetz commented 2 years ago

Here are some real world tests from our codebase -

Photon compiler https://gist.github.com/dustingetz/12a6eb81e585942e277b4bec47d98dd5

Missionary m/cp explanation https://gist.github.com/dustingetz/9f8f8ae739fc6efc70f4f5855817b85d

and of course in the wild https://github.com/clojure/core.unify/blob/e2ae03c08b45528f3274d154a8f4625e7acbeccd/src/main/clojure/clojure/core/unify.clj#L246-L358

xificurC commented 2 years ago

The "in the wild" example doesn't seem to be using RCF?

How would you write repl-style code that uses bindings to *1 etc?

Care to elaborate how does infix notation pose challenges to this? If (keyword "a") := :a is written as e.g. (rcf/assert= (keyword "a") :a), we still know what LHS is.

If you're thinking of changes in a broader sense I'd like to propose to allow third-party extensions like matcher-combinators or https://github.com/lomin/sinho to integrate into rcf. It would also be nice to get reporting right

dustingetz commented 2 years ago

If you paste (assert= (keyword "a") :a) into the REPL, *1 would be expected to bind to the result of the assert (a bool or something), not :a, which is broken, we'd have to hack the behavior right?

Thanks for the sinho and matcher combinators links, we definitely feel the need for m/embeds etc, happy to document and support the best patterns after https://github.com/hyperfiddle/rcf/issues/50 ships.

Thanks for the ideas regarding reporting - can you please open a new issue for reporting and specify exactly how reporting should work?

xificurC commented 2 years ago

I thought you are already hacking the REPL bindings?

user=> (keyword "B") := :a
:B
:=
:a
user=> *1
:a

Pasting the infix check into the REPL doesn't bind the result of (keyword "B") into *1

dustingetz commented 2 years ago

Our worldview is that in this real-world RCF:

; https://clojuredocs.org/clojure.core/map
(map inc [1 2 3 4 5])
;;=> (2 3 4 5 6)

To learn what this does, you send this to the repl: (map inc [1 2 3 4 5]); *1 is now '(2 3 4 5 6)

The point of the RCF is to document what the target does (as if in a tutorial) without altering the target

xificurC commented 2 years ago

in this real-world RCF

I don't think you documented what RCF stands for and I don't know what the acronym means, care to define it for me? Thanks!

The "REPL bindings work" example test you posted shows that *1 and friends are somehow made to work. That means you already have some code that makes them work. Can't you reuse that code for a prefix operator? I'm sorry but I still don't see how adding a prefix operator would make your work any harder.

dustingetz commented 2 years ago

Ah the question is not can we do it, the question is what is the least surprising behavior. Consider:

in (tests (inc 1) := _) it is clear that *1 should bind to 2; in (tests (is (= (inc 1) 2))) what is *1? There are two reasonable behaviors - what is returned and what = returned. And neither of those behaviors are the thing you actually want, which is to refer to the result of (inc 1) in the next assertion so that you can do (tests (inc 1) := 2 (dec *1) := 1).

The root cause of the mess is that the testing boilerplate has infected the target expression.

RCF means rich comment form

xificurC commented 2 years ago

I understand now, thanks.

I see a couple of options:

  1. prefix matchers will have to implement this behavior somehow. If the typical setup is (operator actual expected) then a simple default might already cover 90% of the use cases. If this is documented it doesn't seem too surprising, to me at least.
  2. prefix matchers will not support REPL history.

The root cause of the mess is that the testing boilerplate has infected the target expression.

But the point of the library is to test, you can't consider it boilerplate! :)

Anyway, there are other alternatives too

(testing (keyword "B")
  (= :B)
  (= (namespace %) nil)
  (= (name %) "B"))

Off the top of my head :)