Merge branch 'spy-scope-error'

* spy-scope-error:
  Raise an error if spy-on is used in the wrong context

3 files changed, 31 insertions, 12 deletions

diff --git a/buttercup.el b/buttercup.el
index ed58dc3..45ce245 100644
--- a/buttercup.el
+++ b/buttercup.el
@@ -1135,14 +1135,16 @@ responsibility to ensure ARG is a command."
                 nil))
             (_
              (error "Invalid `spy-on' keyword: `%S'" keyword)))))
-    (buttercup--spy-on-and-call-replacement symbol replacement)))
+    (unless (buttercup--spy-on-and-call-replacement symbol replacement)
+      (error "Spies can only be created in `before-each'"))))
+
 
 (defun buttercup--spy-on-and-call-replacement (spy fun)
   "Replace the function in symbol SPY with a spy calling FUN."
   (let ((orig-function (symbol-function spy)))
-    (fset spy (buttercup--make-spy fun))
-    (buttercup--add-cleanup (lambda ()
-                              (fset spy orig-function)))))
+    (when (buttercup--add-cleanup (lambda ()
+                                  (fset spy orig-function)))
+      (fset spy (buttercup--make-spy fun)))))
 
 (defun buttercup--make-spy (fun)
   "Create a new spy function wrapping FUN and tracking calls to itself."
@@ -1182,7 +1184,10 @@ responsibility to ensure ARG is a command."
                (apply ',this-spy-function args))))
     this-spy-function))
 
-(defvar buttercup--cleanup-functions nil)
+(defvar buttercup--cleanup-functions :inactive
+  "Stack of cleanup operations.
+Should always be set to a value that is not `listp', except while
+in a `buttercup-with-cleanup' environment.")
 
 (defmacro buttercup-with-cleanup (&rest body)
   "Execute BODY, cleaning spys and the rest afterwards."
@@ -1194,8 +1199,9 @@ responsibility to ensure ARG is a command."
 
 (defun buttercup--add-cleanup (function)
   "Register FUNCTION for cleanup in `buttercup-with-cleanup'."
-  (setq buttercup--cleanup-functions
-        (cons function buttercup--cleanup-functions)))
+  (when (listp buttercup--cleanup-functions)
+    (setq buttercup--cleanup-functions
+          (cons function buttercup--cleanup-functions))))
 
 (defun spy-calls-all (spy)
   "Return the contexts of calls to SPY."
diff --git a/docs/writing-tests.md b/docs/writing-tests.md
index 8ff86e0..a78ac83 100644
--- a/docs/writing-tests.md
+++ b/docs/writing-tests.md
@@ -367,9 +367,9 @@ frameworks call these mocks and similar, we call them spies, because
 their main job is to spy in on function calls. Also, Jasmine calls
 them spies, and so do we. A spy can stub any function - whether it
 already exists or not - and tracks calls
-to it and all arguments. A spy only exists in the `describe` or `it`
-block it is defined in, and will be activated before and deactivated
-and reset after each spec. There are
+to it and all arguments. Spies may only be created in `before-each` or
+`it` blocks. Spies are removed and all counters reset after each spec
+and its `after-each` blocks have completed. There are
 special matchers for interacting with spies. The
 `:to-have-been-called` matcher will return true if the spy was called
 at all. The `:to-have-been-called-with` matcher will return true if
diff --git a/tests/test-buttercup.el b/tests/test-buttercup.el
index 03aa6bb..a73f2ee 100644
--- a/tests/test-buttercup.el
+++ b/tests/test-buttercup.el
@@ -329,7 +329,7 @@
          buttercup--after-each
          buttercup--before-all
          buttercup--before-each
-         buttercup--cleanup-functions
+         (buttercup--cleanup-functions :invalid)
          buttercup--current-suite
          (buttercup-reporter #'ignore)
          buttercup-suites
@@ -696,7 +696,20 @@
          :not :to-throw)
         (expect
          (test-function 1 2)
-         :to-throw 'error)))
+         :to-throw 'error))
+
+      (describe "will signal en error if"
+        (it "used in before-all"
+          (with-local-buttercup
+            (let ((suite (describe "A bad spy scope"
+                           (before-all
+                             (spy-on 'some-function)))))
+              (expect (run--suite suite)
+                      :to-throw))))
+        (it "used directly in describe"
+          (with-local-buttercup
+            (expect (describe "Not in describe"
+                      (spy-on 'foo)) :to-throw)))))
 
     (describe ":to-have-been-called matcher"
       (before-each