Clean up the added tests, by not calling procedural wrapper and fixing grammar [#2028067]

Follow up for #2027587: Add tests for custom blocks, contact forms, formats, shortcut listings and settings pages.

Updated: Comment #0

Problem/Motivation

Since the two tests issues have been committed,
#2027587: Add tests for custom blocks, contact forms, formats, shortcut listings and settings pages.
and
#2004710: Add tests for block, menu, vocabulary and views listings
@alexpott pointed out that the call to the procedural function strtolower from the class, should actually just directly call the OO method that it is a wrapper for. This will allow the dependencies to be lazy loaded. (We can check for these kind of things if using an IDE like phpstorm by right clicking on procedural functions in classes and seeing if they are simple wrappers around other class methods.)

Also, there is some comment cut and pasted that is

    // Set the machine name so later the translate link can be build.

build should be built.

See: #2027857: Blocks operations cannot be altered for an example of the use to be added and how to call the method strtolower.

Also, the setUp should be public, and the tests should be public.
see the simpletest standards: https://drupal.org/node/325974

Proposed resolution

use
Unicode::strtolower($this->randomName(16));
and fix the grammar.

Remaining tasks

create patch

User interface changes

No.

API changes

No.

Comment	File	Size	Author
#8	clean-up-added-tests-2028067-8.patch	4.65 KB	David Hernández
#8
#8	interdiff.txt	519 bytes	David Hernández
#6	clean-up-added-tests-2028067-6.patch	4.83 KB	David Hernández
#6
#6	interdiff.txt	679 bytes	David Hernández
#5	clean-up-added-tests-2028067-5.patch	4.49 KB	David Hernández
#5
#3	clean-up-added-tests-2028067-3.patch	2.59 KB	David Hernández
#3
#3	interdiff.txt	1.78 KB	David Hernández
#1	clean-up-added-tests-2028067-1.patch	1.85 KB	David Hernández
#1

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #0.0

YesCT CreditAttribution: YesCT commented 25 June 2013 at 21:19

Issue summary:

View changes

clarified it's the dependencies that can be autoloaded.

Comment #1

David Hernández CreditAttribution: David Hernández commented 26 June 2013 at 09:27

File	Size
clean-up-added-tests-2028067-1.patch	1.85 KB

I think that's what is needed.

Comment #2

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 26 June 2013 at 09:34

Status:

Active

» Needs work

+++ b/lib/Drupal/config_translation/Tests/ConfigTranslationListUITest.phpundefined
@@ -57,8 +58,8 @@ class ConfigTranslationListUITest extends WebTestBase {
-    // Set the machine name so later the translate link can be build.
-    $block_machine_name = drupal_strtolower($this->randomName(16));
+    // Set the machine name so later the translate link can be built.
+    $block_machine_name = Unicode::strtolower($this->randomName(16));

I think later should go later. As in "can be built later" not "later... can be built".

@YesCT also says this appears in multiple places, not just this one.

+++ b/lib/Drupal/config_translation/Tests/ConfigTranslationListUITest.phpundefined
@@ -148,5 +149,4 @@ class ConfigTranslationListUITest extends WebTestBase {
-
-}
+}
\No new line at end of file

The newline should be kept at the end of the file :)

Comment #3

David Hernández CreditAttribution: David Hernández commented 26 June 2013 at 14:46

Status:

Needs work

» Needs review

File	Size
interdiff.txt	1.78 KB
clean-up-added-tests-2028067-3.patch	2.59 KB

Ok, I fixed the comment, added the newline and replaced the rest of strtolower functions I found.

Comment #4

26 June 2013 at 14:51

Status:

Needs review

» Needs work

The last submitted patch, clean-up-added-tests-2028067-3.patch, failed testing.

Comment #5

David Hernández CreditAttribution: David Hernández commented 26 June 2013 at 15:14

Status:

Needs work

» Needs review

File	Size
clean-up-added-tests-2028067-5.patch	4.49 KB

Re-rolling

Comment #6

David Hernández CreditAttribution: David Hernández commented 26 June 2013 at 15:17

File	Size
interdiff.txt	679 bytes
clean-up-added-tests-2028067-6.patch	4.83 KB

Missed one change after the reroll. Here's the correct one.

Comment #7

vijaycs85

London, UK

CreditAttribution: vijaycs85 commented 26 June 2013 at 16:35

All good except one minor.

+++ b/lib/Drupal/config_translation/Tests/ConfigTranslationListUITest.phpundefined
@@ -266,5 +267,4 @@ class ConfigTranslationListUITest extends WebTestBase {
   }
-
 }

Needs a line here as per https://drupal.org/node/608152#indenting. otherwise, it is RTBC for me.