Problem/Motivation

See parent(s).

All the simple conversions are for #3402292: Fix strict type errors: Convert FormattableMarkup to strings (simple replacement) in core/tests/Drupal/KernelTests/* and #3402293: Fix strict type errors: Convert FormattableMarkup to strings (simple replacement) in core/modules/*/tests/src/Kernel/*. This issue is for all the more complicated places where we need rewrite how the message is generated. If that's still too huge a set of changes, we can split this one up futher.

Steps to reproduce

Proposed resolution

Convert "everything else" from FormattableMarkup into strings (e.g. with new local variables, other simplifications, etc).

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#16 3402294-nr-bot.txt90 bytesneeds-review-queue-bot

Issue fork drupal-3402294

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

dww created an issue. See original summary.

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

mstrelan’s picture

Status: Active » Needs work

Split these changes from the parent issue, marking NW to make readability improvements.

mstrelan’s picture

Status: Needs work » Needs review

Addressed feedback

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

All feedback/threads appear to be addressed.

mstrelan’s picture

Status: Reviewed & tested by the community » Needs work

MR needs rebasing

mstrelan’s picture

Status: Needs work » Needs review

Rebased, seems like some of the changes were already addressed in a related issue.

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Rebase seems good.

quietone’s picture

Status: Reviewed & tested by the community » Needs work

I found a stray character and left a question about using ...$args.

mstrelan’s picture

Status: Needs work » Needs review

Fixed the stray % character. Perhaps we should get rid of the $args array in those sprintf calls and just add the values directly?

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Perhaps we should get rid of the $args array in those sprintf calls and just add the values directly?

Removing that sounds like it could be a follow up as that could lead down a rabbit hole (I imagine)

quietone’s picture

Status: Reviewed & tested by the community » Needs work

I don't this it is a rabbit hole for reasons I mention in the MR. I wanted to use the suggestion feature for these two changes but it wasn't working for me \Drupal\Tests\system\Kernel\Form\ProgrammaticTest::doSubmitForm so I abandoned that idea.

mstrelan’s picture

Do we even need a $message here? Would make all these issues so much easier. Anyway, that's a topic for another day.

quietone’s picture

Status: Needs work » Needs review

I agree, most of these don't need a message.

To get this moving again I made the changes I suggested.

needs-review-queue-bot’s picture

Status: Needs review » Needs work
StatusFileSize
new90 bytes

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

quietone’s picture

Status: Needs work » Needs review

Arg, I've had trouble with git with this issue. Should be sorted now.

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs Review Queue Initiative

Appears feedback has been addressed

mstrelan’s picture

Thank you @quietone, have looked over the last 3 commits and am happy with these changes.

alexpott’s picture

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

Committed and pushed 955418c2df to 11.x and c9255990e0 to 10.3.x. Thanks!
Committed ac127f0 and pushed to 10.2.x. Thanks!

Backported to 10.2.x to keep tests aligned...

We should file a followup to change \Drupal\Tests\file\Kernel\FileManagedUnitTestBase::assertFileHooksCalled() to something like:

  public function assertFileHooksCalled($expected) {
    \Drupal::state()->resetCache();

    // Determine which hooks were called.
    $actual = array_keys(array_filter(file_test_get_all_calls()));

    // Determine if there were any expected that were not called.
    $uncalled = array_diff($expected, $actual);
    $this->assertEmpty($uncalled, sprintf('Expected hooks %s to be called but %s was not called.', implode(', ', $expected), implode(', ', $uncalled)));

    // Determine if there were any unexpected calls.
    $unexpected = array_diff($actual, $expected);
    $this->assertEmpty($unexpected, sprintf('Unexpected hooks were called: %s.', empty($unexpected) ? '(none)' : implode(', ', $unexpected)));
  }

The $this->assertTrue(FALSE is bizarre... but not added here.

  • alexpott committed ac127f07 on 10.2.x
    Issue #3402294 by mstrelan, quietone, dww, smustgrave: Fix strict type...

  • alexpott committed c9255990 on 10.3.x
    Issue #3402294 by mstrelan, quietone, dww, smustgrave: Fix strict type...

  • alexpott committed 955418c2 on 11.x
    Issue #3402294 by mstrelan, quietone, dww, smustgrave: Fix strict type...

Status: Fixed » Closed (fixed)

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