Problem/Motivation

In #3321905: Add colinodell/psr-test-logger to core's dev dependencies we added a new dev dependency on colinodell/testlogger
In #2932518: Deprecate watchdog_exception we added some usages of it
I think we should use a different test logger that we already have from an existing dependency - \Symfony\Component\ErrorHandler\BufferingLogger

Pros

One less dev dependency to manage

Cons

The api is not as nice, you need to use cleanLogs and then filter things yourself

Steps to reproduce

Proposed resolution

Get folks to agree first - then replace the usages and remove the dependency

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Comments

larowlan created an issue. See original summary.

larowlan’s picture

Issue summary: View changes
larowlan’s picture

Issue summary: View changes
jonathanshaw’s picture

In #2903456: Allow kernel tests to fail or expect logged errors (https://git.drupalcode.org/project/drupal/-/merge_requests/14) we created an AssertableLogger that is much more sophisticated than either colinodell/testlogger or Symfony's BufferingLogger. The idea with colinodell/testlogger was that we would now extend AssertableLogger from that.

What we need for #2903456: Allow kernel tests to fail or expect logged errors is so far beyond the capabilities of BufferingLogger (which offers so little code itself) that I doubt the complexity of adding BufferingLogger as a dependency is worth it. Honestly, I thought it was debatable whether colinodell/testlogger was worth it.

Additionally, as I understand it BufferingLogger basically temporarily stores logs and then flushes them on at the end of a request:

    public function __destruct()
    {
        foreach ($this->logs as [$level, $message, $context]) {
            ...
            error_log(sprintf('%s [%s] %s', date(\DateTimeInterface::RFC3339), $level, $message));
        }
    }

I find it hard to forsee the consequences of this behavior change for test DX.

larowlan’s picture

Status: Active » Closed (won't fix)
Issue tags: -Drupal 10.1 target

Discussed with catch and lauriii

We agree that the DX doesn't justify it