poor messages

[petelomax]

Noticed on the game of life task:
Failed tests give incomplete messages, eg "failed: dead cells with three live neighbors become alive, expected: {"
Compilation and runtime errors don't give line numbers either.

(I'm actually not quite sure anymore, but I think those were from running it online.)

[axtens]

Something you might want to discuss with @ErikSchierboom as to the best way to report errors

[ErikSchierboom]

I don't think I'm necessarily the expert here :)
What you want is for the student to be able to quickly figure out what is wrong with their code. So line numbers, bits of their code and error messages all help with that. As to what's possible in Euphoria, I don't know.

[petelomax]

Running game of life in the CLI (compiler error):

F:\misc\exercism\euphoria\game-of-life>eutest

interpreting t_game-of-life.e:
F:\misc\exercism\euphoria\game-of-life\game-of-life.ex:7
<0132>:: Syntax error - expected to see possibly ']', not ','
      integer mxy = m[x,y], nxy = -mxy
                       ^

FAILURE: t_game-of-life.e EUPHORIA error with status 1

Test results summary:
    FAIL: t_game-of-life.e
Files (run: 1) (failed: 1) (0% success)

Running same online:

We received the following error when we ran your code:
<0132>:: Syntax error - expected to see possibly ']', not ','

Running game of life in the CLI (incorrect output):

  failed: live cells with zero live neighbors die, expected: {
  {0,0,0},
  {0,0,0},
  {0,0,0}
} but got: {
  {0,0,0},
  {0,1,0},
  {0,0,0}
}

Same online:

failed: live cells with zero live neighbors die, expected: {

So the question is what needs changing to get the extra info to show up online?

[ErikSchierboom]

This is the relevant code: https://github.com/exercism/euphoria-test-runner/blob/main/bin/run.ex

[petelomax]

@axtens can I leave that in your capable hands?

[axtens]

Yep. You can. It is a thing and worth the think

[petelomax]

I also think you'd get friendlier messages from t_space-age.e if you did it like this instead

procedure is_close(sequence desc, atom val1, atom val2)
    val1 = floor(val1*100)/100
    test_equal(desc,val1,val2)
end procedure
is_close("age on Earth", ageOn("Earth", 1000000000), 31.68)
is_close("age on Mercury", ageOn("Mercury", 2134835688), 280.87)
is_close("age on Venus", ageOn("Venus", 189839836), 9.77)
is_close("age on Mars", ageOn("Mars", 2129871239), 35.88)
is_close("age on Jupiter", ageOn("Jupiter", 901876382), 2.40)
is_close("age on Saturn", ageOn("Saturn", 2000000000), 2.15)
is_close("age on Uranus", ageOn("Uranus", 1210123456), 0.45)
is_close("age on Neptune", ageOn("Neptune", 1821023456), 0.35)
test_false("invalid planet causes error", ageOn("Sun", 680804807))

However I'm not at all certain whether truncating (as above) or rounding (as current) would be best, and suspect "the Phix way" is subtly incompatible with Euphoria, at least the way I'd naturally do it...

[axtens]

I'm quite okay with you forking the repo and raising a PR. I'm not sure who authored our current one, but it wasn't me. Might have been @glennj or possibly @BNAndras

[BNAndras]

I can make that fix later today.

[BNAndras]

Upon review, The problem I have with this approach is that some of the expected values will deviate slightly from the canonical ones. The deviation is occurring because of an implementation detail. Generally, the languages I've worked with had a way to compare floating point values with a specified tolerance. That'd be a more robust way of approaching this if we could suggest changes to the unit test library.

[axtens]

Not use eutest?? Hmm. Maybe we could do some shims. @petelomax is good at that. Can you unpack your suggestion a bit further?

[petelomax]

I did in fact change the Phix test_equal() to allow the default tolerance of 1e-9 to be overidden. While making a similar change to Euphoria might not be technically difficult, getting a new version shipped with that change in it would be, as things stand..

I suspect though what you really have issues with here is the val1 = floor(val1*100)/100 line above, my intention was you could replace that with round() or even a couple of sprintf(), and for that matter using the existing test but modifying the description with say desc = sprintf("%s: got:%.2f, expected:%.2f",{desc,val1,val2}) and still using a test_true() inner call should be fine.

[axtens]

We have two separate issues in this thread: the precision issue in space-age and the modification of the test-runner to better reflect where an error was encountered. Seeing as right here is #75 (to which I've been assigned), how about the rest of you create a separate issue, put a link to it here, and go over there and discuss it.