ImageFieldTestBase::createImageField() accepts a $field_settings parameter that is supposed to contain both the settings, and the 'description' for the field, next to each other in the same set of array keys.

That makes no sense, and would clash if there was a setting called 'description'.
Not sure that helper method even needs to bother with setting a field description ?

Comments

yched’s picture

Issue tags: +Novice

Good novice task :-)

gábor hojtsy’s picture

Title: Fix ImageFieldTestBase::createImageField() » ImageFieldTestBase::createImageField() takes a description in field settings errorneously
Status: Active » Needs review
StatusFileSize
new2.88 KB
new2.39 KB

Wanted to make sure to attend to this followup, so even though marked as Novice, here it is :) One version to move the argument (but only found one use). And one version to remove it (which as you said should hopefully be even better).

tibbsa’s picture

At the moment, there is no testing purpose for keeping the description, I don't believe. But for the sake of future extensibility, making it apparent that this is a "possibility" is probably not a bad thing. I'd vote for "moving" rather than "removing".

yched’s picture

Thanks @Gabor :-)
No big deal, but I'd vote for "remove". There's little point in keeping the description IMO.
If a specific test needs to specifically test something with the description (should be rare, the "description" is UI-only...), it can always set it itself. This is only a helper method for brevity, it's only normal that it doesn't have all the verbosity and features of calling the actual "create" functions itself.

+ FileFieldTestBase has a similar createFileField() that doesn't take a description, so... consistency++ :-)

So, RTBC for me for the "remove patch".

gábor hojtsy’s picture

StatusFileSize
new2.39 KB

Reuploading for easier committer review then.

yched’s picture

Status: Needs review » Reviewed & tested by the community

:-)

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

This issue is a normal bug fix, and doesn't include any disruptive changes, so it is allowed per https://www.drupal.org/core/beta-changes. Committed 49b1c1d and pushed to 8.0.x. Thanks!

  • alexpott committed 49b1c1d on 8.0.x
    Issue #2376899 by Gábor Hojtsy: ImageFieldTestBase::createImageField()...

Status: Fixed » Closed (fixed)

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