[random test failure] LayoutBuilderBlocksTest::testBlockPlaceholder failing

So MR!9676 (https://git.drupalcode.org/issue/drupal-3477586/-/jobs/2891429) shows that this test fails ~26/1500 runs.

At first glance, I can think of two ways the placeholder is not replaced:

1. Setting the $block_content in \Drupal::state() goes wrong/causes a race condition
2. The "magic" in \Drupal\layout_builder\EventSubscriber\BlockComponentRenderArray::onBuildRender does something unexpected and we end up in the if ($is_content_empty && $is_placeholder_ready) where we set the placeholder again, with similar content as the original one.

Let's assert both.

So looking at all the test failures being:

Drupal\Tests\layout_builder\Functional\LayoutBuilderBlocksTest::testBlockPlaceholder
    Behat\Mink\Exception\ResponseTextException: The text "is_content_empty &&
    is_placeholder_ready Placeholder for the "The block label" block" appears
    in the text of this page, but it should not.

I think we can safely say it's #2.

$content = $block->build() in \Drupal\layout_builder\EventSubscriber\BlockComponentRenderArray::onBuildRender ends up in \Drupal\layout_builder\Plugin\Block\ExtraFieldBlock::build.

The only way I can see that returning empty would be if we land in this if-branch:
if (!isset($extra_fields['display'][$this->fieldName])).

Which in turn would mean \Drupal\Core\Entity\EntityFieldManager::getExtraFields would return an array without $extra_fields['display'][$this->fieldName], which would mean in \Drupal\Core\Entity\EntityFieldManager::getExtraFields, $this->extraFields[$entity_type_id][$bundle] would not be set.

This is where it ends for me, I really have no idea what's happening to cause the random failure.

Ok, so there seems to be, what looks like, a delay for setting a value in State.

A lot of the currently randomly failing tests are using State to trigger some kind of behaviour change in the middle of a test.
Which, if the above statement is true, would make them also randomly fail, which seems to happen.

Time for some nitty-gritty proof why I think this is this case, dug up from the killing fields of test runs I had on this particular issue:

The testruns were all conducted on an isolated \Drupal\Tests\layout_builder\Functional\LayoutBuilderBlocksTest::testBlockPlaceholder which was stripped down to the bare minimum.
Testruns consisted of running \Drupal\Tests\layout_builder\Functional\LayoutBuilderBlocksTest::testBlockPlaceholder and only that test at least 250x, most of the times 1500x.
- Suspected $this->getSession()->reload(); not to work correctly, so if the random failure happened, tried to reload again: https://git.drupalcode.org/project/drupal/-/commit/71cbbeb1a96caf62d23ed.... Random still happened: https://git.drupalcode.org/issue/drupal-3477586/-/jobs/3807484
- Replaced $this->getSession()->reload(); with a drupalGet on the current page: https://git.drupalcode.org/project/drupal/-/commit/a33805b31010aade71155.... Random still happened: https://git.drupalcode.org/issue/drupal-3477586/-/jobs/3807601
- If random happened, retry

\Drupal::state()->set('block_test.content', $block_content); $this->getSession()->reload();

: https://git.drupalcode.org/project/drupal/-/commit/3b2b0bc960f62dcccc4c9.... That worked: https://git.drupalcode.org/issue/drupal-3477586/-/jobs/3817118 1500x without failure.
Since I've ruled out $this->getSession()->reload(); in the first two attempts mentioned here, the problem seems to lay in the State somewhere.
- Tried assertEquals on the state in LayoutBuilderBlocksTest: https://git.drupalcode.org/project/drupal/-/commit/5575da079ce25f4e95546.... Random still happened, but never on the assertEquals: https://git.drupalcode.org/issue/drupal-3477586/-/jobs/3817198.
So the state is set correct in the test itself.
- Tried assertEquals on the state in BlockComponentRenderArray.php: https://git.drupalcode.org/project/drupal/-/commit/889df588a31c494f88c51.... Randoms were gone.

My theory here is that the (very) small delay caused by the added lines in BlockComponentRenderArray.php, give the State enough time to set the value fully.

At this point I'm in need of Big Brains that can explain why there could be a delay in setting the State, or just straight up tell me the above conclusions are rubbish.

I've got the feeling that somehow the use of the CacheCollector inState, introduced in #2575105: Use cache collector for state has something to do with it, but as said, that's just a feeling.

Thanks @godotislate.

There's a Slack thread about this here: https://drupal.slack.com/archives/C079NQPQUEN/p1735214972315009

MR !10692 (which is basically \Drupal\Tests\layout_builder\Functional\LayoutBuilderBlocksTest trimmed down to only contain testBlockPlaceholder() and some pipeline changes to make it, and only it, run 2500 times) gives us 69 fails out of the 2500 runs.

MR !10694 shows that changing \Drupal::state()->set()/get() to Drupal::keyValue()->set()/get(); as suggested in the Slack thread passes a 2500 times run without errors.

Since this involves making changes in core/modules/block/tests/modules/block_test/src/Plugin/Block/TestCacheBlock.php, this means more tests are affected.

@godotislate was already miles ahead of me and created MR !10689, which is the MR to be committed.

@berdir:

\Drupal\Core\Cache\DatabaseBackend::invalidateAll() set the expiration to request time - 10 => MR!1069553 failures out of 2500 runs.

\Drupal\Core\Cache\DatabaseBackend::invalidateAll() set the expiration to 1 => MR!1069682 failures out of 2500 runs.

So roughly the same as the unchanged 2500 run.

I'm going to set this to NR for #22.

so it would be useful to not commit this for 24-48 hours just to get an extra few test runs on that issue.

I can see the usefulness, but I also see numerous MRs failing on this (and others).

We _could_ commit this and in your test-MR reverse the changes made here, which leaves you with exactly the same failing MR as before?