Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

'Robot Name' false positive tests #943

Open
garveychan opened this issue Feb 13, 2022 · 7 comments
Open

'Robot Name' false positive tests #943

garveychan opened this issue Feb 13, 2022 · 7 comments
Assignees

Comments

@garveychan
Copy link

garveychan commented Feb 13, 2022

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

  • This issue may need to be addressed in other tracks as well.
  • Implementing the regex check for each newly generated name adds about 10 seconds to even the most optimal solutions when checking all possible permutations. There may be a more performant approach for strengthening these tests.

<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
Copy link
Member

(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:

  • Some of the top-solutions are passing all the tests but they should not
  • The quick solution is by regex testing all names

@garveychan
Copy link
Author

garveychan commented Feb 13, 2022

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
Copy link
Member

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
Copy link
Author

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

@SleeplessByte
Copy link
Member

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
Copy link
Author

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
Copy link
Member

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

@SleeplessByte SleeplessByte self-assigned this Jul 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants