Nested assertions not reported

[bobbicodes]

Issues have been filed like exercism/clojure#420 and exercism/clojure#421 which indicate that the test runner is not functioning properly. This was due to the limited scope of test suites it was tested against before release, and the richer use of nested assertions we're using in the Concept exercises.

There is still further implementation to be done to get it into a "complete" state, as it currently only handles individual deftest forms (see examples below) as was common in practice exercises. Forms containing nested assertions are tested properly, blocking the student if any assertions fail, but these nested assertions are not included in the results, causing mass confusion and hysteria.

One [cheesy] solution would be to modify the test suites to only contain individual deftests. (see exercism/clojure#422) This would not be ideal, but depending on how quickly the test runner can be upgraded it may be the quickest achievable path to a fully working track.

The work needing to be done is significant but doable. I intend to use this issue as an architectural log, to help gather my thoughts about how the implementation works and communicate them in a manner that can facilitate open collaboration.

[bobbicodes]

The test runner works by using clojure.test, Clojure's built-in testing framework. However, the amount of twisting it into shape necessary to get it to do what we need is pushing me to try a more direct approach.

It was no problem overriding the test reporting methods to output proper maps of the passes, fails and errors. However, they are reported out of order! This could be overcome by using test fixtures, but instead I opted to go straight to the source, and parse the test file and extract the tests in the order defined. Enter rewrite-clj. It worked spectacularly well, and made it trivial to also extract the assertion code to be provided in the output to the student.

My proposed solution to the current problem is to implement a recursive evaluator to traverse the test file's parse tree (a zipper data structure) and take over the job of clojure.test almost completely, merely using it to evaluate individual is assertions. We can then eliminate the custom reporting methods, because we will no longer be pushing it beyond the common use case to run the tests the way we want, we will be testing each assertion directly from the parse tree and building a table of results from that.

See the rewrite-clj docs for a guide to zippers and how they are used to navigate the parse tree.
For context into how the source and test files are loaded and processed, see the test-runner.clj babashka script.

[bobbicodes]

The simplest Clojure test suite looks like this:

(ns leap-test
  (:require [clojure.test :refer [deftest is]]
            leap))

(deftest year-not-divisible-by-4
  (is (not (leap/leap-year? 2015))))

(deftest year-divisible-by-2-but-not-4
  (is (not (leap/leap-year? 1970))))

(deftest year-divisible-by-4-but-not-100
  (is (leap/leap-year? 1996)))

This is the actual code that was used for the development of the current test runner, the form used by most practice exercises, and the same code used in CI to test the test runner. This is the shape of the data that it expects. It parses the file, finds every deftest, and everybody's happy.

[bobbicodes]

The problem arises when we get something like this:

(deftest prep-time-test
  (testing "Preparation time in minutes"
    (testing "for one layer"
      (is (= 2 (lucians-luscious-lasagna/prep-time 1))))
    (testing "for multiple layers"
      (is (= 8 (lucians-luscious-lasagna/prep-time 4))))))

Here's where we need to recursively traverse the tree from the deftest node to grab each child node, going deeper if it's a testing node and concatenate each string until we hit an is node, then we could say return a sequence of test strings mapped to their evaluation results like this:

[{:test-string "Preparation time in minutes for one layer"
  :results [true]}
 {:test-string "Preparation time in minutes for for multiple layers"
  :results [true]}]

There can be any number of testing forms under a deftest, and any number of is forms under a testing. Or like the simple case above, an is need not be inside a testing. In that case, it doesn't have a :test-string and will be associated with the name of the deftest.

Here is the example from the clojure.test API. We'll parse it into a zipper and call it deftest-loc (a loc is a location in a zipper, a node in the tree):

(def deftest-loc
  (z/of-string
"(deftest arithmetic-test
   (testing \"Arithmetic\"
     (testing \"with positive integers\"
       (is (= 4 (+ 2 2)))
       (is (= 7 (+ 3 4))))
     (testing \"with negative integers\"
       (is (= -4 (+ -2 -2)))
       (is (= -1 (+ 3 -4))))))"))

Now descend into the tree and extract the outer testing node:

(def outer-testing
  (-> deftest-loc z/down z/right z/right))

We'll define some predicates for identifying nodes:

(defn testing?
  "Returns true if the given node is a `testing` form."
  [loc]
  (= (symbol 'testing) (-> loc z/down z/sexpr)))

(defn assertion?
  "Returns true if the given node is an `is` form."
  [loc]
  (= (symbol 'is) (-> loc z/down z/sexpr)))

(defn assertion-next?
  "Returns true if the node to the right is an `is` form."
  [loc]
  (= (symbol 'is) (-> loc z/right z/down z/sexpr)))

(defn outer-testing?
  "Returns true if the given node is an outer `testing` form."
  [loc]
  (and
   (= (symbol 'testing) (-> loc z/down z/sexpr))
   (= (symbol 'testing) (-> loc z/down z/right z/right z/down z/sexpr))))

The following gives us the test-strings:

(defn prefix-str 
  "Takes an outer testing node.
   Returns the string of the first testing form."
  [loc]
    (-> loc z/down z/right z/sexpr))

(defn test-strings
  "Traverses a zipper from a nested testing node 
   and returns a sequence of its test strings."
  [loc]
  (loop [loc loc
         test-strings []]
    (cond
      (nil? loc) test-strings
      (outer-testing? loc) (recur (-> loc z/down z/right z/right) test-strings)
      (testing? loc) (recur (-> loc z/right) 
                            (conj test-strings (str (prefix-str node) " " 
                                                    (-> loc z/down z/right z/sexpr)))))))

(test-strings outer-testing)
;;=> ["Arithmetic with positive integers"
;;       "Arithmetic with negative integers"]

[bobbicodes]

Given an inner testing node, this will find and evaluate its assertions:

(defn assertion-true? 
  "Returns true if the given assertion-loc is true."
  [loc]
  (eval (-> loc z/sexpr)))

(def inner-testing
  (-> outer-testing z/down z/right z/right))

(defn results 
  "Traverses a zipper from an inner testing node
   and returns a sequence of its assertion results."
  [loc]
  (loop [loc loc
         assertions []]
    (cond
      (nil? loc) assertions
      (testing? loc) (recur (-> loc z/down z/right z/right) assertions)
      (assertion? loc) (recur (-> loc z/right) (conj assertions (assertion-true? loc))))))

(results inner-testing)
;;=> [true true]

Now we just need to compose these and we're done.

[bobbicodes]

The following evaluator seems to handle any deftest form:

(def deftest-loc3
  (z/of-string
"(deftest signal-prisoner-test
  (testing \"Can signal prisoner if archer is sleeping and prisoner is awake\"
    (is (= true (annalyns-infiltration/can-signal-prisoner? false true))))
  (testing \"Cannot signal prisoner if\"
    (testing \"archer is awake and prisoner is sleeping\"
      (is (= false (annalyns-infiltration/can-signal-prisoner? true false))))
    (testing \"archer and prisoner are both sleeping\"
      (is (= false (annalyns-infiltration/can-signal-prisoner? false false))))
    (testing \"archer and prisoner are both awake\"
      (is (= false (annalyns-infiltration/can-signal-prisoner? true true))))))"))

(let [deftest deftest-loc3]
  (loop [loc deftest
         prefix-string ""
         test-strings []
         results []]
    (cond
      (deftest? loc)
      (recur (-> loc z/down z/right z/right)
             prefix-string test-strings results)
      (outer-testing? loc)
      (recur (-> loc z/down z/right z/right)
             (-> loc z/down z/right z/sexpr)
             test-strings results)
      (testing? loc)
      (recur (-> loc z/down z/right z/right)
             prefix-string
             (conj test-strings (str/trim (str prefix-string " " (-> loc z/down z/right z/sexpr))))
             (conj results []))
      (and (assertion? loc) (assertion-next? loc))
      (recur (-> loc z/right)
             prefix-string
             test-strings
             (conj results [(assertion-true? loc)]))
      (assertion? loc)
      (recur (-> loc z/up z/right)
             prefix-string
             test-strings
             (conj (vec (butlast results))
                   (conj (vec (last results)) (assertion-true? loc))))
      :else
      {:test-name (-> deftest z/down z/right z/sexpr str)
       :results (vec (remove empty? results))
       :test-strings test-strings})))

;;=> {:test-name "signal-prisoner-test",
         :results [[true] [true] [true] [true]],
         :test-strings
         ["Can signal prisoner if archer is sleeping and prisoner is awake"
          "Cannot signal prisoner if archer is awake and prisoner is sleeping"
          "Cannot signal prisoner if archer and prisoner are both sleeping"
          "Cannot signal prisoner if archer and prisoner are both awake"]}

[ErikSchierboom]

👍

[ghost]

Hope that I quick can test things.
WIth the pr that made all test one by one , I still cannot see why things are failing

[miridius]

Hey so I'm not sure if we really need to reinvent the wheel here and recreate clojure.test. In #42 I've updated the existing solution to support multiple is forms inside a deftest, although it currently assumes every top level form is an assertion I think it would be pretty easy to update it to handle nesting.

If we do decide that clojure.test is too restrictive then we could always look for a different off the shelf test runner instead of writing a new one!

[bobbicodes]

Fixed by #42