Rust track: check predictability of names in "robot-name" exercise

Programming Rust
1

The README to the exercise states

The names must be random: they should not follow a predictable sequence.

Yet there are accepted solutions that violate the requirement.

I created a PR with a test that rejects all too predictable name sequences: Check predictability of names in `robot-name` exercise by EugenDueck · Pull Request #2110 · exercism/rust · GitHub

The PR bot encouraged me to discuss the issue here first.

2

Here’s the link to the text in the README:

3

This should be moved to the Rust category.

I strongly dislike our exercises about randomness and how they are tested. They teach users that it’s OK to write essentially untestable code.

So, I have no desire to “improve” the exercise with brittle hacks to work around the fundamentally flawed, untestable design of the exercise.

The PR uses global mutable variables, raw pointers and unsafe. This is really bad code. If the thing you were doing was the right thing to do (which I don’t think it is), you should use std::sync::LazyLock instead:

static PREDICTABLE_SEQ_TEST_LOCK: LazyLock<RwLock<()>> = LazyLock::new(|| RwLock::new(()));
4

I would consider adding a test that better checks explicitly stated requirements an improvement. Currently, exercism accepts solutions openly violating requirements. That’s not great for people trying to compare other “accepted” solutions.

It may be better to remove exercises you cannot or do not want to properly check.

That said, the LazyLock hint is a very valid point, and I have changed the code to use that.

5

I gotta admit, the community solutions are pretty bad. The one you pointed out is actually the first spot. Number 2 is not much better.

Maybe it’s best to break all existing solutions and redesign the exercise from scratch. We could rely on the Rng trait. Rng with the same seed → expect same robot name. Use one Rng twice → expect different robot name.

1 Like
6

I agree, that any kind of randomness is untestable for. And I also agree to the general Exercism position of a) not testing for all possible ways to cheat / circumvent requirements and b) nudging people into mentoring for fixing their bad code and learning that way.

So, I would not test for “proper randomness” in the exercise tests, but add an analyzer to check for using random functionality provided by Rust. That would not bloat the tests so that students actually see and understand what is tested. And it would point students to work on their code again / ask a mentor about what’s wrong with their solution.

Anyways, I understand that doing testing well (injecting an RNG for testing) is the more professional solution. But it moves away from problem specs and automatic test generation.

7

robot-name doesn’t have a canonical-data.json, so drifting from problem-specifications is not an issue in this case. All the tests are manual.

8

Alright, here’s my redesign:

I would very much appreciate reviews from non-Rust-track-maintainers!

2 Likes
9

FWIW: you can test this with probability. I did some calculations on this before.

In other tracks like JS: we don’t test the random nature but we expected people to keep track of all generated, so this became testable. Dunno what’s better. I like the RNG injection the best in most cases…