Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Problem/Motivation
Consisent fail on Postgres - https://git.drupalcode.org/project/drupal/-/pipelines/96003
Random fail on MySQL, went away on retest:
1)
Drupal\Tests\standard\FunctionalJavascript\StandardPerformanceTest::testLoginBlock
Failed asserting that 50 is identical to 49.
/builds/issue/drupal-3390360/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:121
/builds/issue/drupal-3390360/vendor/phpunit/phpunit/src/Framework/Constraint/IsIdentical.php:79
/builds/issue/drupal-3390360/core/profiles/standard/tests/src/FunctionalJavascript/StandardPerformanceTest.php:139
/builds/issue/drupal-3390360/vendor/phpunit/phpunit/src/Framework/TestResult.php:728
https://git.drupalcode.org/issue/drupal-3390360/-/jobs/778969#L793
https://git.drupalcode.org/issue/drupal-3390360/-/jobs/786513#L792
Steps to reproduce
Run StandardPerformanceTest on postgres.
Proposed resolution
Loosen counts.
Remaining tasks
Follow up to add comments.
User interface changes
None
API changes
None
Data model changes
None
Release notes snippet
N/a
Issue fork drupal-3420401
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:
- 3420401-standard
changes, plain diff MR !6553
Comments
Comment #2
spokjeComment #3
spokjeComment #6
catchAdded an MR based on failures in https://git.drupalcode.org/project/drupal/-/jobs/795888
Comment #7
smustgrave commentedSwitch to a range makes sense, and know that was done on another ticket.
Comment #8
spokjeI swear, this specific test hates me: https://git.drupalcode.org/issue/drupal-3395986/-/jobs/805441#L780
Having said that, this MR would have prevent this failure.
Comment #9
quietone commentedI read the issue summary and the comments. The proposed resolution is explained in comment #7.
Reading the MR I think it should have comments to explain 1) why a range is needed and 2) how the numbers to use were picked. I think that information will be useful to those who come after us. However, I do not wish to hold this up since it does cause test failures. I am going to tag for a followup.
Comment #10
alexpottThis very nearly fixes a critical regression in Postgres testing - head is failing due to this. See https://git.drupalcode.org/project/drupal/-/pipelines/96003
But unfortunately there's one more thing to fix in Postgres... will push a commit to MR once the current MR has finished testing on Postgres.
Comment #11
alexpottYep as you can see from https://git.drupalcode.org/project/drupal/-/jobs/828846 this still does not fix postgres.
Comment #12
alexpottFixing the issue title.
Re #9 I'm not that comments are really possible. These number really represent a stake in the ground - this is what Drupal currently does. If we introduce something that improves them then we want this test fail and the issue that makes the improvement should adjust the numbers. If we have an issue that degrades them then we need to carefully consider why and if it worth it or if there is a better way.
Comment #13
alexpottThe latest commit on the MR is a nice colour of green for postgres... https://git.drupalcode.org/project/drupal/-/pipelines/96195
Comment #14
longwaveWondering if an
assertBetween()helper would make these slightly easier to read (and adjust in future).Comment #15
alexpottAfter discussions with @catch we decided to skip core performance tests on Postgres and SQLite because this is not really fixing the extra queries on postgres because they are due to the driver. So I reverted the postgres specific fix and added a skip in PerformanceTestBase.
Comment #16
catchLooks great now.
Comment #17
spokjeRemoving follow-up tag because of explanation in #12.
Comment #18
longwaveCommitted 46ed71c and pushed to 11.x. Thanks!
Did not cherry-pick cleanly to 10.2.x so not backporting this one.