Clean up tests

+++ b/tests/skinr.test
@@ -600,7 +600,8 @@ class SkinrDisplayTestCase extends DrupalWebTestCase {
-  protected $profile = 'testing';
+  // @todo Requires http://drupal.org/node/913086
+  // protected $profile = 'testing';

See #913086: Allow modules to provide default configuration for running tests

You cannot use the Testing installation profile, if the test relies on certain default configuration that is only setup by the Standard installation profile.

In this particular test case, all of the drupalCreateNode() calls should have bailed out with errors...? That is, because the testing profile does not setup any node types by default.

+++ b/tests/skinr.test
@@ -611,28 +612,29 @@ class SkinrRulesApiTestCase extends DrupalWebTestCase {
-    parent::setUp(array('skinr'));
-
-    // Enable php module.
-    module_enable(array('php'));
+    parent::setUp(array('skinr', 'php'));

parent::setUp() installs Drupal core and also installs/enables the passed in modules. No need to do this manually (except for rare edge-cases).

+++ b/tests/skinr.test
@@ -611,28 +612,29 @@ class SkinrRulesApiTestCase extends DrupalWebTestCase {
-    $this->nodes['article'] = $this->drupalCreateNode(array('type' => 'article', 'title' => 'Article node'));
-    $this->nodes['page']    = $this->drupalCreateNode(array('type' => 'page', 'title' => 'Page node'));
+    $this->article = $this->drupalCreateNode(array(
+      'type' => 'article',
+      'title' => 'Article node',
+    ));
+    $this->page = $this->drupalCreateNode(array(
+      'type' => 'page',
+      'title' => 'Page node',
+    ));

If your test only needs one node, make it

$this->node

If you need multiple, either use

$this->nodes[] = ...

or when needing more explicit access, use

$this->article
$this->page

or

$this->article_node
$this->page_node

+++ b/tests/skinr.test
@@ -611,28 +612,29 @@ class SkinrRulesApiTestCase extends DrupalWebTestCase {
-    $this->users['normal_user'] = $this->drupalCreateUser(array());
+    $this->web_user = $this->drupalCreateUser(array());

We commonly use the following properties:

$this->web_user: A user with regular permissions.

$this->admin_user: A user with administrative permissions.

+++ b/tests/skinr.test
@@ -644,74 +646,64 @@ class SkinrRulesApiTestCase extends DrupalWebTestCase {
-    $this->assertTrue(skinr_rule_is_visible($rule->rid), 'The rule is visible on the front page.');
...
+    $this->assertTrue(skinr_rule_is_visible($rule->rid, $front), 'Rule is visible on front page.');

Keep the assertion messages short and to the point.

+++ b/tests/skinr.test
@@ -644,74 +646,64 @@ class SkinrRulesApiTestCase extends DrupalWebTestCase {
-    $rule->roles = array(2 => '2');
+    $rule->roles = array(DRUPAL_AUTHENTICATED_RID => DRUPAL_AUTHENTICATED_RID);

Always use the constant when an ID refers to a constant.

+++ b/tests/skinr.test
@@ -644,74 +646,64 @@ class SkinrRulesApiTestCase extends DrupalWebTestCase {
-    // @todo Is there a way to test role API functionality without temporarily
-    //   changing global user?
-    global $user;
-    $current_user = $user;
...
-    $user = $this->users['normal_user'];
...
+    $user = $this->web_user;
+    $this->assertTrue(skinr_rule_is_visible($rule->rid, $front), 'Role limited rule is visible for authenticated users.');
...
     $user = drupal_anonymous_user();
-    $this->assertFalse(skinr_rule_is_visible($rule->rid, $front), "The rule is not visible for an anonymous user when it has a role filter set to 'authenticated user'.");
...
-    $user = $current_user;

Testing API functions in a DrupalWebTestCase directly from the test method is a special case:

Fundamentally, you need to understand that the test method is executed in the scope of the parent site (running the tests), but which is using the database of the child site (which is only executed when a test method performs a GET or POST request).

When a test is setup, the global $user is replaced with a stub user, which doesn't map to any user anywhere.

If you do a $this->drupalCreateUser(), then a new user record is created in the child site.

If you do a $this->drupalLogin(), then the internal browser of SimpleTest is used to log in to the child site. This means you have a user session in the child site. But NOT in the parent site (where the test method is executed).

The user session affects all requests against the child site; i.e., all $this->drupalGet() and $this->drupalPost().

However, the global $user in the parent site is not affected. You need to replace it manually.

Likewise, clearing caches in the client site does not clear caches in the parent site. Same applies to setting or deleting variables, etc.pp.

Not sure whether this is documented somewhere on drupal.org, but it definitely should be. Feel free to create a handbook page out of this, if you want.

+++ b/tests/skinr.test
@@ -644,74 +646,64 @@ class SkinrRulesApiTestCase extends DrupalWebTestCase {
-    $rule->pages = "<?php\nreturn FALSE;\n?>";
+    $rule->pages = '<?php
+return FALSE;
+?>';

In 99% of all cases, \n in a string can be replaced with a real linefeed. Also doesn't need double-quotes.

This is probably RTBC. However, note that I've only "fixed" this particular test case.