We have a large automatic test suite here at Net-a-Porter, and that test suite gets run automatically whenever we check code changes in to our source code repository. Of course, a developer will run the suite manually before checking in changes, but it’s nice for the rest of the team to be able to see that a particular change hasn’t broken anything.
Occasionally, the test suite will flag problems that weren’t introduced with the most recent code change. This can get frustrating for the poor developer who gets the blame for breaking something that she didn’t touch. So when we find strange problems like this, we try to fix them; this is how we fixed one this morning.
The test suite was failing, and the error said that a uniqueness constraint was being broken when a program was trying to insert test data into the database. We could see what the failing data was, so it was simple to track down the piece of code in the test suite that was generating this data. It looked a bit like this:
my $code = 'VC-' . time() . int(rand(1000));
This code was then called eight times in a loop — one for each of the test cases. Obviously, that loop runs quickly, so there’s every chance that this code is called more than once per second; disambiguating the generated values is what the random number on the end is for.
But we’re only choosing a number between 0 and 999. Given the number of times that this test is run, we’ll occasionally generate the same random number twice (or more) in the same second. When that happens, we get the same code and the database insert will fail. That seemed to explain why the error sometimes happened.
On our internal developer chat channel, we started to discuss ways to solve the problem. At first, it seemed that an incrementing integer would be safer than a random number. So the code would look like this:
my $code = 'VC-' . time() . ++$i;
We’re guaranteed that we won’t generate the same code twice. Well, as long as there’s only one version of the test running. We can make it support multiple processes by including the process ID.
my $code = 'VC-' . time() . $$ . ++$i;
That works. But it’s all looking a bit over-engineered. Surely there’s a better solution.
And, of course, there is. We’re already using Data::UUID in other parts of the code, and this is perfect for this use case too. According to the documentation, any UUID generated by this module “is guaranteed to be different from all other UUIDs/GUIDs generated until 3400 CE” — which sounds long beyond the expected lifespan of this software.
The code now looks like this:
my $code = 'VC-' . Data::UUID->new->create_hex;
And that’s one fewer false negative in our test suite. There are already plans afoot to search the code base for other similar (ab)uses of rand().