jorgenschaefer / emacs-buttercup

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

Allow clean abort #167

Closed doublep closed 4 years ago

doublep commented 4 years ago

Would Buttercup be interested in the following functionality?

  1. Test run can be aborted cleanly at any point (by a test, by the reporter). "Cleanly" means that corresponding *-done for started specs and suites are emitted, the run ends in buttercup-done event, but no more specs are executed after aborting.

  2. New option: immediately abort if a test fails. This is useful if your tests are so interdependent (at least in the context of the current bug) that one failure typically means there will be 20 more. Also when tests are slow enough that you don't want to wait for all of them to finish to get the backtrace of the first failure.

jorgenschaefer commented 4 years ago

Thank you for your contribution! :-)

snogge commented 4 years ago

I'm not saying we should not have this functionality, but I would like to revert it now, refine it a bit and possibly merge it later. Here's why:

I think @doublep meant this PR mostly as an example of what he wanted to achieve anyway. What do you think, @jorgenschaefer and @doublep?

By the way, I added these (the first one fails) tests to check the reporter and the before/after functions:

(describe "When tests abort"
  (it "the before and after functions should still run"
    (let (all-before all-after each-before each-after)
      (expect
       (with-local-buttercup
         (setq buttercup-stop-on-first-failure t)
         (describe "suite"
           (before-all (setq all-before t))
           (after-all    (setq all-after t))
           (before-each 
             (setq each-before t)
             (spy-on 'not-a-real-function))
           (after-each (setq each-after t))
           (it "spec that fails"
             (not-a-real-function)
             (expect nil)))
         (buttercup-run)) :to-throw)
      (expect (symbol-function 'not-a-real-function) :to-be nil)
      (expect (list all-before all-after each-before each-after) :to-equal '(t t t t)))))

(describe "The `buttercup-run' function"
  ;; .....
  (it "should call all reporter steps even on abort"
    (let (reporter-args)
      (with-local-buttercup
       (setq buttercup-reporter (lambda (&rest args) (push args reporter-args)))
       (describe "suite"
         (it "spec"
           (signal 'buttercup-abort "buttercup-abort")))
       (buttercup-run))
      (expect (nreverse (mapcar #'car reporter-args)) :to-equal
              '(buttercup-started
                suite-started
                spec-started
                spec-done
                suite-done
                buttercup-done))))
)
jorgenschaefer commented 4 years ago

Sorry, you are right.

snogge commented 4 years ago

I have reverted the two commits from this PR that @jorgenschaefer cherry-picked to master. I don't know whether this PR can be reopened, but otherwise feel free to open a new PR. I've mentioned some things above that I think need to be addressed before this PR can be merged. My focus right now is finding solutions for #149 and #157, but I will return to this and @doublep's other eldev-related PRs eventually.