EntityQueryTest::testCaseSensitivity() can fail randomly

Ah, I thought we used $i explicitly for $fixtures, we don't, that's what I meant.

Yes, the fix makes sense to me.

The only way this can happen is if the last 12 of the total 16 characters are repeated in 3 groups of 4 characters (I have no idea this could ever happen, whoever had that patch tested should play lottery), so the comment could be a bit more clear.

If we can clarify this a bit then I think this is RTBC.

This just failed again in #2397691: Random fail in Drupal\taxonomy\Tests\TermTest::testNodeTermCreationAndDeletion().

Which means that there is either something else or this is more likely to happen than we thought.

Playing with this, made a script to test my theory and executed it 4 times in parallel, 4x 10'000'000 tries. No match.

I can not believe that we had two fails here already and I had none in 40 million tries.

So I modified it a bit to do a strpos() instead of explicitly looking for a match on the second try, which is the same as the test is doing. And voila!

M5EO0872/xzkvapnb => 0872xzkv, position 4
MATCH on attempt 982137!
QHOJ4467/slzp6uli => 4467slzp, position 4
MATCH on attempt 983261!
S3PJ3499/oev0wfb4 => 3499oev0, position 4
MATCH on attempt 984321!
QAHL8803/qxphssp0 => 8803qxph, position 4
MATCH on attempt 986817!
AROU1418/znazess6 => 1418znaz, position 4
MATCH on attempt 987227!
SYBW7658/jzlidldl => 7658jzli, position 4
MATCH on attempt 988289!
BXS91272/shakj1pl => 1272shak, position 4
MATCH on attempt 989988!
VLG04012/jy9bhakp => 4012jy9b, position 4
MATCH on attempt 991143!
QFLD4325/zv0b4u0h => 4325zv0b, position 4
MATCH on attempt 991240!
JJA90232/mzppkbh0 => 0232mzpp, position 4
MATCH on attempt 991626!
NME78014/jobhi78x => 8014jobh, position 4
MATCH on attempt 991761!
FFTS3151/ngshfgay => 3151ngsh, position 4
MATCH on attempt 992680!
EFGH4646/xwraqfud => 4646xwra, position 4
MATCH on attempt 993565!
PQDK7664/bj4jpogx => 7664bj4j, position 4
MATCH on attempt 993647!
AJLD0607/letmcqe5 => 0607letm, position 4
MATCH on attempt 997589!
665 matches in 1000000 tries: 0.0665%

(I changed it to only run 1 million times, more than enough)

The pattern is easy enough to spot: numbers!

If the last 4 characters of the first string are all numbers, then strtoupper() doesn't change a thing and it matches.

So the correct fix is to append a character, then this can not happen. Note that it is not possible that the first or all characters are numbers, as the first character is guaranteed to be non-numeric.

The script:

use Drupal\Component\Utility\Random;

function test() {
  $fixtures = array();
  $random = new Random();
  for ($i = 0; $i < 2; $i++) {
    $string = $random->name(8);
    $fixtures[] = array(
      'original' => $string,
      'uppercase' => strtoupper($string),
      'lowercase' => strtolower($string),
    );
  }

  $match = strtolower(substr($fixtures[0]['uppercase'] . $fixtures[1]['lowercase'], 4, 8));
  $pos = strpos($fixtures[0]['uppercase'] . $fixtures[1]['lowercase'], $match);
  if ($pos !== FALSE) {
    print $fixtures[0]['uppercase'] . "/" . $fixtures[1]['lowercase'] . ' => ' . $match . ", position $pos\n";
    return TRUE;
  }
  return FALSE;
}

$matches = 0;
$tries = 1000000;

for ($i = 0; $i < $tries; $i++) {
  $match = test();
  if ($match) {
    $matches++;
    print "MATCH on attempt $i!\n";
  }
  if ($i > 0 && $i % 100000 === 0) {
    print "$i attempts\n";
  }

}

if ($matches) {
  print "$matches matches in $tries tries: " . (100 / $tries * $matches) . "%\n";
}
else {
  print "No matches in $tries tries\n";
}

I don't think that would make sense. It already protects against getting a number as first character, because that would be a problem for many use cases.

This scenario isn't a problem in any of our existing test (and unlikely to be in all upcoming ones), only in this very specific and rather special test. There's nothing wrong with a field name like something1234, for example.