Extracted record accessor have an extra .

[jiegillet]

Here is an snippet of the test_code.json running on robot-simulator (but I see this everywhere)

  {
    "name": "coordinates",
    "testCode": "Robot anyBearing {x = -1, y = 1} |> ..coordinates\n |> Expect.equal {x = -1, y = 1}"
  }

For some reason there is an extra dot in ..coordinates. No idea where it's from.

[mpizenberg]

@ceddlyburge when you have time to check this one can you have a look?

[ceddlyburge]

This happens at this line of code, so I think it must be a problem with elm-syntax.

elm-test-runner/extract-test-code/src/ExtractTestCode.elm

Line 118 in 121f9be

writeExpression (Node.Node emptyRange code)

I'll create an issue / check if it has been fixed. I think there is a new version either out now or coming soon as well, so we might want to wait for that ..

[ceddlyburge]

I think this bug was fixed in this commit:
stil4m/elm-syntax@b7e66d7
So we should be able to update to the latest version of elm-syntax to get the fix

[ceddlyburge]

@jiegillet : can you add this to your currently open pull request and see if it works?
Thanks, Cedd

[jiegillet]

First of all: nice, this version solves the issue!

Second: OMG, this lead me to find a huge issue! @mpizenberg @ceddlyburge

In my PR, I changed the code to group merge the tests and the code snippets. However, whenever there are duplicate names, the jq script merges those. robot-simulator has 20 tests, but the final result only has 4 because most of the tests are named "bearing" or "coordinates".
This can't be fixed by a better jq script since there is no way to know which test goes with which snippet. I can think of 2 things to do:

1. Require all tests to have unique names in the Elm repo
2. Change both elm-test-rs and extract-test-code so that the name becomes "description 1 > Description 2 > ... > Name" or something like that. These should be unique too of course.

What do you think? We can't merge my PR before we fix that.

[mpizenberg]

Oh damn, I remember we discussed with @ceddlyburge about the fact that we didn't need the ExerciseName in the full name of the tests ExerciseName > test description so we decided to keep only the final description. But I didn't realize at that time that there was tests with more than 1 level in descriptions hierarchies. I suppose instead of keeping just the last part of the test description, we could keep everything after the ExerciseName and we have a guarantee that this will be unique.

This also means that when there are task groups we will have a leading number in the description like 1 > subtest 1a which is not too bad. That's just a reminder of the grouping made by the UI and it doesn't take much characters. And if we were to remove also the task ids from descriptions, we could not guarantee that the name are unique.

[jiegillet]

I checked quickly:

There are some duplicate names in:

• allergies (12 tests -> 11 names)
• list-ops (20 -> 8)
• robot-simulator (20 -> 4)
• run-length-encoding (14 -> 11)

No issues with in concept exercises.

Not too hard to fix by hand quickly if we need to.

[jiegillet]

This also means that when there are task groups we will have a leading number in the description like 1 > subtest 1a which is not too bad. That's just a reminder of the grouping made by the UI and it doesn't take much characters. And if we were to remove also the task ids from descriptions, we could not guarantee that the name are unique.

We could also trim the names after we merge the files if we want.

[mpizenberg]

We could, though better have the names being guaranteed unique even for exercism

[mpizenberg]

This means I'll need another version of elm-test-rs. There are ongoing things with elm-test-rs, so I have to check and it might take me a week. I suppose we are not in any hurry here and can wait a week.

[mpizenberg]

Hum maybe this can go faster. I'll let you guys know.

[ceddlyburge]

I think waiting a week is ok ...

[mpizenberg]

Hum, all tests are already required to have unique names. Otherwise the test runner crashes

…

On Tue, Apr 5, 2022, 08:32 Jie ***@***.***> wrote: First of all: nice, this version solves the issue! Second: OMG, this lead me to find a huge issue! @mpizenberg <https://github.com/mpizenberg> @ceddlyburge <https://github.com/ceddlyburge> In my PR, I changed the code to group merge the tests and the code snippets. However, whenever there are duplicate names, the jq script merges those. robot-simulator has 20 tests, but the final result only has 4 because most of the tests are named "bearing" or "coordinates". This can't be fixed by a better jq script since there is no way to know which test goes with which snippet. I can think of 2 things to do: 1. Require all tests to have unique names in the Elm repo 2. Change both elm-test-rs and extract-test-code so that the name becomes "description 1 > Description 2 > ... > Name" or something like that. These should be unique too of course. What do you think? We can't merge my PR before we fix that. — Reply to this email directly, view it on GitHub <#34 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAWFOCKNY5C6BXLPVVH2NWDVDPNAZANCNFSM5RW54KWA> . You are receiving this because you were mentioned.Message ID: ***@***.***>