Problem/Motivation

In ::display() --

      $this->assertEquals('status', $type, $message);

PHPUnit now expects a string for $message.

But when MigrateExecutable does this:

      $this->message->display(
        $this->t('Migration failed with source plugin exception: @e in @file line @line', [
          '@e' => $e->getMessage(),
          '@file' => $e->getFile(),
          '@line' => $e->getLine(),
        ]), 'error');

It's calling MigrateTestBase::display() -- presumably because the test class is being used as a test version of whatever handles a message.

This means that if a migration fails in a test, the real failure is obscured by a TypeError.

Steps to reproduce

Proposed resolution

In \Drupal\Tests\migrate\Kernel\MigrateTestBase::display cast $message to a string.

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3552984

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:

Comments

joachim created an issue. See original summary.

quietone’s picture

Issue summary: View changes

Yes, I could replicate this. The MigrateMessageInterface defines $message as a string, maybe that should be changed.

But for the assertion we can just cast $message to a string.

joachim’s picture

Issue tags: +Novice

I'm concerned there could be other places where migrate messages are misused, but let's fix this as a novice issue.

sanket.tale made their first commit to this issue’s fork.

u7aro’s picture

Issue tags: +Nara2025

The Drupal Contribution Mentoring team is triaging issues for DrupalCon Nara 2025, and we are reserving this issue for Mentored Contribution during the event.

After November 19, this issue returns to being open to all. Thanks!

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

priyansu18’s picture

Status: Active » Needs review
joachim’s picture

Status: Needs review » Reviewed & tested by the community

Thanks! RTBC.

sujal kshatri made their first commit to this issue’s fork.

benjifisher’s picture

@sujal kshatri:

It looks as though you accidentally added an empty commit to the 11.x branch in the issue fork. I am not sure what you were trying to do, but that commit will not have any effect on this issue. The relevant branch is 3552984-migratetestbasedisplay-causes-a.

The next time you update an issue, please leave a comment explaining what you are trying to do.

catch’s picture

Version: 11.x-dev » 11.3.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to main, 11.x and 11.3.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.

  • catch committed c72febb0 on 11.3.x
    fix: #3552984 MigrateTestBase::display() causes a TypeError if a test...

  • catch committed ae170d7d on 11.x
    fix: #3552984 MigrateTestBase::display() causes a TypeError if a test...

  • catch committed ccd5164e on main
    fix: #3552984 MigrateTestBase::display() causes a TypeError if a test...

Status: Fixed » Closed (fixed)

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