- Issue created by @acbramley
- First commit to issue fork.
- 🇬🇧United Kingdom catch
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
This could be hard to track down. The test already uses WaitTerminateTestTrait correctly afaict.
- 🇦🇺Australia acbramley
@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.
- First commit to issue fork.
- 🇦🇺Australia mstrelan
In light of 📌 Identify usages in tests of State that can be replaced with key value instead Active , should WaitTerminateTestTrait be using Key/Value instead of State? Pushing up an MR to see if that passes.
- 🇦🇺Australia mstrelan
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:- Update performance tests to expect this lookup
- Filter this lookup in performance test data
- Try to detect performance tests and return early in the middleware
- Enable a module in performance test base that disables this middleware
- 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
- 🇬🇧United Kingdom catch
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.
- 🇦🇺Australia mstrelan
Ok I might look in to that option a little more. I also opened 📌 Move test middleware out of CoreServiceProvider Active which we can probably use for that.
- 🇺🇸United States smustgrave
Seems to be pretty inline with the others that were converted from state to key value and those appear to have worked great
- 🇦🇺Australia mstrelan
Postponing on 📌 Move test middleware out of CoreServiceProvider Active which, in its current state, should fix this without the need to update performance tests.
