ECL regression since deleting support for top-level assertions

[jonatack]

CL-Kraken uses Rove for testing SBCL, CCL, CLISP, ABCL and ECL since early this year (2019).

Commit 8859b0a seems to have caused a bug when used with ECL 16.1.3. The other implementations seem unaffected. After that commit, when the tests are run with ECL, Rove appears to be mutating varables under test between assertions.

Thanks again for this excellent test library.

[fukamachi]

Can you tell me the issue more clearly?

• Reproducible code or steps
• What is the unexpected behaviour you get

[jonatack]

~/quicklisp/local-projects/rove ((HEAD detached at ca5bf91))$ ecl
To load "cl-kraken":
  Load 1 ASDF system:
    cl-kraken
; Loading "cl-kraken"
.........................
ECL (Embeddable Common-Lisp) 16.1.3

CL-USER[1]> (rove:run :cl-kraken/tests/time)

Testing System cl-kraken/tests/time

;; testing 'cl-kraken/tests/time'
  cl-kraken/tests/time
    unix-time-in-microseconds
      is an integer
        ✓ Expect (INTEGERP TIME) to be true.
      is 51 bits in length
        ✓ Expect (= 51 (INTEGER-LENGTH TIME)) to be true.
    generate-kraken-nonce
      is a string
        ✓ Expect (SIMPLE-STRING-P NONCE) to be true.
      is 16 characters in length
        ✓ Expect (= 16 (LENGTH NONCE)) to be true.
      is continually increasing
        ✓ Expect (> NEW-NONCE OLD-NONCE) to be true.

✓ 1 test completed

Summary:
  All 1 test passed.
T
(#<a ROVE/CORE/RESULT:PASSED-TEST>)

CL-USER[3]> (quit)

~/quicklisp/local-projects/rove ((HEAD detached at ca5bf91))$ git checkout 8859b0a

Previous HEAD position was ca5bf91 Merge pull request #22 from y2q-actionman/cleanup-equal-star

HEAD is now at 8859b0a Delete a support for toplevel assertions for reducing memory since it's not used, I think.

~/quicklisp/local-projects/rove ((HEAD detached at 8859b0a))$ ecl

[recompiles rove...]

To load "cl-kraken":
  Load 1 ASDF system:
    cl-kraken
; Loading "cl-kraken"
.........................
ECL (Embeddable Common-Lisp) 16.1.3

CL-USER[1]> (rove:run :cl-kraken/tests/time)

Testing System cl-kraken/tests/time

;; testing 'cl-kraken/tests/time'
  cl-kraken/tests/time
    unix-time-in-microseconds
      is an integer
        ✓ Expect (INTEGERP TIME) to be true.
      is 51 bits in length
        × 0) Expect (= 51 (INTEGER-LENGTH TIME)) to be true.
    generate-kraken-nonce
      is a string
        ✓ Expect (SIMPLE-STRING-P NONCE) to be true.
      is 16 characters in length
        × 1) Expect (= 16 (LENGTH NONCE)) to be true.
      is continually increasing
        × 2) Raise an error while testing.

× 1 of 1 test failed

0) cl-kraken/tests/time
     › unix-time-in-microseconds
       › is 51 bits in length

   Expect (= 51 (INTEGER-LENGTH TIME)) to be true.
     (= 51 #1=(INTEGER-LENGTH TIME))
         TIME = 46005
         #1# = 16

1) cl-kraken/tests/time
     › generate-kraken-nonce
       › is 16 characters in length

   Expect (= 16 (LENGTH CL-KRAKEN/TESTS/TIME::NONCE)) to be true.
   TYPE-ERROR: 46011 is not of type SEQUENCE.
     (= 16 (LENGTH CL-KRAKEN/TESTS/TIME::NONCE))

     0: (ROVE/CORE/ASSERTION::MAIN 35618 (46011) (46011) (NONCE) INTERRUPT-WITH-ERROR NIL)
     1: (#:MAIN321)
     2: (#:MAIN266)
     3: (#:G265)
     4: (SI:BYTECODES)
     5: (SI:BYTECODES)

2) cl-kraken/tests/time
     › generate-kraken-nonce
       › is continually increasing

   Raise an error while testing.
   SIMPLE-TYPE-ERROR: In function PARSE-INTEGER, the value of the first argument is
     46013
   which is not of the expected type STRING

     0: (PARSE-INTEGER)
     1: (#:MAIN399)
     2: (#:MAIN266)
     3: (#:G265)
     4: (SI:BYTECODES)
     5: (SI:BYTECODES)

Summary:
  1 test failed.
    - cl-kraken/tests/time

(#<a ROVE/CORE/RESULT:FAILED-TEST>)

CL-USER[3]> (quit)

~/quicklisp/local-projects/rove ((HEAD detached at 8859b0a))$ 

[jonatack]

So, it looks like LET no longer works within Rove tests using ECL in the case of nested LETs or after the first test reference to the local variable, e.g. the first test that uses a local variable defined in LET is fine but all the following tests within the scope of the LET see the wrong value.

The workaround for me has been to not use LETs with more than one testing defined inside their scope, and to not use any nested LETs in Rove tests. The issue remains, but this is the workaround I've found.

[jonatack]

Examples (red used to work but is now broken, green is the workaround):

 (deftest unix-time-in-microseconds
-  (let ((time (cl-kraken/src/time:unix-time-in-microseconds)))
-    (testing "is an integer"
-      (ok (integerp time)))
-    (testing "is 51 bits in length"
-      (ok (= 51 (integer-length time))))))
+  (testing "is an integer"
+    (ok (integerp (cl-kraken/src/time:unix-time-in-microseconds))))
+  (testing "is 51 bits in length"
+    (ok (= 51 (integer-length (cl-kraken/src/time:unix-time-in-microseconds))))))
 
 (deftest generate-kraken-nonce
-  (let ((nonce (cl-kraken/src/time:generate-kraken-nonce)))
-    (testing "is a string"
-      (ok (simple-string-p nonce)))
-    (testing "is 16 characters in length"
-      (ok (= 16 (length nonce))))
-    (testing "is continually increasing"
-      (let ((old-nonce (parse-integer
-                        nonce))
-            (new-nonce (parse-integer
-                        (cl-kraken/src/time:generate-kraken-nonce))))
-        (ok (> new-nonce old-nonce))))))
+  (testing "is a string"
+    (ok (simple-string-p (cl-kraken/src/time:generate-kraken-nonce))))
+  (testing "is 16 characters in length"
+    (ok (= 16 (length (cl-kraken/src/time:generate-kraken-nonce)))))
+  (testing "is continually increasing"
+    (let ((old-nonce (parse-integer
+                      (cl-kraken/src/time:generate-kraken-nonce)))
+          (new-nonce (parse-integer
+                      (cl-kraken/src/time:generate-kraken-nonce))))
+      (ok (> new-nonce old-nonce)))))

 (deftest server-time
-  (let* ((now (timestamp-to-unix (now)))
-         (response  (cl-kraken:server-time))
-         (time (filter response "result" "unixtime")))
-    (testing "evaluates to the expected server time"
+  (testing "evaluates to the expected server time"
+    (let* ((now      (timestamp-to-unix (now)))
+           (response (cl-kraken:server-time))
+           (time     (filter response "result" "unixtime")))
       (ok (consp response))
       (ok (consp (cadr (cdaddr response))))
       (ok (consp (caddr (cdaddr response))))
-      (ok (equal response (expected-list time))))
-    (testing "evaluates to a Unix Time component expressed as an integer"
-      (ok (integerp time)))
-    (testing "evaluates to Unix Time ±20 seconds of current time, given skew"
+      (ok (equal response (expected-list time)))
+      ;; evaluates to a Unix Time component expressed as an integer"
+      (ok (integerp time))
+      ;; evaluates to Unix Time ±20 seconds of current time, given skew
       (ok (< (abs (- time now)) 20))))

 (deftest post-http-headers
-  (let* ((path   "/0/private/Balance")
-         (nonce  "1234567890123456789")
-         (key    "01dB/y38ooyXBUWpS7XUNguXCk1trgN/LEj7FF8LgHmk3fcvX4dNQIFD")
-         (secret (concatenate 'string
-                              "YS/EXE3mfINjlKeegUVPT0uDUYkUX2Ed0OZp9dzCe1LOs+d"
-                              "9vZErAQKMY9o7WVQlTpvDodSlOONkZK7rngdJNw==")))
-    (testing "with nonce data, evaluates to the correct headers alist"
-      (let ((data     `(("nonce" . ,nonce)))
-            (api-sign (concatenate
-                       'string
-                       "kc0yOGvxuk+LzgTXuvPp3Cs6BvkVhGaGZUNkatqtX2iCb30"
-                       "znwbuVX8JJYdwCisyG/7mScSYl7nZ7ihzvMXrXA==")))
-        (ok (equalp
-             (cl-kraken/src/http::post-http-headers path nonce data key secret)
-             `(("api-key" . ,key) ("api-sign" . ,api-sign))))))
-    (testing "with nonce + params data, evaluates to the correct headers alist"
-      (let ((data     `(("pair" . "xbteur, xbtusd") ("nonce" . ,nonce)))
-            (api-sign (concatenate
-                       'string
-                       "lQjzgTnvmjJ9HMiucF+M3T7cI/VTYjZFptWDbf0uFG6RXLH"
-                       "sedsZaJ8n+HPn8G5exNwkzQC3phqXRqUi7g96Gw==")))
-        (ok (equalp
-             (cl-kraken/src/http::post-http-headers path nonce data key secret)
-             `(("api-key" . ,key) ("api-sign" . ,api-sign))))))))
+  (testing "with nonce data, evaluates to the correct headers alist"
+    (let* ((path "/0/private/Balance")
+           (nonce "1234567890123456789")
+           (key "01dB/y38ooyXBUWpS7XUNguXCk1trgN/LEj7FF8LgHmk3fcvX4dNQIFD")
+           (secret (concatenate 'string
+                                "YS/EXE3mfINjlKeegUVPT0uDUYkUX2Ed0OZp9dzCe1LO"
+                                "s+d9vZErAQKMY9o7WVQlTpvDodSlOONkZK7rngdJNw=="))
+           (data `(("nonce" . ,nonce)))
+           (api-sign (concatenate
+                      'string
+                      "kc0yOGvxuk+LzgTXuvPp3Cs6BvkVhGaGZUNkatqtX2iCb30"
+                      "znwbuVX8JJYdwCisyG/7mScSYl7nZ7ihzvMXrXA==")))
+      (ok (equalp
+           (cl-kraken/src/http::post-http-headers path nonce data key secret)
+           `(("api-key" . ,key) ("api-sign" . ,api-sign))))))
+  (testing "with nonce + params data, evaluates to the correct headers alist"
+    (let* ((path "/0/private/Balance")
+           (nonce "1234567890123456789")
+           (key "01dB/y38ooyXBUWpS7XUNguXCk1trgN/LEj7FF8LgHmk3fcvX4dNQIFD")
+           (secret (concatenate 'string
+                                "YS/EXE3mfINjlKeegUVPT0uDUYkUX2Ed0OZp9dzCe1LO"
+                                "s+d9vZErAQKMY9o7WVQlTpvDodSlOONkZK7rngdJNw=="))
+           (data `(("pair" . "xbteur, xbtusd") ("nonce" . ,nonce)))
+           (api-sign (concatenate
+                      'string
+                      "lQjzgTnvmjJ9HMiucF+M3T7cI/VTYjZFptWDbf0uFG6RXLH"
+                      "sedsZaJ8n+HPn8G5exNwkzQC3phqXRqUi7g96Gw==")))
+      (ok (equalp
+           (cl-kraken/src/http::post-http-headers path nonce data key secret)
+           `(("api-key" . ,key) ("api-sign" . ,api-sign)))))))

[jonatack]

Note that this has been an issue with ECL only. SBCL, CCL, CLISP, and ABCL are all unaffected.