Problem/Motivation

DrupalConsoleLogger claims to throw a Psr\Log\InvalidArgumentException but instead throws InvalidArgumentException. According to the PSR Log standard it should throw Psr\Log\InvalidArgumentException

Steps to reproduce

Proposed resolution

Change to the correct Psr\Log\InvalidArgumentException (which extends InvalidArgumentException so should not cause BC issues)

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3622446

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

mfb created an issue. See original summary.

mfb opened merge request !17085

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

dcam’s picture

dcam commented

In my opinion this didn't qualify as a bug report that can omit a test. So I added one.

mfb’s picture

mfb
they or he
commented

Thanks @dcam, test looks good (GitLab tells me I am "not authorized" to run the test-only changes job)

ironnuts’s picture

ironnuts commented

I have reviewed this issue. Could be a bit more detail in the IS. But it is a one liner fix. I do not have permission to run the test-only test in the pipeline. The pipeline is green.

Instead I locally git checkout'ed the unit test on main branch. The output LGTM:

===============================================================================================================
F 1 / 1 (100%)

Time: 00:00.010, Memory: 8.00 MB

There was 1 failure:

1) Drupal\Tests\Core\Command\DrupalConsoleLoggerTest::testToPsr3Exception
Failed asserting that exception of type "InvalidArgumentException" matches expected exception "Psr\Log\InvalidArgumentException". Message was: "Invalid log level: -1000" at
/var/www/html/core/lib/Drupal/Core/Command/DrupalConsoleLogger.php:57
/var/www/html/core/tests/Drupal/Tests/Core/Command/DrupalConsoleLoggerTest.php:26

================================================================================================================

Changing to RTBTC.

ironnuts’s picture

ironnuts commented
Status: Needs review » Reviewed & tested by the community
mfb’s picture

mfb
they or he
commented
Issue summary: View changes
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Version: main » 11.4.x-dev
Status: Reviewed & tested by the community » Fixed

Committed and pushed 55148447cf6 to main and fba56e1cba4 to 11.x and a3448448f69 to 11.4.x. Thanks!

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.

  • longwave committed a3448448 on 11.4.x 
    fix: #3622446 DrupalConsoleLogger throws incorrect exception

By: mfb
By...

  • longwave committed fba56e1c on 11.x 
    fix: #3622446 DrupalConsoleLogger throws incorrect exception

By: mfb
By...

  • longwave committed 55148447 on main 
    fix: #3622446 DrupalConsoleLogger throws incorrect exception

By: mfb
By...
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Version: main » 11.4.x-dev
Status: Reviewed & tested by the community » Fixed

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.

godotislate’s picture

godotislate
he/him
commented
Status: Fixed » Needs work

This failed 11.4.x PHPStan: https://git.drupalcode.org/project/drupal/-/jobs/12163722

------ ----------------------------------------------------------------------- 
  Line   core/tests/Drupal/Tests/Core/Command/DrupalConsoleLoggerTest.php       
 ------ ----------------------------------------------------------------------- 
  25     Call to an undefined method                                            
         Drupal\Tests\Core\Command\DrupalConsoleLoggerTest::expectExceptionMes  
         sageIs().                                                              
         🪪  method.notFound                                                    
 ------ -----------------------------------------------------------------------

Thanks to @mherchel for flagging in Slack.

godotislate opened merge request !17110

godotislate’s picture

godotislate
he/him
commented
Status: Needs work » Needs review

11.4.x fix https://git.drupalcode.org/project/drupal/-/merge_requests/17110

ironnuts’s picture

ironnuts commented

LGTM. ::expectExceptionMessage() exists in core ::expectExceptionMessageIs() does not. Hence the PHPSTAN issue.

Simple fix has been applied: remove the 'Is'. Pipeline including PHPSTAN is green. RTBTC.

ironnuts’s picture

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

  • longwave committed 3d04522b on 11.4.x 
    fix: #3622446 DrupalConsoleLogger throws incorrect exception

By: mfb
By...
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed 3d04522b4eb to 11.4.x. Thanks!

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.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.