Problem/Motivation
This is follow-up issue from #3496117: Remove unnecessary usage of dblog module at tests.
Some tests use dblog module to check log messages, this requires database connection.
In order to simplify this and improve test speed, we can use some alternatives.
We already has dependency on
\Symfony\Component\ErrorHandler\BufferingLogger\ColinODell\PsrTestLogger\TestLogger
BufferingLogger fails if on tests end there are some unhadled errors. It can be used in case you need to be sure there is no error messages on test finish.
TestLogger allows to have log messages on test finish, and much more flexible. The class is final, it's not possible to extend.
But both of these classes collect all context, and if you just need to check log message and log message variables, you need to filter log context.
Steps to reproduce
Proposed resolution
To improve DX, and allow to easily check logs during tests:
1) Create proxy class for \ColinODell\PsrTestLogger\TestLogger and filter context to contain only message variables.
2) Replace existing usage of dblog module at Kernel test with new proxy class for
- Drupal\Tests\comment\Kernel\CommentIntegrationTest
- Drupal\Tests\field_ui\Kernel\EntityDisplayTest
- Drupal\Tests\image\Kernel\ImageItemTest
- Drupal\Tests\jsonapi\Kernel\ResourceType\RelatedResourceTypesTest
- Drupal\KernelTests\Core\Action\EmailActionTest
3) As follow-up we also can convert existing usage of \Symfony\Component\ErrorHandler\BufferingLogger,\ColinODell\PsrTestLogger\TestLogger and other approaches to new proxy class. So all code base use similar approach.
Remaining tasks
These functional tests maybe we can also make replacement, but this need to be discussed.
- Drupal\Tests\big_pipe\Functional\BigPipeTest
- Drupal\Tests\media\FunctionalJavascript\MediaSourceOEmbedVideoTest
- Drupal\Tests\node\FunctionalNodeCreationTest
- Drupal\Tests\system\Functional\Module\ModuleTestBase
User interface changes
Introduced terminology
API changes
Data model changes
Release notes snippet
Issue fork drupal-3496184
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:
Comments
Comment #2
nikolay shapovalov commentedComment #3
nikolay shapovalov commentedThese tests:
have same goal. Find single message and check variables. Maybe we can create trait for this, to be able to register logger and check message.
Example:
Comment #5
nikolay shapovalov commentedI create trait that register \Symfony\Component\ErrorHandler\BufferingLogger as logger service.
And have new assert method.
I found that using BufferingLogger have some cons:
While working CommentIntegrationTest.php, there was one error log that was not checked by tests, that caused error fail on test finish. Because of BufferingLogger::__desctruct() showing all errors on test finish.
Also BufferingLogger::__desctruct() method doesn't support drupal token replacement.
I believe this is not behavior I want.
If tests doesn't care about log, we also need to ignore that.
Also I don't like implementing register() method at trait. Maybe we can directly inject logger to KernelTest.
Comment #7
nikolay shapovalov commentedI decide that it would be great to have TestLogger to use at tests as dblog alternative.
And found that we already using \ColinODell\PsrTestLogger\TestLogger at some tests.
Another great example is \Drupal\Tests\feeds\Kernel\TestLogger and Drupal\Tests\feeds\Kernel\FeedsKernelTestBase.
Maybe we can use \ColinODell\PsrTestLogger\TestLogger::hasRecordThatPasses to find message with argument. So we don't need to reinvent the wheel.
Comment #10
nicxvan commentedThat would be interesting! Just in case you didn't know, adding a dependency to core has a lot of gates:
https://www.drupal.org/about/core/policies/core-dependency-policies/depe...
Might be worth asking in the core development channel for advice before going too far down this path.
Here is the project: https://github.com/colinodell/psr-testlogger
Comment #11
nikolay shapovalov commented@nicxvan thanks for feedback.
But as I can see colinodell/psr-testlogger is already a part of Core dependency.
I will ask for advice anyway, thanks. Just want to make some research before, to have better understanding of the topic.
Comment #12
nicxvan commentedOh sorry, I thought were researching a new dependency, I misunderstood!
Comment #14
nikolay shapovalov commentedI create helper class TestLogger. Attaching it directly in the test method looks very easy, and hasn't a lot of boilerplate.
I was thinking to use colinodell/psr-testlogger, but there are a lot of data in the log->context. In order to filter it, we can use proxy and facade approach, but I want to keep it simple. Creating custom class looks like good alternative.
Also I added rendered message to log, and TestLogger can search rendered message, this is not used in MR, but this should be useful for someone who is searching simple rendered message.
Set to NR to get some feedback.
Comment #15
nikolay shapovalov commentedI created a Proxy for \ColinODell\PsrTestLogger\TestLogger to filter context of log records.
And result looks good. All \ColinODell\PsrTestLogger\TestLogger methods can be used.
Probably this is better than creating custom TestLogger from scratch.
Comment #20
nikolay shapovalov commentedComment #21
nikolay shapovalov commentedComment #22
nikolay shapovalov commentedComment #23
nikolay shapovalov commentedThis is gonna be solved at #3496184: Replace usage of dblog module at tests with alternative