There are a few erroneous comments in common.test and two functions that are named "UnitTest" that are not unit tests. The attached patch fixes them.

CommentFileSizeAuthor
#9 comment_cleanup_common_tests-1347836-d7-9.patch770 bytesAlbert Volkman
#5 comment_cleanup_common_tests-1347836-5480960.patch806 bytesLSU_JBob
common-test-comments.patch2 KBlinclark
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
Issue tags: +Novice
Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
Component: markup » base system
Issue tags: -Novice
tregeagle’s picture

tregeagle CreditAttribution: tregeagle commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +DDU2012

Patched fine, looks good.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Status: Reviewed & tested by the community » Needs work

Thanks for the cleanup! One standards fix and one question:

+++ b/core/modules/simpletest/tests/common.testundefined
@@ -345,7 +345,8 @@ class CommonURLUnitTest extends DrupalWebTestCase {
 /**
- * Tests for the check_plain(), filter_xss() and format_string() functions.
+ * Tests for the check_plain(), filter_xss(), format_string(), and check_url
+ * functions.

This should be one line, 80 chars or less. I'd suggest making the description a little more generic and moving the list of function names to a second paragraph if desired.

+++ b/core/modules/simpletest/tests/common.testundefined
@@ -1833,7 +1837,7 @@ class ValidUrlTestCase extends DrupalUnitTestCase {
-      'description' => "Performs tests on Drupal's valid url function.",
+      'description' => "Performs tests on Drupal's valid_url function.",

While we're changing this, do we want to add parens for valid_url()?

LSU_JBob’s picture

LSU_JBob CreditAttribution: LSU_JBob commented
Status: Needs work » Needs review
FileSize
comment_cleanup_common_tests-1347836-5480960.patch806 bytes

re-rolled, it seems that the function names have been taken care of.

picked up from the novice queue.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Status: Needs review » Reviewed & tested by the community

I just went through and confirmed all the other fixes were covered by #899444: Declutter "System" test group by applying coding standards fixes to common.test. Thanks @LSU_JBob!

Dries’s picture

Dries CreditAttribution: Dries commented
Status: Reviewed & tested by the community » Fixed

Committed to 8.x. Thanks.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Version: 8.x-dev » 7.x-dev
Status: Fixed » Patch (to be ported)
Issue tags: +Needs backport to D7

I think we can probably backport this cleanup too.

Albert Volkman’s picture

Albert Volkman CreditAttribution: Albert Volkman commented
Status: Patch (to be ported) » Needs review
FileSize
comment_cleanup_common_tests-1347836-d7-9.patch770 bytes

D7 backport.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Status: Needs review » Reviewed & tested by the community

Yep.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed to 7.x. Thanks!

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