Problem/Motivation

PathAliasTest::testPathCache seems to contain a random fail, I've seen it a few times but most recently here
https://git.drupalcode.org/issue/drupal-3426302/-/jobs/4324922

Drupal\Tests\path\Functional\PathAliasTest::testPathCache
Cache entry was created.
Failed asserting that a boolean is not empty.

core/modules/path/tests/src/Functional/PathAliasTest.php:84

Steps to reproduce

?

Proposed resolution

Use key/value instead of state in WaitTerminateTestTrait.

Issue fork drupal-3505952

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

acbramley created an issue. See original summary.

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

catch opened merge request !11182

catch’s picture

catch
he/him
Primary language English
commented

Ran the test 500 times and 100% pass https://git.drupalcode.org/project/drupal/-/jobs/4330926

Trying another run: https://git.drupalcode.org/project/drupal/-/jobs/4331111

It could be that faillures are more like 1/1000 or 1/5000 or it could be that running only this test concurrently doesn't create the same failure conditions as a normal test job.

The test already uses WaitTerminateTestTrait correctly afaict.

acbramley’s picture

acbramley commented

@catch yeah it passed on a rerun on that PR but I've seen it fail more than once. Definitely a rare one but it stood out cause it was a functional test.

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

mstrelan’s picture

mstrelan commented
Status: Active » Needs review

In light of #3506148: [meta] Replace all uses of state in tests with direct key value, should WaitTerminateTestTrait be using Key/Value instead of State? Pushing up an MR to see if that passes.

mstrelan opened merge request !11526

mstrelan’s picture

mstrelan commented
Issue summary: View changes

So that makes the performance tests fail because we're doing an extra key/value lookup for every request, in test environments only. Not sure why a get from state is different, maybe it's filtered out by PerformanceTestTrait::isDatabaseCache? So we have a few options:

  1. Update performance tests to expect this lookup
  2. Filter this lookup in performance test data
  3. Try to detect performance tests and return early in the middleware
  4. Enable a module in performance test base that disables this middleware
  5. Move this middleware to a module that needs to be enabled in tests that use the trait, and have the trait throw an exception if the module is not enabled
catch’s picture

catch
he/him
Primary language English
commented

Move this middleware to a module that needs to be enabled in tests that use the trait, and have the trait throw an exception if the module is not enabled

tbh this is how I think it should work in general - it's a lot of extra database queries when taken across the entire test suite.

mstrelan’s picture

mstrelan commented

Ok I might look in to that option a little more. I also opened #3513875: Move test middleware out of CoreServiceProvider which we can probably use for that.

smustgrave’s picture

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

Seems to be pretty inline with the others that were converted from state to key value and those appear to have worked great

mstrelan’s picture

mstrelan commented
Status: Reviewed & tested by the community » Postponed

Postponing on #3513875: Move test middleware out of CoreServiceProvider which, in its current state, should fix this without the need to update performance tests.

catch’s picture

catch
he/him
Primary language English
commented
Status: Postponed » Closed (outdated)

#3496369: Multiple load path aliases without the preload cache path cache and this test no longer exist.

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.

quietone closed merge request !11182

quietone closed merge request !11526