Tests for user_relationship_node_access

Hm, looks like your test is the wrong way round, the patch attempts to remove the file as opposed to add :)

+++ b/user_relationship_node_access/tests/user_relationship_node_access.test
@@ -0,0 +1,149 @@
+  }
+  /**

Could use a space between functions.

+++ b/user_relationship_node_access/tests/user_relationship_node_access.test
@@ -0,0 +1,149 @@
+
+    //clear cache
+    user_relationships_types_load(TRUE);
+
+    $rtype = new StdClass;

Comments should be formatted like this:

// Clear cache.

(space after //, first character upper case, . at the end).

+++ b/user_relationship_node_access/tests/user_relationship_node_access.test
@@ -0,0 +1,149 @@
+    //I know the nid will be 1, but how do I find this dynamically???
+    $this->drupalPost("node/1/delete",null,t('Delete'));

What I usually do is this:

    $this->drupalPost('node/add/simplenews', $english, ('Save'));
    $this->assertTrue(preg_match('|node/(\d+)$|', $this->getUrl(), $matches), 'Node created');
$english_node = node_load($matches[1]);

+++ b/user_relationship_node_access/tests/user_relationship_node_access.test
@@ -0,0 +1,149 @@
+    $this->drupalLogout($this->users['u1']);

No need to pass the user object to drupalLogout(), you only log out the currently logged in user anyway ;)

Both branch tests seem to be hung up. Re-triggered, should be picked up.

There are still a few missing (test methods) or incorrectly formatted (class properties should be documented with /** ... */ like methods, clear cache comments, ...) comments.

Apart from that, I would suggest to commit this asap, then it will be easier for bugs and other issues to extend the existing tests, rather than continue working on this patch. Can you provide a port for 7.x-1.x as well once you commited it? Will probably require quite a few changes to strings and url's, but should work the same way conceptually.

Here is some additional feedback, feel free to work on that after commiting a first patch.

+++ b/user_relationship_node_access/tests/user_relationship_node_access.test
@@ -0,0 +1,152 @@
+    $this->drupalLogin($this->users['u1']);
+    $this->drupalPost("node/{$matches[1]}/delete",null,t('Delete'));
+    $this->drupalLogout();

Might make sense to have an explicit test assertion that the delete actually worked. (e.g. add a assertText() on the confirmation message). you're currently only asserting this implicitly (as it would generate errors when drupalPost( can't find the form that should be submitted if you're on a 404 page).

Also, it is in general also useful to have tests for cases where something should not happen/work. In this case, you might want to set up a third user that is not related, or even better, in another relationship that should not have delete access.

That gives you much better coverage if the module works *correctly*.

+++ b/user_relationship_node_access/tests/user_relationship_node_access.test
@@ -0,0 +1,152 @@
+    $this->drupalLogin($this->users['u1']);
+    $this->drupalGet("node/{$matches[1]}");
+    $this->assertResponse(200, 'Can view node.');
+    $this->drupalLogout();

I guess this will always work because we do not deny access.

To make this check actually useful, you would need to additionally install a content access module which allows to deny access (or implement this feature in UR node access), this would also be necessary to check that users do not have access if they're not supposed to.

The problem with that is that the testbot doesn't know that the tests needs another module. I'm currently adding a empty test module to #1115956: Revamp relationship type name display which allows to add a dependency on i18n so that I will be able to write tests that depend on it, you could do something similar.

+++ b/user_relationship_node_access/tests/user_relationship_node_access.test
@@ -0,0 +1,152 @@
+  function tearDown() {
+    unset($this->rtypes);
+    parent::tearDown();
+  }

This isn't necessary as you already initialize $this->rtypes to an empty array in your setUp() method. So you can remove this method.