Problem/Motivation

See title

Proposed resolution

Fix

Remaining tasks

Review

User interface changes

None

API changes

None

Nomination for trivial patch of the month?

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Task
Unfrozen changes Unfrozen because it removes old/dead code
Prioritized changes The main goal of this issue is removing previously deprecated code.
CommentFileSizeAuthor
#10 various_tests_still-2365965-9.patch6.45 KBgaurav.pahuja
#7 various_tests_still-2365965-7.patch6.45 KBer.pushpinderrana
#7 interdiff-2365965-4-7.txt3.03 KBer.pushpinderrana
#4 various_tests_still-2365965-4.patch6.42 KBCristian.Andrei
contact-test.1.patch806 byteslarowlan
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran commented
Status: Needs review » Reviewed & tested by the community
Arla’s picture

Arla
they/them
CreditAttribution: Arla commented

Why don't we get rid of some more getInfo()'s in this issue too.

$ grep -l 'function getInfo' $(find . -type f -iname '*Test.php')
./core/modules/aggregator/src/Tests/FeedCacheTagsTest.php
./core/modules/aggregator/src/Tests/ItemCacheTagsTest.php
./core/modules/contact/src/Tests/ContactStorageTest.php
./core/modules/language/src/Tests/EntityDefaultLanguageTest.php
./core/modules/rdf/src/Tests/Field/LinkFieldRdfaTest.php
./core/modules/system/src/Tests/Entity/EntitySchemaTest.php
./core/modules/user/tests/src/Unit/UserAccessControlHandlerTest.php
./core/tests/Drupal/Tests/Component/Utility/SafeMarkupTest.php
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan commented
Title: ContactStorageTest still has getInfo method » Various tests still have getInfo method
Status: Reviewed & tested by the community » Needs work

Yes please

Cristian.Andrei’s picture

Cristian.Andrei CreditAttribution: Cristian.Andrei commented
FileSize
various_tests_still-2365965-4.patch6.42 KB

low hanging fruit, picked

Cristian.Andrei’s picture

Cristian.Andrei CreditAttribution: Cristian.Andrei commented
Status: Needs work » Needs review
Arla’s picture

Arla
they/them
CreditAttribution: Arla commented
Status: Needs review » Needs work

Nice.

+++ b/core/modules/aggregator/src/Tests/FeedCacheTagsTest.php
@@ -13,6 +13,7 @@
  * Tests the Feed entity's cache tags.
+ * @group aggregator

I would say there should be an empty line before the @group annotation in all these files. I cannot find any references to that in the coding standards, but since that's how every other test file looks afaics, I think we should keep to that.

er.pushpinderrana’s picture

er.pushpinderrana CreditAttribution: er.pushpinderrana commented
Status: Needs work » Needs review
FileSize
interdiff-2365965-4-7.txt3.03 KB
various_tests_still-2365965-7.patch6.45 KB

As per #6 updated patch.

Arla’s picture

Arla
they/them
CreditAttribution: Arla commented
Status: Needs review » Reviewed & tested by the community

I like.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/tests/Drupal/Tests/Component/Utility/SafeMarkupTest.php
@@ -13,18 +13,12 @@
+ * @group Drupal
+ *

Should be @group Utility and the new line is unnecessary.

gaurav.pahuja’s picture

gaurav.pahuja CreditAttribution: gaurav.pahuja commented
Status: Needs work » Needs review
FileSize
various_tests_still-2365965-9.patch6.45 KB 
+++ b/core/tests/Drupal/Tests/Component/Utility/SafeMarkupTest.php
@@ -13,18 +13,12 @@
+ * @group Drupal
+ *

Updated as per #9. Not attaching interdiff as this is minor change.

Changed to:

/**
  * Tests marking strings as safe.
  *
+ * @group Utility
  * @coversDefaultClass \Drupal\Component\Utility\SafeMarkup
  */
Valentine94’s picture

Valentine94
Location Kyiv, Ukraine
CreditAttribution: Valentine94 commented

Looks good, +1 RTBC

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
CreditAttribution: jhedstrom commented
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

I added a beta phase evaluation. RTBC since it removes a bunch of dead code we don't want to maintain.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Fixed

Committed eb68f25 and pushed to 8.0.x. Thanks!

Thanks for adding the beta evaluation for to the issue summary.

  • alexpott committed eb68f25 on 8.0.x 
    Issue #2365965 by er.pushpinderrana, larowlan, Cristian.Andrei, gaurav....

Status: Fixed » Closed (fixed)

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