jorgenschaefer / emacs-buttercup

Behavior-Driven Emacs Lisp Testing
GNU General Public License v3.0
360 stars 44 forks source link

Spies don't work in `before-all` #180

Open lastquestion opened 4 years ago

lastquestion commented 4 years ago

I spent a hot minute quickly debugging into this, and it's because before-all doesn't run in a buttercup-with-cleanup, so buttercup--add-cleanup doesn't work.

I think it would be nice if spies worked in before-all, as well as before-each. ~For now, I've worked around this by making a nested describe with just one before-each in the parent suite.~ Oh, that doesn't actually do what I want. :-(

snogge commented 3 years ago

This is actually documented in https://github.com/jorgenschaefer/emacs-buttercup/blob/master/docs/writing-tests.md#spies

Trying to defines spies in before-all is supposed to cause an error. Does that not happen for you?

You should also see #122 for more background information.

snogge commented 3 years ago

What are you trying to achieve with the spy in before-all? If you want to preserve some sort of state across tests in a suite something like the code below should work:

(describe "A suite"
  :var (callcount)
  (before-each 
    (spy-on 'some-function :and-call-fake (lambda () (incf callcount))))
  (it "with a spec"
    (some-function)))
lastquestion commented 3 years ago

Oh, so sorry! I didn't check the documentation carefully enough! And yes, indeed, it does print an error.

I expected something like this to work:

(describe "A suite"
  (before-all
    (spy-on 'a-function :and-call-fake (lambda () t))
    (very-expensive-thing-that-runs-once-that-calls-a-function))

  (it
    "generates the right XYZ in the buffer"
  t)

  (it
    "correctly called the spy with whatever"
  t)

  (it
    "Does the right ABC in the buffer"
  t))

In other words, I would like to run some expensive thing once, and during that thing, I would want to spy on the various things it calls. Then, in each it I assert the various behaviors of that thing worked correctly.

Now, yes, I could write that as a bunch of assertions all in one it block, but I guess I prefer breaking out the its into logical grouping of assertions.

Would you be open to extending spies to be supported at the before-all clause, and the after-all clause will reset the spy? I would contribute a PR to that effect if that feature does not seem problematic.

snogge commented 3 years ago

I wont commit to accepting a PR until I've seen it:)

I'm a bit hesitant to change how spies work. At the moment they will be reset - including call statistics - at the end of each it. I think that is a useful feature of spies and don't want to loose it. It feels like mixing spy scopes for both describe and it will be tricky to do in a clean way.

But if you can figure out a way to deal with that I'm not opposed to the feature as such.

It looks like what you want is similar to Pythons subtests. I'm not sure what a syntax for that would look like in buttercup.

snogge commented 3 years ago

Hm, maybe #13 is also relevant here.

lastquestion commented 3 years ago

Haha, of course, but I really just meant if you were interested in the direction.

Hm, my read of subtests is that it is a way to repeat a test against different repeating input.

I'm more interested in testing different aspects of "one run" in multiple "it" blocks. It's not so much I want to carry state or repeat things or anything like that. I think it's just an organizational thing; perhaps it comes from web development:

describe 
  the home page (visit it once)
  it
    shows the hero image
  it
    shows the call to action
  it
     has a menu bar
  it
    shows the login button

is obviously equivalent to

describe the home page
  it works
    visit the page once
    assert the hero image appears
    assert the call to action
    ....

but the former is laid out nicer and the reporting is better, I think.

anyway, I think I will give it a go. Spyies can report and be reset in the after all of the suite, I think it's conceptually the same as having them reset in between each it. Will report back how it turns out.