V3: [ALL] Extending the test-runner specification

[TheLostLambda]

Pretext

If this is dredging up some deep-seated trauma for you, you've probably been here before; my condolences. This issue is taking the lessons learned from that discussion and turning them into a fully fleshed-out proposal.

TL;DR

I'm proposing the addition of two new fields to the tests section of the existing test-runner interface: "cmd" and "expected".

The "cmd" key contains a cleaned-up version of the code run by the test suite. This code should most likely return some expected value and not contain any testing-suite cruft.

The "expected" key is whatever the "cmd" is supposed to return.

These new keys are important for displaying helpful information in the Web UI and supplement – not replace – the existing information exported by the test runners.

To bring your tracks up to spec, there are two ways forward:

1. Leave your test-runner as is and add a .meta/tests.json with an array of objects containing test "name"s plus the "cmd" and "expected" fields. We (probably I) will write a bit of tooling for merging keys from .meta/tests.json into the results.json generated by the test-runner – you'll just need to add a line to your bin/run.sh triggering this operation.
2. Generate the values of "cmd" and "expected" from within your test runner. You could parse doc-comments, the AST, or take some other approach. Both Common Lisp and C# have functional AST-based prototypes for doing this.

If you'd like a bit more context, boy have I got the essay for you...

The Problem

When using the new, in-browser coding in v3, it's important that the student is given detailed and easy to understand error messages when a given test fails. This means abstracting away as much of the testing framework as possible. While some test runners may be able to return the testing code run via the results.json output of the test-runner, this faces a couple of limitations. For example, in the Common Lisp Pokémon battle exercise, a single test looks like this:

(is (equal "A" (battle (make-test-pokemon "A")
                       (make-test-pokemon "B"))))

The test-runner then returns this code to the student when that test fails. The are a number of issues with this:

1. This exposes the testing framework via functions like is, something which is not part of the language standard or a concept being taught.
2. The student's code can be hidden behind helper functions like make-test-pokemon which is private to the test file.
3. Finally, there are numerous ways to test for equality in most languages, but the student shouldn't need to worry about what type of equality is involved – they should just have an output to match.

The Current System

The "beta" for parts of v3, research.exercism.io, solves this problem by having an additional JSON file (.meta/config.json) for each exercise with a list of test fields that look like the following:

"tests": [
    {
      "test": "TWIN-POKEMON-1",
      "name": "If identical Pokémon are battling, the first always wins",
      "cmd": "(battle (make-pokemon :name \"A\" :type 'fire :atk 5 :hp 40)\n        (make-pokemon :name \"B\" :type 'fire :atk 5 :hp 40))",
      "expected": "\"A\""
    },
    {
      "test": "TWIN-POKEMON-2",
      "name": "If identical Pokémon are battling, the first always wins",
      "cmd": "(battle (make-pokemon :name \"A\" :type 'grass :atk 5 :hp 40)\n        (make-pokemon :name \"B\" :type 'grass :atk 5 :hp 40))",
      "expected": "\"A\""
    },
// And so on...

Pros:

1. The code being returned to the student (in the cmd field) has all of testing-specific cruft stripped away. There are no library functions, no equality testing, no opaque helper functions.
2. The expected return value is made very explicit via the expected field. There is no work the student needs to do to find the proper return value.

Cons:

1. These files are massive and incredibly repetitive. The full .meta/config.json file shown above is nearly 100 lines. The repetitive nature is reflected in the fact that a bzip2 compressed .meta/config.json goes from 4.8K to just 801 bytes.
2. They are very error-prone. At the moment, these files are largely copy-pasted (both from the test file and other parts of the same .meta/config.json file. This has already resulted in production bugs (and, in my opinion, not by the fault of the maintainers).
3. The linkage between test runners and .meta/config.json is brittle. The test field of .meta/config.json must match the name field of the test-runner results.json file. If the name of a test is changed in the test suite, then .meta/config.json breaks.
4. The results.json and .meta/config.json files are also somewhat redundant in the message and cmd fields. Both fields are meant to relay to the user information about a test-failure.
5. Testing information is split between two files. If you want to change a test, you need to edit both the test suite and .meta/config.json – it becomes twice as much work to write a test file. Additionally, .meta/config.json doesn't have any validity checking, so a forgotten parenthesis or semi-colon will often go unnoticed.

The Solutions

Using .meta/tests.json

This new file (with details quite subject to change) would be a trimmed down version of the existing .meta/config.json used by research.exercism.io. The file will be a top-level array containing objects with "name", "cmd", and "expected" keys. The value of "name" in .meta/tests.json must match exactly the value of "name" within the results.json produced by the test-runner. It is this "name" match that enables the merging tool to associate the "cmd" and "expected" keys with individual tests.

An example .meta/tests.json with only two tests would look something like the following:

[
    {
      "name": "If identical Pokémon are battling, the first always wins",
      "cmd": "(battle (make-pokemon :name \"A\" :type 'fire :atk 5 :hp 40)\n        (make-pokemon :name \"B\" :type 'fire :atk 5 :hp 40))",
      "expected": "\"A\""
    },
    {
      "name": "If identical Pokémon are battling, the first always wins",
      "cmd": "(battle (make-pokemon :name \"A\" :type 'grass :atk 5 :hp 40)\n        (make-pokemon :name \"B\" :type 'grass :atk 5 :hp 40))",
      "expected": "\"A\""
    }
]

The merge tool will help the maintainers keep this file up to date by reporting whenever it can't find a matching "name" key in the output of the test runner. To accommodate tracks that would like to take a hybrid approach (using some .meta/tests.json and some auto-generation), the merge script will be capable of overwriting an arbitrary subset of the "cmd" and "expected" values returned by the test-runner.

Extending the Test-Runner

The auto-generation of the "cmd" and "expected" keys requires more upfront effort (as the test-runner would need to be updated) but reduces the amount of repetitive work in the long-term.

This can be done in a number of track-specific ways, like using comments:

; (battle (make-pokemon :name \"A\" :type 'fire :atk 5 :hp 40)
;         (make-pokemon :name \"B\" :type 'fire :atk 5 :hp 40))
; => A
(is (equal "A" (battle (make-test-pokemon "A")
                       (make-test-pokemon "B"))))

Or via a more sophisticated parsing of the AST, as Erik has done for C# and I've done for Common Lisp. Erik wrote up his efforts quite nicely here, so I won't rehash the whole process, but I will show off a couple of extra super-powers that the Common Lisp test-runner has.

Showing Off the Common Lisp Test-Runner

Disclaimer

Not only is this PoC code, but it's something I wrote about half a year ago – who knows what state of mind I was in then... The code will obviously be cleaned up before deployment. For the brave, here is the entirety of the Common Lisp test-runner.

A Bunch of Examples

Let's take a look at a shrunk (but fully functional) test file:

;; Some fluff handling imports and package setup
(load "common-lisp-1-a")
(ql:quickload :fiveam)
(defpackage common-lisp-1-a-test
  (:use :cl :fiveam :common-lisp-1-a)
  (:export :run-tests))
(in-package :common-lisp-1-a-test)

(import '(common-lisp-1-a::fire
          common-lisp-1-a::grass
          common-lisp-1-a::water))

;; Define and enter a new testing suite
(def-suite common-lisp-1-a-suite)
(in-suite common-lisp-1-a-suite)

;; This is a private helper "function" for writing tests. It is actually a macro
;; so that it can be expanded by the test runner to generate the `cmd` field.
(defmacro make-test-pokemon (name &optional (type 'fire) (atk 5) (hp 40))
  `(make-pokemon :name ,name :type ',type :atk ,atk :hp ,hp))

;; For reference, a function doing the same thing looks nearly identical:
(defun make-test-pokemon-fun (name &optional (type 'fire) (atk 5) (hp 40))
  (make-pokemon :name name :type type :atk atk :hp hp))

;; Here is a test, containing a number of assertions
(test twin-pokemon "If identical Pokémon are battling, the first always wins"
  (is-true (equal "A" (battle (make-test-pokemon "A")
                              (make-test-pokemon "B"))))
  (is-false (equal "A" (battle (make-test-pokemon "A")
                               (make-test-pokemon "B"))))
  (signals division-by-zero (/ 42 0))
  (finishes (princ "My only job is to not die!"))
  (is (equal "A" (progn (princ "Howdy there")
                        (battle (make-test-pokemon "A")
                                (make-test-pokemon "B"))))))

;; A helper function to be used by both the student and the test-runner
(defun run-tests (&optional (explain t))
  (let ((tests (run 'common-lisp-1-a-suite)))
    (if explain (explain! tests) tests)))

Example 1

For this first demonstration, let's just look at the first test:

(test twin-pokemon "If identical Pokémon are battling, the first always wins"
  (is-true (equal "A" (battle (make-test-pokemon "A")
                              (make-test-pokemon "B"))))
;; -- SNIP --
)

And it's corresponding JSON output:

{
   "name":"If identical Pok\u00E9mon are battling, the first always wins",
   "cmd":"(EQUAL \"A\"\n       (BATTLE (MAKE-POKEMON :NAME \"A\" :TYPE 'FIRE :ATK 5 :HP 40)\n               (MAKE-POKEMON :NAME \"B\" :TYPE 'FIRE :ATK 5 :HP 40)))",
   "expected":"T",
   "status":"pass",
   "message":null,
   "output":""
}

Here, the "name" key is the description given in the declaration of the surrounding test block, but it could just as easily be something like twin-pokemon.

The "cmd" key is the first interesting thing we see, showing off a somewhat unique feature of the Common Lisp test runner. The "cmd" is pretty-printed as follows:

(EQUAL "A"
       (BATTLE (MAKE-POKEMON :NAME "A" :TYPE 'FIRE :ATK 5 :HP 40)
               (MAKE-POKEMON :NAME "B" :TYPE 'FIRE :ATK 5 :HP 40)))

Curiously, that code doesn't actually show up anywhere within the test declaration! What the test-runner has done is automatically expand our private make-test-pokemon helper into code that the student can run directly – make-pokemon is a function they define. Additionally, the is-true testing function (not actually part of standard Common Lisp) is dropped. The result is a "cmd" that the student can copy-and-paste directly into their REPL for debugging.

The "expected" value of T is again not present in the code directly, but implied by the is-true assertion (T is the canonical "true" value in Common Lisp).

The rest of the keys are unchanged from the current test-runner spec. The test runner is also capable of interpreting other assertion types, as can be seen in the remaining examples.

Example 2

(is-false (equal "A" (battle (make-test-pokemon "A")
                             (make-test-pokemon "B"))))

{
   "name":"If identical Pok\u00E9mon are battling, the first always wins",
   "cmd":"(EQUAL \"A\"\n       (BATTLE (MAKE-POKEMON :NAME \"A\" :TYPE 'FIRE :ATK 5 :HP 40)\n               (MAKE-POKEMON :NAME \"B\" :TYPE 'FIRE :ATK 5 :HP 40)))",
   "expected":"NIL",
   "status":"fail",
   "message":"(EQUAL \"A\" (BATTLE (MAKE-TEST-POKEMON \"A\") (MAKE-TEST-POKEMON \"B\"))) returned the value T, which is true",
   "output":""
}



md5-a4061f9da3a7673ffdd356d2c19d7bc7



```json
{
   "name":"If identical Pok\u00E9mon are battling, the first always wins",
   "cmd":"(/ 42 0)",
   "expected":"Signals: DIVISION-BY-ZERO",
   "status":"pass",
   "message":null,
   "output":""
}



md5-dc56844a04ff890f05b4415615dc9408



```json
{
   "name":"If identical Pok\u00E9mon are battling, the first always wins",
   "cmd":"(PRINC \"My only job is to not die!\")",
   "expected":"Completes Execution",
   "status":"pass",
   "message":null,
   "output":"My only job is to not die!"
}



md5-5e6caa83ba6fc7fb2d4c72443c41d0ab



```json
{
   "name":"If identical Pok\u00E9mon are battling, the first always wins",
   "cmd":"(PROGN\n (PRINC \"Howdy there\")\n (BATTLE (MAKE-POKEMON :NAME \"A\" :TYPE 'FIRE :ATK 5 :HP 40)\n         (MAKE-POKEMON :NAME \"B\" :TYPE 'FIRE :ATK 5 :HP 40)))",
   "expected":"\"A\"",
   "status":"pass",
   "message":null,
   "output":"Howdy there"
}

This is perhaps the most complex example and uses the is assertion. Here the equal is actually stripped away and the "cmd" is separated from the "expected" value of "A". Only the progn (a standard Lisp way of grouping several expressions) is left as the command, entirely wiping any traces of test-framework from the output. Output to stdout is once again independently captured.

Conclusion

While Common Lisp might be one of the easier languages to automate the generation of "cmd" and "expected" keys in (perhaps ~70 additional lines of code), it's also been done in C# without too dire of an effort. However, for tracks where this proves impractical, the addition of a .meta/tests.json file will suffice. Most importantly, this change to the spec allows the individual tracks to decide which approach works best for them, as opposed to mandating a single approach.

If there are no major objections to this, I'll write up a PR to update The Interface and get started on the JSON merging tool (maybe in Rust, maybe in Nim, maybe in RISC-V assembly).

[coriolinus]

The most important thing, to me, is that tests.json should be entirely optional. Consider the output from a failure of a naive test in Rust:

---- times_seven_multiplies_by_seven stdout ----
thread 'times_seven_multiplies_by_seven' panicked at 'assertion failed: `(left == right)`
  left: `36`,
 right: `42`', src/lib.rs:7:5

It's not as perfectly explicit as the proposed extension here, but it's pretty clean already.

Now consider the output given a simple macro-based extension:

---- times_seven_multiplies_by_seven stdout ----
assertion failed: times_seven(6) == 42
thread 'times_seven_multiplies_by_seven' panicked at 'assertion failed: `(left == right)`
  left: `36`,
 right: `42`', src/lib.rs:22:5

This now produces the same information that tests.json would. You're going to have to take my word for it, but a macro-based solution like this is _much_ simpler to implement than this extension to the test runner. We could inject the appropriate code with a sed script.

---

Beyond the specific objection for Rust, it's not clear to me that the benefits outweigh the costs. I can't argue the pros you've listed, but every con you mentioned with respect to the current system also still applies, with the possible partial exception of points 3 and 4.

If it were up to me, I would not do this.

[TheLostLambda]

Hi @coriolinus ! Thanks for the comment and feedback! I certainly agree that some languages' test outputs are already quite clear. Rust in particular has some of the best error messages out of the languages I've worked with.

With that being said, I think that (and do correct me if I'm wrong, @iHiD ) the eventual addition of these keys somewhere was inevitable in V3? This PR is primarily just trying to avoid the excruciatingly manual research.exercism.io system when that day eventually comes. I think we've had a long discussion about why they were needed at some point, but Jeremy might need to help me find that. I suppose that the assumption these keys would be mandated is something I went into this PR with, but I'm happy to challenge that assumption as well.

I agree that only the automatic generation of these keys fixes all of the cons that I listed, and the tests.json file is a bit of a hacky stop-gap. The idea is more about flexibility though. For example, in the output you gave above, you could write a grep or sed script to extract the times_seven(6) and 42 bits of that output, then just add those in your JSON. Shifting the automatic generation of these keys to the test-runner doesn't mean it needs to be done on the Rust side of things. The tests.json file is certainly optional, but – so far as I understand – I don't think that the new keys will be.

If there is a tool that you feel would help a number of tracks with this autogeneration (say, parsing the captured stdout into cmd and expected fields), we can look into making that part of the tool.

Let me know what you think!

[coriolinus]

My preference would be for the web interface to capture and display the
native output to the greatest extent possible. If people learn a language
via exercism, then eventually they will need to interact with its tooling
without exercism as a mediating layer. That transition will be smoothest if
its output clearly resembles the native output.

If the problem is that students can't see the test file, then providing an
interface to display the test file is both less work and a better solution
than faking it with dedicated json.

On Sat, Oct 31, 2020, 11:05 Brooks Rady [e-mail] wrote:

Hi @coriolinus https://github.com/coriolinus ! Thanks for the comment
and feedback! I certainly agree that some languages' test outputs are
already quite clear. Rust in particular has some of the best error messages
out of the languages I've worked with.

With that being said, I think that (and do correct me if I'm wrong, @iHiD
https://github.com/iHiD ) the eventual addition of these keys
somewhere was inevitable in V3? This PR is primarily just trying to
avoid the excruciatingly manual research.exercism.io system when that day
eventually comes. I think we've had a long discussion about why they were
needed at some point, but Jeremy might need to help me find that. I suppose
that the assumption these keys would be mandated is something I went into
this PR with, but I'm happy to challenge that assumption as well.

I agree that only the automatic generation of these keys fixes all of the
cons that I listed, and the tests.json file is a bit of a hacky stop-gap.
The idea is more about flexibility though. For example, in the output you
gave above, you could write a grep or sed script to extract the
times_seven(6) and 42 bits of that output, then just add those in your
JSON. Shifting the automatic generation of these keys to the test-runner
doesn't mean it needs to be done on the Rust side of things. The
tests.json file is certainly optional, but – so far as I understand – I
don't think that the new keys will be.

If there is a tool that you feel would help a number of tracks with this
autogeneration (say, parsing the captured stdout into cmd and expected
fields), we can look into making that part of the tool.

Let me know what you think!

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/exercism/v3/issues/2535#issuecomment-719912521, or
unsubscribe
https://github.com/notifications/unsubscribe-auth/AB3V4TQ47KS5OZJ6NE3CWRTSNPOPFANCNFSM4S76RVMA
.