Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
AssertLegacyTrait::assertOptionByText() is deprecated in drupal:8.4.0 and is removed from drupal:10.0.0. Use $this->assertSession()->optionExists() instead. See https://www.drupal.org/node/3129738
Proposed resolution
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Comment | File | Size | Author |
---|---|---|---|
#15 | interdiff-3139422-12-15.txt | 1.77 KB | sja112 |
#15 | 3139422-15.patch | 5.8 KB | sja112 |
#4 | interdiff-3-4.txt | 1015 bytes | jungle |
#4 | 3139422-4.patch | 6.17 KB | jungle |
Comments
Comment #2
jungleAdding the "Deprecated assertions" tag to add this into the kanban board https://contribkanban.com/board/Deprecatedassertions
Comment #3
bibliophileaxeComment #4
jungleThanks, @akanksha-hp!
\Drupal\KernelTests\AssertContentTrait::assertOptionByText()
exists.Removing it directly here is a BC change so that we can not do it here, right? File an issue to deprecate it, and adding a test?
Comment #5
mondrakeThese would need to get the t() calls removed, too. But I suppose it's a follow-up.
Other than that, it's RTBC for me.
Comment #6
mondrake#4.2 I do not think that needs to be deprecated - it's a separate implementation, for the kernel tests. What we are removing here are the Simpletest legacy for functional/javascript tests. Now - that does not seem to be used for kernel tests either, though... maybe a separate issue might investigate which of the
\Drupal\KernelTests\AssertContentTrait
methods are really needed?Comment #7
mondrakeActually, maybe we do not need the separate test. Just add the @expectedDeprecation to the docblock of
testFieldAssertsForOptions
, without replacing the calls in the method.Comment #8
xjmGood catch on #7.
Agreed on #5 also; I think we should have a separate issue to stop translating assertion messages. There's #2552067: Remove t() from assertion messages in tests, which is very old (still using
SafeMarkup::format()
, of all things). That said, it might be better to do the issues to remove unnecessary assertion messages first, since that will also get rid of many of thet()
, and then we can proceed with othert()
using on strings in value comparisons. In any case, best not to change here.Comment #9
mondrake#5 that’s not about assertion messages, it’s the text to be found in the option. The method dropped the assertion message argument already.
Comment #10
xjm@mondrake Well I did refer to both. :) Just could not find the issue for removing other unnecessary translation use. But yeah, I see now that there's no messages in the patch.
Comment #11
sja112 CreditAttribution: sja112 at Srijan | A Material+ Company for Srijan | A Material+ Company, Drupal India Association commentedComment #12
sja112 CreditAttribution: sja112 at Srijan | A Material+ Company for Srijan | A Material+ Company, Drupal India Association commentedAddressing #7.
Comment #14
sja112 CreditAttribution: sja112 at Srijan | A Material+ Company for Srijan | A Material+ Company, Drupal India Association commentedComment #15
sja112 CreditAttribution: sja112 at Srijan | A Material+ Company for Srijan | A Material+ Company, Drupal India Association commentedRealized the mistake I was making in #12. Corrected it in this patch.
Comment #16
daffie CreditAttribution: daffie commentedAll occurrences of
$this->assertOptionByText()
have been replaced.Deprecation message testing has been added.
The deprecation message suppression has been removed.
Followup created. See: #3144732: Remove the calls to t() from $this->assertSession()->optionExists().
The point from @mondrake in comment #7 has been addressed.
All code changes look good to me.
For me it is RTBC.
Comment #17
alexpottCommitted 803a34b and pushed to 9.1.x. Thanks!
Backported the test changes and not the deprecation changes to 8.9.x and 9.0.x.
Committed and pushed 5a289071ee to 9.0.x and 69dd75d974 to 8.9.x. Thanks!