exercism / elm-test-runner

GNU Affero General Public License v3.0
3 stars 5 forks source link

Elm Track - Robot Simulator - All tests passed but not able to complete the exercise #33

Closed 552020 closed 2 years ago

552020 commented 2 years ago

I tried many times also to submit the solution through the CLI: all tests have been passed but it says 'failed' nonetheless.

image

github-actions[bot] commented 2 years ago

Hi and welcome to Exercism! :wave:

Thanks for opening an issue :slightly_smiling_face:

NobbZ commented 2 years ago

@exercism/haskell can you take a look?

552020 commented 2 years ago

I guess it has to do with my solution, cause I tried now with the example solution and it worked normally.

It seems like my code broke the page: I'm posting it therfore here down. It's dirty beginner spaghetti code. Sorry for that.

module RobotSimulator exposing
    ( Bearing(..)
    , Robot
    , advance
    , defaultRobot
    , simulate
    , turnLeft
    , turnRight
    )

type Bearing
    = North
    | East
    | South
    | West

type alias Robot =
    { bearing : Bearing
    , coordinates : { x : Int, y : Int }
    }

defaultRobot : Robot
defaultRobot =
    { bearing = North
    , coordinates = { x = 0, y = 0 }
    }

turnRight : Robot -> Robot
turnRight robot =
    let
        turnRightBearing =
            case robot.bearing of
                North -> East
                East -> South
                South -> West
                West -> North
    in
    { robot | bearing = turnRightBearing }

turnLeft : Robot -> Robot
turnLeft robot =
    let
        turnLeftBearing =
            case robot.bearing of
                North -> West
                East -> North
                South -> East
                West -> South
    in
    { robot | bearing = turnLeftBearing }

advance : Robot -> Robot
advance robot =
    case robot.bearing of
        North ->
            let
                previousCoordinates = robot.coordinates
                myY = robot.coordinates.y
            in
            { robot | coordinates = { previousCoordinates | y = myY + 1}}
        East ->
            let previousCoordinates = robot.coordinates
                myX = robot.coordinates.x
            in
            { robot | coordinates = { previousCoordinates | x = myX + 1}}
        South ->
            let previousCoordinates = robot.coordinates
                myY = robot.coordinates.y
            in
            { robot | coordinates = { previousCoordinates | y = myY - 1}}
        West ->
            let previousCoordinates = robot.coordinates
                myX = robot.coordinates.x
            in
            { robot | coordinates = { previousCoordinates | x = myX + 1}}

execute : Char -> Robot -> Robot
execute directionChar thisRobot =
            case directionChar of
                'A' -> advance thisRobot
                'L' -> turnLeft thisRobot
                'R' -> turnRight thisRobot
                _ -> thisRobot

simulate : String -> Robot -> Robot
simulate directions robot =
    let
        charList = String.toList directions
    in
    case charList of
        [] -> robot
        first :: rest ->
            execute first (simulate (List.foldl (++) "" (List.map (\x -> String.fromChar x) rest )) robot)
iHiD commented 2 years ago

@ErikSchierboom Could you take a look at this one pls?

ErikSchierboom commented 2 years ago

I've tested this locally, and the problem seems to be in the produced results.json file:

{
  "version": 3,
  "status": "fail",
  "tests": [
    {
      "name": "coordinates",
      "task_id": null,
      "status": "pass",
      "message": null,
      "output": null,
      "test_code": "Expect.equal {x = 0, y = 0} defaultRobot.coordinates"
    },
    {
      "name": "bearing",
      "task_id": null,
      "status": "pass",
      "message": null,
      "output": null,
      "test_code": "Expect.equal North defaultRobot.bearing"
    },
    {
      "name": "coordinates",
      "task_id": null,
      "status": "pass",
      "message": null,
      "output": null,
      "test_code": "Robot anyBearing {x = -1, y = 1} |> ..coordinates\n |> Expect.equal {x = -1, y = 1}"
    },
    {
      "name": "bearing",
      "task_id": null,
      "status": "pass",
      "message": null,
      "output": null,
      "test_code": "Robot South anyCoordinates |> ..bearing\n |> Expect.equal South"
    },
    {
      "name": "step 0",
      "task_id": null,
      "status": "pass",
      "message": null,
      "output": null,
      "test_code": "Robot North {x = 0, y = 0} |> advance |> ..coordinates\n |> Expect.equal {x = 0, y = 1}"
    },
    {
      "name": "step 1",
      "task_id": null,
      "status": "pass",
      "message": null,
      "output": null,
      "test_code": "Robot North {x = 0, y = 0} |> advance |> ..bearing\n |> Expect.equal North"
    },
    {
      "name": "step 2",
      "task_id": null,
      "status": "pass",
      "message": null,
      "output": null,
      "test_code": "Robot East {x = 0, y = 0} |> advance |> ..coordinates\n |> Expect.equal {x = 1, y = 0}"
    },
    {
      "name": "step 3",
      "task_id": null,
      "status": "pass",
      "message": null,
      "output": null,
      "test_code": "Robot East {x = 0, y = 0} |> advance |> ..bearing\n |> Expect.equal East"
    },
    {
      "name": "step 0",
      "task_id": null,
      "status": "pass",
      "message": null,
      "output": null,
      "test_code": "Robot South {x = 0, y = 0} |> advance |> ..coordinates\n |> Expect.equal {x = 0, y = -1}"
    },
    {
      "name": "step 1",
      "task_id": null,
      "status": "pass",
      "message": null,
      "output": null,
      "test_code": "Robot South {x = 0, y = 0} |> advance |> ..bearing\n |> Expect.equal South"
    },
    {
      "name": "step 2",
      "task_id": null,
      "status": "pass",
      "message": null,
      "output": null,
      "test_code": "Robot West {x = 0, y = 0} |> advance |> ..coordinates\n |> Expect.equal {x = -1, y = 0}"
    },
    {
      "name": "step 3",
      "task_id": null,
      "status": "pass",
      "message": null,
      "output": null,
      "test_code": "Robot West {x = 0, y = 0} |> advance |> ..bearing\n |> Expect.equal West"
    },
    {
      "name": "coordinates",
      "task_id": null,
      "status": "pass",
      "message": null,
      "output": null,
      "test_code": "Robot North {x = 0, y = 0} |> simulate \"LAAARALA\" |> ..coordinates\n |> Expect.equal {x = -4, y = 1}"
    },
    {
      "name": "bearing",
      "task_id": null,
      "status": "pass",
      "message": null,
      "output": null,
      "test_code": "Robot North {x = 0, y = 0} |> simulate \"LAAARALA\" |> ..bearing\n |> Expect.equal West"
    },
    {
      "name": "coordinates",
      "task_id": null,
      "status": "pass",
      "message": null,
      "output": null,
      "test_code": "Robot East {x = 2, y = -7} |> simulate \"RRAAAAALA\" |> ..coordinates\n |> Expect.equal {x = -3, y = -8}"
    },
    {
      "name": "bearing",
      "task_id": null,
      "status": "pass",
      "message": null,
      "output": null,
      "test_code": "Robot East {x = 2, y = -7} |> simulate \"RRAAAAALA\" |> ..bearing\n |> Expect.equal South"
    },
    {
      "name": "coordinates",
      "task_id": null,
      "status": "pass",
      "message": null,
      "output": null,
      "test_code": "Robot South {x = 8, y = 4} |> simulate \"LAAARRRALLLL\" |> ..coordinates\n |> Expect.equal {x = 11, y = 5}"
    },
    {
      "name": "bearing",
      "task_id": null,
      "status": "pass",
      "message": null,
      "output": null,
      "test_code": "Robot South {x = 8, y = 4} |> simulate \"LAAARRRALLLL\" |> ..bearing\n |> Expect.equal North"
    }
  ]
}

The fail status should be present if one or more tests have the fail status. In this case, there are no tests that have the fail status, so either the fail status is calculated incorrectly, or some tests are missing.

ErikSchierboom commented 2 years ago

CC @ceddlyburge @mpizenberg

mpizenberg commented 2 years ago

@552020 @ErikSchierboom thanks for the report. We are indeed aware of this bug. This is related to our test parser that extracts the test code for the test runner report. It has been fixed as seen in https://github.com/exercism/elm-test-runner/issues/29. This will be rolled out as soon as I take some time updating the test runner, which I suspect is next week.

ErikSchierboom commented 2 years ago

Excellent!

mpizenberg commented 2 years ago

Hum, my bad, the fix was done by removing the let expression in the robot simulator test. The support for let expression in the tests is not done yet as registered in that issue https://github.com/exercism/elm-test-runner/issues/30.

I'm reopening this issue then because It should be solved since the exercise was updated (https://github.com/exercism/elm/pull/463). Is it possible @552020 that you are still running the tests of the old version (previous to our fix), how could we check that @ErikSchierboom ?

ErikSchierboom commented 2 years ago

@mpizenberg I don't think it is fixed. I've upgraded the exercise to the latest version (which you can do on the website) and tested it in the online editor, and found the same issue.

jiegillet commented 2 years ago

I took a look, there is definitely a problem, and it's not exactly the same as before. There are 26 tests in the exercise, but only 18 show up in results.json and the names are all messed up.

Basically, the test code extractor fails on the 8 tests in describe "right" and describe "left", because of this line

|> List.indexedMap (\i e -> test ("step " ++ String.fromInt i) (\() -> e))

The code extractor cannot work with test ("step " ++ String.fromInt i) it can only deal with a single string literal (like test "step 1"). Changing the code extractor to deal with this would be very complex, and even if it did manage to extract the code, it would display e, which is fairly useless, so I think we should rewrite those tests instead.

jiegillet commented 2 years ago

@552020 the tests have been updated, please try again.

You should get some failing tests this time, because your simulate reverses the order of operations. Let me know if you need some mentoring once you pass the tests :)

ceddlyburge commented 2 years ago

Thanks for reporting / fixing this everyone, I've been away on holiday . Should we update this issue to check that the test runner works with the test code do you think? https://github.com/exercism/elm/issues/412

552020 commented 2 years ago

Thank you everyone for fixing this. @jiegillet I run the updated exercise with my old spaghetti code and it works as it is. I will open nonetheless a mentoring request, cause I struggled a lot with this one.

Probably this is not the right place for this question and I should open another issue for it: sometimes, as a learner, you would need mentoring before you pass all the tests. Probably this is not possible cause you want to avoid too many mentoring requests but in certain cases, it would help to be mentored before you pass all the tests. I'm stuck for example on the Pythagorean Triplets challenge. I solved it conceptually with a brute force approach based on recursion which is too expensive memory-wise and I get a stackoverflow on the test with the big numbers. I tried to refactor it but my alternative solutions give similar problems and now I'm trying to understand the maths behind the triplet and deliver a mathematical solution. I think that for cases like these, where the learner came up with a valid working solution with some flaws it would be meaningful to allow him to open a mentoring request. If you think that is issue/proposal is worth to be discussed I can open a separate issue.

ErikSchierboom commented 2 years ago

Thanks for fixing @jiegillet !

552020 commented 2 years ago

And by the way, what I wrote is not true. My spaghetti code solution throws 4 errors, so everything is fine. I forgot that I've copied the example solution in the editor.

jiegillet commented 2 years ago

@552020 I'm glad that your code fails now (lol). I will close the issue then.

Concerning receiving mentorship, there is trick: if you submit via the CLI you should be able to ask for it even if the tests fail, but your request is a common one, please check exercism/exercism to see if there is an issue open already.

Concerning pythagorean triples, my favorite way to solve the exercise very efficiently is via this mathematical trick, (there are also a bunch ways on that link).

552020 commented 2 years ago

@jiegillet Thank you for the trick suggestion. I found a forgotten open issue/suggestion about it: https://github.com/exercism/exercism/issues/6187