When I run the Content Access Module Tests (no ACL) I get 22 failures. Most of them are caused by the following lines:

    // Create test user with seperate role
    $this->test_user = $this->drupalCreateUser();
    debug('test_user has roles ' . print_r($this->test_user->roles, TRUE));

    // Get the value of the new role
    // Needed in D7 because it's by default create two roles for new users
    // one role is Authenticated and the second is new default one
    // @see drupalCreateUser()
    foreach ($this->test_user->roles as $rid => $role) {
      if (!in_array($rid, array(DRUPAL_AUTHENTICATED_RID))) {
        $this->rid = $rid;
        break;
      }
    }

(I added the debug line.)

If you call drupalCreateUser() with a non-empty array of permissions, then it will create a new role with those permissions and assign that role to the new user, as well as the Authenticated User role. If you do not assign any permissions, then it does not create a new role. Furthermore, it assigns both Anonymous and Authenticated roles to the newly created user. (This looks like a bug to me, so I just created this issue: #1873606: drupalCreateUser() creates a user with Anonymous AND Authenticated roles..) Thus the above lines have the effect of setting $this->rid to 1 (the anonymous rid).

I can get it to pass 21 of 22 tests by adding array('access content') when calling drupalCreateUser(). I will attacha patch as soon as I have an issue number.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

benjifisher’s picture

FileSize
877 bytes

The patch I described is attached.

benjifisher’s picture

Status: Active » Needs review
FileSize
20.97 KB

I fixed the remaining test by adding a permission to drupalCreateUser() in testOwnViewAccess(). The attached patch includes this fix. As long as I was editing the files, I could not resist fixing some formatting errors--mostly comment formatting and spaces around . (concatenation) operators.

One other change I made was to simplify (in one place) how the code looks for the new rid.

I did not study the logic of the tests: maybe giving test users the 'access content' permission was the wrong idea. But the tests pass, so I am satisifed.

benjifisher’s picture

bvanmeurs’s picture

Priority: Normal » Major

The tests are still failing in the dev version.

This module affects security so automated tests are very important.

This should be fixed. Maintainers, please have a look at the patches.

stefan.r’s picture

Yes this is still failing -- adding array('access content') fixes it (otherwise the code wrongly thinks the $test_user's role is anonymous)

@benjifisher's fix looks good :)

If it makes it easier for whomever can commit this, this 1-line patch should work as well.

benjifisher’s picture

@stefan.r: I think your patch in #5 is the same as mine in #1.

stefan.r’s picture

Status: Needs review » Active

Ah that was unintentional, HEAD must be the same a year ago as it is right now... :)

Let's see if we can get this committed.

stefan.r’s picture

Status: Active » Needs review
gisle’s picture

Status: Needs review » Needs work

The patch (ca-tests-fail-1873606-2.patch) applied cleanly against 7.x-1.x-dev.

I ran it on Ubuntu 18.04 (PHP 7.2), and got:
541 passes, 0 fails, 10 exceptions, and 185 debug messages

All 10 exceptions are due to testing with PHP 7.2:

Warning: count(): Parameter must be an array or an object that implements Countable

For background:
https://stackoverflow.com/questions/51594817/php-7-2-warning-count-param...

gisle’s picture

Rerolled the patch by benjifisher for PHP 7.2.

Running tests against 7.x-1.x-dev now produces:
541 passes, 0 fails, 0 exceptions, and 185 debug messages.

Please review.

gisle’s picture

PHP 7.2 test passed without fails locally, but failed here. Looks like the call to parent::setUp() is outdated. Trying again.

Please review.

gisle’s picture

I now see that the single failure are due to tests of the "Content Access Module with ACL Module Tests". I didn't test those locally.

AFAIK, the bits of Content Access that make use of the ACL project has never worked on Drupal 7 (it looks like it is only half-ported from Drupal 6). At least, I gave up on it ages ago and instead opted to use Flexi Access for ACL-based access control.

It might by a good idea to merge with in Flexi Access to get ACL working again in Content Access.

I am not going to fix "Content Access Module with ACL Module Tests", as the whole ACL-thing needs fixing (not just the tests).

gisle’s picture

gisle’s picture

Status: Needs review » Reviewed & tested by the community
gisle’s picture

Status: Reviewed & tested by the community » Fixed

Fixed in 7.x-1.x

benjifisher’s picture

I have not used Contact Access in years, but it is nice to see this issue finally get fixed. Thanks for maintaining the module!

Status: Fixed » Closed (fixed)

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