[random test failure] Try to un-skip and fix InlineBlockPrivateFilesTest::testPrivateFiles() in context of [#3353085]

The last submitted patch, 2: InlineBlockPrivateFilesTest_testPrivateFiles-no_#3350972-1500x-should_fail.patch, failed testing. View results →

That is very encouraging.

Indeed, but every silver lining needs a cloud: By the looks of it, we should be able to unskip all/a lot of Layout Builder JS tests, which will add somewhere around 5 minutes to a full test run by my estimates.

The last submitted patch, 2: InlineBlockPrivateFilesTest_testPrivateFiles-no_#3350972-1500x-should_fail.patch, failed testing. View results →

The last submitted patch, 2: InlineBlockPrivateFilesTest_testPrivateFiles-no_#3350972-1500x-should_fail.patch, failed testing. View results →

Both should_fail and non_fail patch run 5 times. That's 7500 individual tests
per patch.

Back to RTBC.

Alright one thing at a time!

Committed/pushed to 10.1.x, thanks!

Since we're going to try to commit one of these per day, skipping the backport for now, let's see how everything works together first (if at all, we could also just leave them skipped in earlier branches).

Setting PTPB per discussion with @catch.

Just did a follow-up commit for this:

diff --git a/core/modules/layout_builder/tests/src/FunctionalJavascript/InlineBlockPrivateFilesTest.php b/core/modules/layout_builder/tests/src/FunctionalJavascript/InlineBlockPrivateFilesTest.php
index 6e491cc97b..95f213ac93 100644
--- a/core/modules/layout_builder/tests/src/FunctionalJavascript/InlineBlockPrivateFilesTest.php
+++ b/core/modules/layout_builder/tests/src/FunctionalJavascript/InlineBlockPrivateFilesTest.php
@@ -62,7 +62,6 @@ protected function setUp(): void {
    * Tests access to private files added to inline blocks in the layout builder.
    */
   public function testPrivateFiles() {
-    // Skipped due to frequent random test failures.
     $assert_session = $this->assertSession();
     LayoutBuilderEntityViewDisplay::load('node.bundle_with_section_field.default')
       ->enableLayoutBuilder()

Ouch, sloppy work from me there, thanks @catch.

I have provided Patch for 9.4.x. please review

Why are we adding to 9.4.x?

@smustgrave Because these random failures also impair the security advisory process, which is currently provided back to 9.4.x.

Are we sure we want to mark this as fixed?
Doesn't seem it has landed anywhere else but in 10.1.x.

Yes this still need backport, but the original commit should cherry-pick I think.

Sorry, had the issue open in two tabs and crossposted.

Removing credit for the unnecessary backport patch.

I agree with backporting the original bugfix back to 9.4.x since that will fix random test failures there. With these patches though we're only unskipping previously-skipped tests, some/most of which were only skipped in the first place in 9.5.x or later, so I think it's better to stop at 9.5. We wouldn't backport new test coverage to 9.4.x at this point and this is similar.

I want to get this round of fixes all in since it's taken weeks to commit them 12-24 hours apart in 10.1.x, so going ahead with the cherry-picks to 10.0.x and 9.5.x but leaving there. @xjm if you strongly think they should go back to 9.4.x too please re-open although also we should double check they're skipped in the first place on that branch.

Made the changes directly in the 10.0.x branch since there were two commits on this issue and cherry-picked that commit to 9.5.x, thanks all!

Automatically closed - issue fixed for 2 weeks with no activity.