Problem/Motivation

---- Drupal\Tests\comment\Kernel\Views\CommentUserNameTest ----
Status    Group      Filename          Line Function                            
--------------------------------------------------------------------------------
Exception Other      phpunit-271.xml      0 Drupal\Tests\comment\Kernel\Views\C
    PHPUnit Test failed to complete; Error: PHPUnit 10.5.35 by Sebastian
    Bergmann and contributors.
    
    Runtime:       PHP 8.3.12
    Configuration: /builds/project/drupal/core/phpunit.xml.dist
    
    E                                                                   1 / 1
    (100%)
    
    Time: 00:03.480, Memory: 8.00 MB
    
    There was 1 error:
    
    1) Drupal\Tests\comment\Kernel\Views\CommentUserNameTest::testUsername
    Error: Call to a member function access() on null
    
    /builds/project/drupal/core/modules/comment/src/CommentAccessControlHandler.php:35
    /builds/project/drupal/core/lib/Drupal/Core/Entity/EntityAccessControlHandler.php:109
    /builds/project/drupal/core/lib/Drupal/Core/Entity/ContentEntityBase.php:738
    /builds/project/drupal/core/lib/Drupal/Core/Entity/EntityAccessCheck.php:68
    /builds/project/drupal/core/lib/Drupal/Core/Access/AccessManager.php:160
    /builds/project/drupal/core/lib/Drupal/Core/Access/AccessManager.php:136
    /builds/project/drupal/core/lib/Drupal/Core/Access/AccessManager.php:93
    /builds/project/drupal/core/lib/Drupal/Core/Url.php:813
    /builds/project/drupal/core/lib/Drupal/Core/Field/Plugin/Field/FieldFormatter/StringFormatter.php:132
    /builds/project/drupal/core/lib/Drupal/Core/Field/FormatterBase.php:91
    /builds/project/drupal/core/lib/Drupal/Core/Entity/Entity/EntityViewDisplay.php:268
    /builds/project/drupal/core/modules/views/src/Entity/Render/EntityFieldRenderer.php:257
    /builds/project/drupal/core/modules/views/src/Entity/Render/EntityFieldRenderer.php:143
    /builds/project/drupal/core/modules/views/src/Plugin/views/field/EntityField.php:869
    /builds/project/drupal/core/modules/views/src/Plugin/views/field/FieldPluginBase.php:1195
    /builds/project/drupal/core/modules/views/views.theme.inc:231
    /builds/project/drupal/core/lib/Drupal/Core/Theme/ThemeManager.php:261
    /builds/project/drupal/core/lib/Drupal/Core/Render/Renderer.php:446
    /builds/project/drupal/core/lib/Drupal/Core/Render/Renderer.php:203
    /builds/project/drupal/core/modules/views/src/Plugin/views/field/FieldPluginBase.php:1796
    /builds/project/drupal/core/modules/views/src/Plugin/views/style/StylePluginBase.php:769
    /builds/project/drupal/core/lib/Drupal/Core/Security/DoTrustedCallbackTrait.php:107
    /builds/project/drupal/core/lib/Drupal/Core/Render/Renderer.php:825
    /builds/project/drupal/core/lib/Drupal/Core/Render/Renderer.php:387
    /builds/project/drupal/core/lib/Drupal/Core/Render/Renderer.php:203
    /builds/project/drupal/core/lib/Drupal/Core/Render/Renderer.php:120
    /builds/project/drupal/core/lib/Drupal/Core/Render/Renderer.php:593
    /builds/project/drupal/core/lib/Drupal/Core/Render/Renderer.php:119
    /builds/project/drupal/core/modules/views/src/Plugin/views/style/StylePluginBase.php:711
    /builds/project/drupal/core/modules/views/src/Plugin/views/style/StylePluginBase.php:574
    /builds/project/drupal/core/modules/views/src/Plugin/views/style/StylePluginBase.php:462
    /builds/project/drupal/core/modules/views/src/Plugin/views/display/DisplayPluginBase.php:2177
    /builds/project/drupal/core/modules/views/src/ViewExecutable.php:1592
    /builds/project/drupal/core/modules/views/src/Plugin/views/display/DisplayPluginBase.php:2467
    /builds/project/drupal/core/modules/views/src/ViewExecutable.php:1721
    /builds/project/drupal/core/modules/comment/tests/src/Kernel/Views/CommentUserNameTest.php:158

https://git.drupalcode.org/project/drupal/-/jobs/3012953

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#37 3479887-nr-bot.txt91 bytesneeds-review-queue-bot
#30 3479887-nr-bot.txt91 bytesneeds-review-queue-bot
#6 xdebug_postgres.png309.87 KBmcdruid
#6 xdebug_mysql.png351.92 KBmcdruid

Issue fork drupal-3479887

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

catch created an issue. See original summary.

spokje made their first commit to this issue’s fork.

spokje opened merge request !9804

spokje’s picture

spokje
Primary language Dutch
commented

With 5000 runs without a failure this one is too random for me, I think you might have hit a fluke there @catch.

mcdruid’s picture

mcdruid commented

This one seems to fail consistently on PostgreSQL as far as I can see; I've reproduced the failure manually with postgres:16 (in ddev).

mcdruid@drupal-10:/var/www/html$ git status
Refresh index: 100% (18217/18217), done.
On branch 11.x
Your branch is up to date with 'origin/11.x'.

mcdruid@drupal-10:/var/www/html$ vendor/phpunit/phpunit/phpunit -c core core/modules/comment/tests/src/Kernel/Views/CommentUserNameTest.php
PHPUnit 10.5.35 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.3.10
Configuration: /var/www/html/core/phpunit.xml.pgsql

E                                                                   1 / 1 (100%)

Time: 00:02.411, Memory: 4.00 MB

There was 1 error:

1) Drupal\Tests\comment\Kernel\Views\CommentUserNameTest::testUsername
Error: Call to a member function access() on null

...snip...
mcdruid’s picture

mcdruid commented
StatusFileSize
new xdebug_mysql.png351.92 KB
new xdebug_postgres.png309.87 KB

edit - I think I got this wrong..

mcdruid’s picture

mcdruid commented
mcdruid’s picture

mcdruid commented
Title: [random test failure] CommentUserNameTest::testUsername() random failure » [PostgreSQL test failure] CommentUserNameTest::testUsername() random failure

Looks like the test fail that @catch originally flagged up was under PostgreSQL:

SIMPLETEST_DB = pgsql://drupaltestbot:drupaltestbotpw@database/drupaltestbot?module=pgsql
mondrake’s picture

mondrake
Location 🇮🇹
commented
Priority: Major » Critical

I think this is critical as it affects consistently tests on PostgreSql

bradjones1’s picture

bradjones1
Primary language English
commented

Just ran into this as well. https://git.drupalcode.org/issue/drupal-3343634/-/pipelines/320105/test_...

bradjones1’s picture

bradjones1
Primary language English
commented
Component: phpunit » comment.module

This is consistently reproducible locally... in fact it's less random and much more like, totally broken.

The test is set up wrong. Fixed test MR proposed.

bradjones1 opened merge request !9954

bradjones1’s picture

bradjones1
Primary language English
commented
Status: Active » Needs review
bradjones1’s picture

bradjones1
Primary language English
commented

All green on pgsql.

https://git.drupalcode.org/issue/drupal-3479887/-/pipelines/321059/

bradjones1 changed the visibility of the branch 3479887-random-test-failure to hidden.

bradjones1’s picture

bradjones1
Primary language English
commented
Title: [PostgreSQL test failure] CommentUserNameTest::testUsername() random failure » CommentUserNameTest::testUsername() failure due to missing comment type
Parent issue: #2829040: [meta] Known intermittent, random, and environment-specific test failures »
mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Needs review » Reviewed & tested by the community

Looks good to me.

  • larowlan committed 50b490ba on 11.x 
    Issue #3479887 by bradjones1, mcdruid: CommentUserNameTest::testUsername...
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Version: 11.x-dev » 11.0.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

@bradjones1 and I discussed this on the weekend, this is the correct fix

Committed to 11.x

Doesn't apply to 11.0.x but I think we should backport this all the way back to 10.3.x

Tagging as 'Patch (to be ported)' as a result

larowlan closed merge request !9954

mondrake’s picture

mondrake
Location 🇮🇹
commented

I'd be curious to know why this only failed on pgsql given the fix, and not on mysql and sqlite

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented

The default target of an entity reference field is the user entity. Comment::bundleFieldDefinitions sets this to the entity type configured on the comment type
In this test it was not being set because the comment type wasn't created
As a result the commented entity was loaded as a user
On MySQL/sqlite it was entity id 1, on pgsql it was 2
This should have been loading an entity test entity, but instead was loading a user
User 1 existed, user 2 did not

Kudos to bradjones1 for working out it was loading the wrong entity type, at the weekend I pointed him towards why on slack

larowlan opened merge request !10076

larowlan opened merge request !10077

larowlan opened merge request !10078

larowlan opened merge request !10079

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Patch (to be ported) » Needs review

Added MRs all the way back to 10.2.x

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Funny almost all MRs have random failures but none appear to be around this test.

catch’s picture

catch
he/him
Primary language English
commented

Makes sense to backport this all the way back to 10.2.x

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Reviewed & tested by the community » Needs work
StatusFileSize
new 3479887-nr-bot.txt91 bytes

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

quietone’s picture

quietone commented
Title: CommentUserNameTest::testUsername() failure due to missing comment type » [Patch to be ported] CommentUserNameTest::testUsername() failure due to missing comment type

quietone closed merge request !10076

quietone closed merge request !10079

quietone closed merge request !10078

quietone changed the visibility of the branch 3479887-10point2 to hidden.

quietone’s picture

quietone commented
Version: 11.0.x-dev » 10.6.x-dev
Status: Needs work » Needs review
needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 3479887-nr-bot.txt91 bytes

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

alexpott changed the visibility of the branch 3479887-10point4 to hidden.

  • alexpott committed d5189c4e on 10.5.x authored by larowlan 
    Issue #3479887 by bradjones1, mcdruid: CommentUserNameTest::testUsername...

  • alexpott committed cf3075ce on 10.6.x authored by larowlan 
    Issue #3479887 by bradjones1, mcdruid: CommentUserNameTest::testUsername...
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Version: 10.6.x-dev » 10.5.x-dev
Status: Needs work » Fixed

I don't understand why this was not just cherry-picked to 10.x - it cherry picks clean and the tests pass...

Anyhow done that now.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Title: [Patch to be ported] CommentUserNameTest::testUsername() failure due to missing comment type » CommentUserNameTest::testUsername() failure due to missing comment type

Status: Fixed » Closed (fixed)

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