From 67eea929cb38b01a84a5285baf35e297e21b67cf Mon Sep 17 00:00:00 2001
From: Olof-Joachim Frahm <olof@macrolet.net>
Date: Mon, 3 Dec 2012 22:34:41 +0100
Subject: [PATCH] Change IN-SUITE* to update the suite.

See also issue #1.

With this change, the properties, including :IN, are updated while
keeping the suite object alive.  This means however, that because of the
default value of *SUITE* for the parent, resetting the relationship is
error-prone, so an explicit self-check is inserted to prevent a link
from a suite onto itself.

I would be really happy to change the default to T instead of *SUITE*.
That would also make things more explicit in the long run.

Included is also a testcase, which would previously fail (to update the
old object).
---
 src/suite.lisp |   63 +++++++++++++++++++++++++++++++++++---------------------
 t/tests.lisp   |   13 ++++++++++++
 2 files changed, 53 insertions(+), 23 deletions(-)

diff --git a/src/suite.lisp b/src/suite.lisp
index 88bb34b..e9d9e22 100644
--- a/src/suite.lisp
+++ b/src/suite.lisp
@@ -51,39 +51,57 @@ def-test) to pass to tests in this suite."
      (def-suite ,name ,@def-suite-args)
      (in-suite ,name)))
 
-(defun remove-from-suites (test-name)
-  (when (get-test test-name)
-    ;; if this suite already exists, and its :IN some other suite, remove it.
-    (dolist (s (list-all-suites))
-      (when (gethash test-name (tests s))
-        (remhash test-name (tests s))))))
-
 (declaim (special *suite*))
 
 (defun make-suite (name &key description ((:in parent-suite) *suite*) fixture)
   "Create a new test suite object.
 
 Overrides any existing suite named NAME."
-  (remove-from-suites name)
-  (let ((suite (make-instance 'test-suite :name name :fixture fixture)))
-    (when description
-      (setf (description suite) description))
+  (let ((suite (make-instance 'test-suite :name name)))
+    (%update-suite name suite description parent-suite fixture)
     (setf (gethash name *suites*) suite)
-    (loop for i in (ensure-list parent-suite)
-          for in-suite = (get-test i)
-          do (progn
-               (when (null in-suite)
-                 (cerror "Create a new suite named ~A." "Unknown suite ~A." i)
-                 (setf (get-test in-suite) (make-suite i)
-                       in-suite (get-test in-suite)))
-               (setf (gethash name (tests in-suite)) suite)))
     (setf (get-test name) suite)
     suite))
 
+(defun update-suite (name suite
+                     &key description ((:in parent-suite) *suite*) fixture)
+  (%update-suite name suite description parent-suite fixture))
+
 (defun list-all-suites ()
   "Returns an unordered LIST of all suites."
   (hash-table-values *suites*))
 
+(defun remove-from-suites (name &optional parents)
+  (when (get-test name)
+    ;; if this suite already exists, and its :IN some other suite, remove it.
+    (dolist (s (list-all-suites))
+      (let ((tests (tests s)))
+        (when (and (not (member (name s) parents :test #'eq))
+                   (gethash name tests))
+          (remhash name tests))))))
+
+(defun add-to-suites (name suite parents)
+  (dolist (i parents)
+    (let ((in-suite (get-test i)))
+      (when (null in-suite)
+        (cerror "Create a new suite named ~A." "Unknown suite ~A." i)
+        (setf in-suite (make-suite i)
+              (get-test in-suite) in-suite))
+      (setf (gethash name (tests in-suite)) suite))))
+
+(defun remove-from-add-to-suites (test-name suite parent-suite)
+  ;; prevent cycles
+  (unless (eq suite parent-suite)
+    (let ((parents (ensure-list parent-suite)))
+      (remove-from-suites test-name parents)
+      (add-to-suites test-name suite parents))))
+
+(defun %update-suite (name suite description parent-suite fixture)
+  (setf (description suite) description)
+  (setf (fixture suite) fixture)
+  (remove-from-add-to-suites name suite parent-suite)
+  suite)
+
 ;;;; ** Managing the Current Suite
 
 (defvar *suite* (setf (get-test 'T) (make-suite 'T :description "Default global suite" :in nil))
@@ -100,7 +118,7 @@ See also: `DEF-SUITE` and `*SUITE*`. "
 
 (defmacro in-suite* (suite-name &rest def-suite-args)
   "Same effect as `IN-SUITE`, but if `SUITE-NAME` does not exist it
-will be created (as per DEF-SUITE)"
+will be created (as per DEF-SUITE), or updated with the new arguments."
   `(%in-suite ,suite-name
               :fail-on-error nil
               ,@def-suite-args))
@@ -115,13 +133,12 @@ will be created (as per DEF-SUITE)"
     (with-gensyms (suite)
       `(progn
          (if-let (,suite (get-test ',suite-name))
-           (setf *suite* ,suite)
+           (setf *suite* (update-suite ',suite-name ,suite ,@def-suite-args))
            (progn
              ,@(when fail-on-error
                  `((cerror "Create a new suite named ~A."
                            "Unknown suite ~A." ',suite-name)))
-             (setf (get-test ',suite-name) (make-suite ',suite-name ,@def-suite-args)
-                   *suite* (get-test ',suite-name))))
+             (setf *suite* (make-suite ',suite-name ,@def-suite-args))))
          ',suite-name))))
 
 ;; Copyright (c) 2002-2003, Edward Marco Baringer
diff --git a/t/tests.lisp b/t/tests.lisp
index 27b4229..e3a7e40 100644
--- a/t/tests.lisp
+++ b/t/tests.lisp
@@ -292,3 +292,16 @@
 
     (def-test foo (:suite nil) t)
     (is-false (gethash 'foo (tests *suite*)))))
+
+
+;;;; test suites, *suite*, in-suite* behaviour
+
+(def-test suite-redefinition ()
+  (rem-test 'a-suite)
+  (in-suite* a-suite :description "a suite")
+  (let ((a-suite (get-test 'a-suite)))
+    (is (string= "a suite" (description a-suite)))
+
+    (in-suite* a-suite :description "the same suite")
+    (is (eq a-suite (get-test 'a-suite)))
+    (is (string= "the same suite" (description a-suite)))))
-- 
1.7.10.4