exercism / typescript

Exercism exercises in TypeScript.
https://exercism.org/tracks/typescript
MIT License
147 stars 160 forks source link

'Robot Name' false positive tests #943

Open garveychan opened 2 years ago

garveychan commented 2 years ago

Issue

The top-ranked solutions for 'Robot Name' in Typescript are currently passing all unit tests even though (and primarily because) they are generating names that do not conform to the problem specification.

While the tests interpret these names as 'unique' because they do not exist within their respective Sets, repeating the regex check for each 'new name' function invocation exposes these strings as invalid robot names.

2nd Solution

Screenshot 2022-02-14 at 1 38 19 am

3rd Solution

Screenshot 2022-02-14 at 1 33 08 am

Notes

<200ms solution

Screenshot 2022-02-14 at 1 42 20 am

Quick Links

https://github.com/exercism/typescript/blob/main/exercises/practice/robot-name/.docs/instructions.md https://github.com/exercism/typescript/blob/main/exercises/practice/robot-name/robot-name.test.ts https://exercism.org/tracks/typescript/exercises/robot-name

SleeplessByte commented 2 years ago

(sidenote: you can generate all the names within about 3 seconds, if you take care of not generating collisions)

If I understand correctly, you're reporting the following things:

garveychan commented 2 years ago

Yep, that's right. My main concern with the regex quick fix is that it may distort the timeout validation when the solution is submitted online (I'm unfamiliar with the inner workings). I'll experiment with other options when I can find some more time.

Edit - I should clarify, the successful test case in the notes above (>11000ms) is bobahop's solution which hits <200ms without the regex test.

Screenshot 2022-02-14 at 2 46 38 am
SleeplessByte commented 2 years ago

Ah. Understood, thank you for that clarification. Want to post the regex test change you made? I can probably help you optimise the regex to make it faster.

947 actually starts skipping the all the names can be generated test, so the timeout won't be that much of an issue anymore.

garveychan commented 2 years ago

I took the existing regex test and put it into the loops where new robots were instantiated - on lines 68 and 137.

SleeplessByte commented 2 years ago

Sad. That's already pretty much as optimal as we can get. I'll figure out if there is a way to still test each and every name...

garveychan commented 2 years ago

Yeah I've been trying to think of a way to validate names that wouldn't expose a solution in the test file haha. Thank you for looking into it.

SleeplessByte commented 1 month ago

I have thought of a way to implement this, so I will.