[ALL] A proposed extension to the test-runner spec

[TheLostLambda]

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.

Some Solutions

Automatically generating the .meta/config.json file

This certainly isn't a bad idea, but isn't a magical solution either. How do you keep the .meta/config.json up to date with the test suite? Do you add it as an automatic CI option? While that's an option, it puts a lot of faith in the automated generator. On that subject, these automated generators would almost-certainly be track specific so there would likely need to be a different CI check for every track (disclaimer: I'm not a CI expert).

My Proposed Solution: Extending the test-runner spec

I think that a lot of the aforementioned problems could be mitigated if the functionality of the.meta/config.json file were integrated into the test runner.

This would allow each individual track to decide how best to generate human-friendly errors

Some options are:

Continue using .meta/config.json

There are some tracks where it's somewhat impractical to do any automated error generation. All test runners already have the ability to write JSON files, so it's reasonable to assume that a number of them can also read JSON. The test runner simply reads some sort of config.json and writes that directly (or with some formatting) to results.json.

Generation via Comments

This idea is from @iHiD! It was suggested that tests could be annotated with a comment containing some information about the correct message to show. Something like:

; (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"))))

While this still involves some duplication, all of the test code is in the same place. If you make a change to the test, it's easy enough to edit the line right above it. In some languages, you could easily uncomment and test this code is valid as well.

Fully Automated Generation

This would be different for every track and likely involve some AST parsing (unless your language's syntax is its AST tree, Lisp FTW 😉). Implementing this would mean no more duplication of testing information.

A Hybrid Approach + Literally Anything Else

Maybe you are able to automatically generate messages in 90% of cases but need to fall-back to a comment-annotation or config.json, that's not a problem! If you want to do things differently, you only need to work with the people in your track and don't need to write massive Github issues like this.

The real advantage of shifting this responsibility to the test-runner is flexibility

Some Questions I Still Have

Should we change the tests field of results.json? It currently looks like this:

// CURRENT
"tests": [
  {
    "name": "Test that the thing works",
    "status": "fail",
    "message": "Expected 42 but got 123123",
    "output": "Debugging information output by the user"
  }
]

Should we add the cmd and expected fields from the .meta/config.json? Something like this?

"tests": [
  {
    "name": "Test that the thing works",
    "status": "fail",
    "cmd": "NumberBox.gimme42()"
    "expected": "42"
    "message": "Expected 42 but got 123123",
    "output": "Debugging information output by the user"
  }
]

Or is that just more silly redundancy and this is best?

"tests": [
  {
    "name": "Test that the thing works",
    "status": "fail",
    "message": "Error when running `NumberBox.gimme42()`: Expected 42 but got 123123",
    "output": "Debugging information output by the user"
  }
]

I'd love to know what people think! Sorry for the marathon GitHub issue!

[SleeplessByte]

FWIW, this is exactly what JavaScript had, but @iHiD had reasons why that wasn't "enough"; I don't recall those reasons, but it was probably important. Jeremy, do you still recall those convo's we had?

[iHiD]

Thanks for the writeup, @TheLostLambda! :)

I'm fine with moving the output into the results.json. I like it and think it's a good idea. However, I do think it makes the test runners much more complicated. I would therefore want to keep the current option in play somehow, albeit through maybe passing the config.json into the test-runner so it can just extract keys etc.

---

Should we add the cmd and expected fields from the .meta/config.json?

Yes :)

Or is that just more silly redundancy and this is best?

No (see below)

Jeremy, do you still recall those convo's we had?

Yes. Look at the research UI. The different parts of this (cmd, expected, actual) are in demarcated areas. I want this level of granular control within the UI so that our product/UX team can work out how it's best displayed for student clarity, rather than track teams making their own per-track decisions.

[SleeplessByte]

Oh yeah, I definitely want a standard (and not track-specific decisions) but you didn't want the output.json to have this; are we coming back from that?

As for JS/TS, we can generate all the required fields from a custom test runner/helper.

[TheLostLambda]

@iHiD I think that adding cmd and expected to the results.json works for me!

Also, I can develop a script that takes the .meta/config.json and merges it with results.json file rather easily. That means that, if a test runner doesn't want to fuss with this kind of message forming, they can just call: ./merge_tests.sh or something in the last line of their bin/run.sh and be done with it.

Do you reckon that would be a good compromise? We would get all of the additional flexibility of this proposal with only a one-line change for those who wish to continue with the .meta/config.json approach.

[iHiD]

but you didn't want the output.json to have this; are we coming back from that?

I didn't want the output to be in the test-runner. In this proposal it's still in the tests, just extracted (e.g. if it's in the comments, or via AST). The crucial thing is that we can change the output without redeploying the test-runner.

That means that, if a test runner doesn't want to fuss with this kind of message forming, they can just call: ./merge_tests.sh or something in the last line of their bin/run.sh and be done with it.

This would make me happy, yes. Let's get a couple of POCs (@ErikSchierboom maybe does C# and F# which are quite different internally, and you do CL) and let's see if they're as easy as we hope :)

[TheLostLambda]

Sounds good! I'll give it a go sometime this week I hope!

[wolf99]

Sidebar question that may have been covered in other discussions.
Re

This means abstracting away as much of the testing framework as possible

Is it not a benefit that a student would learn what output from a testing framework idiomatic to that language would look like , and learn to parse it? If is is non-trivial, should track's explain it (maybe as part of the first concept)?

[iHiD]

If is is non-trivial, should track's explain it (maybe as part of the first concept)?

I'd say a track should maybe explain this, yes. But only once the person is comfortable in a language. The test-suite is going to contain loads of extra ideas/concepts that a student won't be familiar with at the start, so we don't want to have to expose them to that until they're comfortable. I've personally found having to decipher test-suites a real blocker to learning certain languages via Exercism.

[SleeplessByte]

Let's not forget that Testing is a concept on its own.

[ErikSchierboom]

Definitely ☝️ . I can even remember a Ruby exercise in which I couldn't even properly read the tests, even though I did have some experience in Ruby.

[ErikSchierboom]

TL;DR I've written a PoC for the C# test runner that outputs the command, expected value and human-friendly test name automatically.

Introduction

I've worked on a PoC last week to see if I could modify the C# test runner to output all required information directly in the results.json file generated by the test runner. In other words: could I modify the C# test runner such that the information that was already in the test code (expected value, code that is tested) would be automatically output to the results.json file? I've managed to get this working. Here is how it works.

Implementation

• The C# test runner directly calls the test framework as a library. This allows the C# test runner to get a reference to the metadata for the test method that was executed.
• The C# test runner has access to the syntax trees of the code that is run. This includes the test methods, so we can now use the test method metadata from the test runner to find the correspond syntax tree node.
• By examining the test method's syntax node, the test runner can infer the expected value (which is output as a string). It does this by looking at the type of assertion that is used. Here are some examples:

Assertion	Expected
Assert.Equal("test", "String.Hello()")	"\"test\""
Assert.Equal(new DateTime(2020, 2, 22), DateTimes.BirthDate())	"new DateTime(2020, 2, 22)"
Assert.True(Booleans.IsLarge(2))	"true"
Assert.False(Booleans.IsLarge(7))	"false"
Assert.InRange(Integers.First(2, 3), 5, 10)	">= 5 && <= 10"

I've not made this exhaustive, but the PoC code shows how easy it is to do this.

• • By examining the test method's syntax node, the test runner can infer the cmd value (which is output as a string). In most cases, this means just returning the code used to pass the actual value (which is usually a method call):

Assertion	Cmd
Assert.Equal("test", "String.Hello()")	"String.Hello()"
Assert.Equal(new DateTime(2020, 2, 22), DateTimes.BirthDate())	"DateTimes.BirthDate()"
Assert.True(Booleans.IsLarge(2))	"Booleans.IsLarge(2)"
Assert.False(Booleans.IsLarge(7))	"Booleans.IsLarge(7)"
Assert.InRange(Integers.First(2, 3), 5, 10)	"Integers.First(2, 3)"

However, in some cases there will be some setup code involved too, so the PoC also takes them into account:

public void TestInstance()
{
   var car = new Car();
   car.Speedup();
   car.Brake();
   Assert.Equal(10, car.Speed());
}

This code will result in the following command (effectively everything but the assertion and the expected value: "var car = new Car();\ncar.Speedup();\ncar.Brake();\ncar.Speed()".

• The C# test runner tries to convert test method names to a human-readable format:

Test method	Name
public void Hello()	"Hello"
public void Add_should_add_numbers()	"Add should add numbers"

This works due to the test methods using a consistent naming format, and having a library that allows converting strings to a normal, human-readable format.

Difficulty

Writing the above code wasn't that hard (2,5 hours work or so), but that was mainly due to the fact that the C# test runner had several things that work in its advantage:

• The test framework can be called as a library.
• The test framework provides easy access to the method names of the executed tests.
• The C# compiler allows us to find the syntax node for a method by its name easily.
• Specific parts of the syntax node can easily be accessed and worked with.
• The C# compiler allows syntax nodes to be output as (C#) code again.

Furthermore, I had some experience with C# AST's having already written the C# representer and analyzer. As other tracks will also have to write a representer, that knowledge will be essential if you want to do something similar to the C# PoC. It could also work the other way around, where writing a representer becomes easier having worked with AST's in the test runner.

Alternatives

Not all languages will have the above options available. I suspect converting an AST back to code is not built-into all compiler frameworks, but there may be libraries available to do this. An alternative to do this is to do some text processing on the source code of the test suite, to extract the source code as text and do text manipulation. This can be tedious to write and error-prone.

As an alternative to any text-based processing, one could also look into adding comments to test methods and then using that as the source. As an example, in C# methods can be annotated with structured XML comments that can relatively easily be accessed in code:

/// <summary>My test method</summary>
/// <returns>2</returns>
/// <example>
/// var car = new Car();
/// car.Speed();
/// </example>
public void MyTestMethod()
{
   var car = new Car();
   Assert.Equal(2, car.Speed());
}

If there is no structured comment option, one could consider defining one.

The text- and comment-based options are less than ideal, as they are either error-prone (text-based) or possibly out of sync with the actual code (command-based).

Manual options

For the above reasons and the fact that we don't want to force tracks to output this information automatically, we should have some manual option available too (the .meta/config.json option), with a merge that can be done between the two outputs.

N.B. the C# test runner PoC is PoC code, which means that it hasn't been cleaned-up/refactored.

[wolf99]

Very cool Erik! 🚀

I feel like this part is critical though, for the tracks that may have the difficulties that Erik has mentioned:

For the above reasons and the fact that we don't want to force tracks to output this information automatically, we should have some manual option available too (the .meta/config.json option), with a merge that can be done between the two outputs.

[verdammelt]

+1 - if the track maintainers can and want to write code to generate this result data - great. Otherwise they can hard code it into config.json.

[SleeplessByte]

@iHiD and I will be working on a comments based (annotations) approach to this, that can be used across almost all tracks. I think that proves that this proposal has merit.

I'm behind both.