Support from Acquia helps fund testing for Drupal Acquia logo

Comments

alexpott created an issue. See original summary.

tacituseu’s picture

Status: Needs review » Needs work

The last submitted patch, 2: back-to-113c6dc258.patch, failed testing. View results

alexpott’s picture

mradcliffe’s picture

Shoot. It looks like 5.5 test run has the same fail on #6: https://dispatcher.drupalci.org/job/drupal_patches/36959/consoleFull

18:00:38 Drupal\Tests\rest\Functional\EntityResource\EntityTest\Entit   0 passes             1 exceptions             
18:00:38 FATAL Drupal\Tests\rest\Functional\EntityResource\EntityTest\EntityTestJsonInternalPropertyNormalizerTest: test runner returned a non-zero error code (2).
18:00:38 Drupal\Tests\rest\Functional\EntityResource\EntityTest\Entit   0 passes   1 fails 
Anonymous’s picture

alexpott’s picture

It's weird that #6 failed whilst https://dispatcher.drupalci.org/job/php5.5_postgres9.1/2567/consoleFull was a pass. That is the log of the last successful branch test on php5.5 / postgres / 8.5.x and the commit hash was:

00:07:40 Git commit info:
00:07:40 	113c6dc (grafted, HEAD, origin/HEAD, origin/8.5.x, 8.5.x) Issue #2840392 by Wim Leers, JacobSanford, Yogesh Pawar, dawehner, xjm, effulgentsia, Mixologic, Bojhan, webchick, Gábor Hojtsy, yoroy: Enable BigPipe by default in the Standard install profile

So the codebase is exactly the same. This suggests that something else has changed. Perhaps on DrupalCI or on the underlying AWS infrastructure.

tacituseu’s picture

alexpott’s picture

tacituseu’s picture

@alexpott: my bad, I somehow assumed you ment to revert all of Nov 6.
The fail isn't 100%, e.g. for SQLite:
https://www.drupal.org/pift-ci-job/805468 is green
while
https://www.drupal.org/pift-ci-job/803921
https://www.drupal.org/pift-ci-job/804598
had the same problem

alexpott’s picture

@tacituseu well the fail seems to be 100% for PHP5.5 + Postgres + 8.5.x at the moment and it wasn't until the commit after 113c6dc. But the fact that revert all the changes doesn't fix it does seem to suggest that something else is at play other than the 8.5.x codebase.

alexpott’s picture

@vaplas so any ideas why enabling book seems to the issue?

tacituseu’s picture

Looking at @vaplas patches it fails 2-8 times per 30 runs of affected tests, both SQLite and PostgeSQL.

Edit:
@alexpott: Retested #6 on PostgreSQL and it passed (https://www.drupal.org/pift-ci-job/806292).

Anonymous’s picture

#14: This patch from here. It is change the set of dependencies, the collection of which leads to random bugs in PHP 5.x. Maybe this bug (found by @xjm, see more in #2919863: Discover why gc_disable() during non-interactive install improves tests stability).
But why for Postgresql and SQLite need different tests? And why it does appears on PHP 5.6 as before? I do not know. See also #2879048-189: Ignore: patch testing issue for #2919863 from @tacituseu.

#15: No, just different modules are differently useful. And 'tour' is useless even here) Three other modules mixed the cards well enough)

tacituseu’s picture

@vaplas: #15 was meant to be a reply to #13.

alexpott’s picture

Wim Leers’s picture

#18 looks sensible.

tacituseu’s picture

Sure beats reverting important patches each time we step on unsupported PHP's toes, all just because of some vintage Ubuntu fetish ;).

Anonymous’s picture

Funny, the opposite approach in #2879048-202: Ignore: patch testing issue for #2919863 also works!)

catch’s picture

Status: Needs review » Reviewed & tested by the community

The revert patch in the other issue is likely because it's affecting how the tests interact at high concurrency, however it's very likely to result in re-introducing previous segfaults we've already fixed. Moving this to RTBC.

Anonymous’s picture

+1. Perhaps large test can cause this bug even with the gc_disable() during the installation phase. But this patch is in any case a good strengthening before #2842431: [policy] Remove PHP 5.5, 5.6 support in Drupal 8.7

tacituseu’s picture

@catch: that revert wasn't meant to be a solution, just a check to see if it will move the segfault out of EntityTest*JsonInternalPropertyNormalizerTest, it shows that there's nothing inherently wrong with #2871591: Allow ComplexData in TypedData to specify computed properties that should be exposed in normalization and other contexts itself.
+1.

Anonymous’s picture

#25: no) @catch said about #21, where I pointed to @tacituseu's patch with demonstration of passing tests after revert #2859704: Intermittent segfaults on DrupalCI (some "did not complete due to a fatal error" with no additional info).

In #24 @tacituseu already confirmed that he did not pursue the goal of pushing this revert. I'm sorry if my #21 comment has caused confusion.

catch’s picture

Version: 8.5.x-dev » 8.4.x-dev
Status: Reviewed & tested by the community » Fixed

Yep sorry that wasn't really for tacituseu just for anyone looking at this issue in 4 years who might get confused, but now everyone's confused now...

Committed/pushed to 8.5.x and cherry-picked to 8.4.x. Thanks!

  • catch committed dbc0162 on 8.5.x
    Issue #2921862 by alexpott, vaplas, tacituseu: Segfault on PHP5.5 and...

  • catch committed 29e70e8 on 8.4.x
    Issue #2921862 by alexpott, vaplas, tacituseu: Segfault on PHP5.5 and...
Anonymous’s picture

🎉

Hah!) It means that in four years anyone looking at this issue will feel more comfortable with confused :)

I reruned #2879048-178: Ignore: patch testing issue for #2919863 with test-only for PostgreSQL and SQLite, and now both green as expected. Super!

#2919863: Discover why gc_disable() during non-interactive install improves tests stability fixed as support request, because now we know what happened. Thank you all, and especially @tacituseu, who did a lot of work on this, and many times corrected me in my research. 🙏

Anonymous’s picture

By the way, @Lendude also made a significant contribution (#2879048-98 - #2879048-109), creating a very useful test at an early stage research #2879048-102: Ignore: patch testing issue for #2919863.

Wim Leers’s picture

🎉👏

tacituseu’s picture

This would also mean #2900399: Adding test coverage for batch bulk updates can go in now.

catch credited Lendude.

catch’s picture

Status: Fixed » Closed (fixed)

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