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
The LogActionsTest::testRescheduleMultipleLogsRelative() tests are currently failing on PostgreSQL. They pass on MySQL/MariaDB.
There was 1 failure:
1) Drupal\Tests\log\Functional\LogActionsTest::testRescheduleMultipleLogsRelative
Logs have been rescheduled
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
Array (
- 0 => 1694100128
- 1 => 1694186528
- 2 => 1694272928
+ 0 => '1694272928'
+ 1 => '1694186528'
+ 2 => '1694100128'
)
/var/www/html/vendor/phpunit/phpunit/src/Framework/Constraint/Equality/IsEqual.php:96
/var/www/html/modules/contrib/log/tests/src/Functional/LogActionsTest.php:298
/var/www/html/vendor/phpunit/phpunit/src/Framework/TestResult.php:728
FAILURES!
Tests: 6, Assertions: 119, Failures: 1.
I traced it to the fact that $logs = $this->storage->loadMultiple(); is returning logs in reverse order... but only in this one function. Everywhere else it returns them in the correct order. I don't know why that is.
Steps to reproduce
See: https://www.drupal.org/pift-ci-job/2780177
Proposed resolution
TBD
Remaining tasks
TBD
User interface changes
None.
API changes
None.
Data model changes
None.
Issue fork log-3392223
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:
changes
Comments
Comment #2
shreya_th commentedComment #4
shreya_th commentedComment #5
m.stentaThank you for the merge request @Shreya_th! It looks like it fixes the issue and it makes sense.
I would still like to know *why* this is necessary, though. And why *isn't* it necessary in the other tests? And why does it only happen with PostgreSQL?
Why is there inconsistency in the order of results returned by
loadMultiple()in the first place?My concern is that, even though this MR fixes the tests, will it start happening in the other tests in the future? I would like to understand the underlying logic before we apply any fixes, so that we can be confident that they are the right fixes long-term.
I wonder if this is a known behavior of Drupal core. Perhaps we can find another issue or documentation that describes the same behavior.
Comment #8
paul121 commentedI updated the gitlab CI to run on multiple DBs inlcuding pgsql-13.5 and could reproduce this issue: https://git.drupalcode.org/project/log/-/pipelines/210166
Indeed this is weird. I wonder if it has to do with the order the logs are modified in the tests and postgres uses modified time as a default sort? But loadMultiple() actually returns an associative array with log IDs as the keys, so I'm not sure the order of the key/values in the array is guaranteed.
I've updated the tests so that the $timestamp arrays are keyed by the log IDs. I think this is a more explicit test anyways: we want to guarantee certain logs have certain timestamps, this code isn't responsible for returning logs in any particular order.
I'd like to merge these changes so that we can have tests running on multiple DBs (without failures!) What do you think @mstenta?
Comment #9
paul121 commentedComment #10
paul121 commentedOh, and I didn't change the other tests that use similar $timestamp arrays. In #3457116: Update owner of cloned logs to the current user I've actually refactored how these particular action tests work so they are simpler. But
LogActionsTest::testRescheduleMultipleLogsRelative()is particularly complex and can still use this approach, I think :-)Comment #11
m.stentaPostponing this until 2.x branch tests are passing again.
Comment #12
m.stentaNevermind! Tests are passing. I was confused by the old DrupalCI results still showing up. :-)
Comment #13
m.stenta@paul121 I think this all makes sense - thanks!
One thought and one request:
.gitlab-ci.ymlwe are giving ourselves something else to maintain moving forward, which we didn't have before. Without this change, what are the defaults? And what would happen if this were run in a GitLab instance that's different from Drupal.org's? I assume this configuration is tightly tied to that instance?Comment #14
m.stentaIs it possible to declare the database types without declaring the versions, and fall back on whatever the default versions are in the GitLab instance? I assume that's what it's doing when we don't declare anything (and falls back on a version of MySQL or MariaDB)?
I acknowledge that it is better to declare specific versions. I'm just curious how it works so we can decide what makes sense.
Comment #15
paul121 commentedYes, as I understand this template is very very tightly configured to the Drupal gitlab instance. I'm not sure what would happen if you ran this on another gitlab instance. From what I understand they have designed the template to "work out of the box" for the majority of drupal contrib modules & to always run on against latest Drupal core and default services. Depending on this template is so much less maintenance work than anything else we can do.
Though I see how database versions could create more work here but I'm not sure what we can do to avoid this. I followed the Drupal Gitlab CI template docs which links to these allowed images. In fact, I had first tried only specifying pgsql-13 and it failed (see the pipeline) because I wasn't specific enough (13.5 minor version is required). For pgsql 16 and mysql 8 it looks like they are maintaining a single major version of the image that will always use the latest minor release, but they don't have this for mariadb.
I just set the merge settings for this log project to use merge commit with semi linear history - is that OK? I think these descriptive commits are more valuable for describing how code is changing rather than just why. Or can we do this when we merge? Worrying about issue #s while working on PRs just makes things more confusing IMO
Comment #17
m.stentaMerged - thanks @paul121!