It would be a great help to have a set of tests for ACL. I'm a newbie as far as SimpleTest is concerned (I can't even get it to run — #699434: Workaround check for dom support), and I don't really know what is possible with ST.

If you have some experience with ST (and you can actually get it to run), then please provide some tests. Ideally, we'd end up with

  • each function in acl.module being called at least once,
  • its effect being verified independently, and on a higher level
  • creating some users and nodes, settings some ACLs, rebuilding permissions, and then verifying that each of Read/Update/Delete works correctly and independently.
  • Maybe we'll find additional functionality / test cases that ought to be nailed down.

Once this works on the stable D6 version, we'll be able to move it to the D7 version, find and fix the porting bugs, and quickly gain the confidence we need, especially for an API module, which is difficult to test interactively.

CommentFileSizeAuthor
#3 testing.patch4.53 KBNick_vh
#3 tests.zip1.66 KBNick_vh
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Nick_vh’s picture

I started by creating some simpletests for the D6 and D7 module. Currently I'm in a public place so not able to diff against CVS.

So the simpletests we want to create ->

function testNodeAclCreateDelete()
Includes acl_create_new_acl, acl_delete_acl, acl_get_id_by_name (READY)
function testNodeAclAddRemoveUsers()
Includes acl_add_user, acl_remove_user, acl_has_users, acl_get_id_by_name, acl_has_user, acl_get_uids
function testNodeAclAddRemoveFromNode()
Includes acl_node_add_acl, acl_node_remove_acl, acl_get_id_by_name
function testNodeAclAddRandomAndClearNode()
Includes acl_node_clear_acls, acl_get_id_by_name
function testNodeAclAddAndRemoveNode()
Included acl_node_delete
function testNodeAclAddAndRemoveUser()
Included acl_user_cancel
function testNodeAclPermissionCheck()
Includes independ check of the permissions by checking the grants table AND by trying to view the node as a unauthorised user and an authorized user for each of the 3 use cases (view, edit, delete)

Please comment if you need additional ones

salvis’s picture

Priority: Normal » Critical

I need some free time to really sit down and look into this...

Nick_vh’s picture

FileSize
1.66 KB
4.53 KB

In attachment the first write of the simpletest for D7
Included :
testNodeAclCreateDelete
testNodeAclSingleUserAddRemove

+ the skeletons for the other ones

It is easier to write the simpletests for D7 since this already comes with the framework built in. Please allow me to try and make this for D7 first and then backport (trough the help of the easy helper module in the test) it to D6

Caution : Not yet finished

PS.: I also couldn't diff a new directory, CVS is giving me some headache for that (It was already an headache for diffinf new files..) so I included the new directory as a ZIP

salvis’s picture

Version: 6.x-1.x-dev » 7.x-1.x-dev

Sorry for the long delay!

Re #1: Yes, your list looks pretty complete. The embedded form is difficult to test without an ACL client module. We can test this in the context of Forum Access.

Re #3: Conceptually it would be better to do D6 first, because we have a high confidence that ACL6 works correctly. This would serve to test the tests. But ACL is not overly complex, so it should well be possible to write the D7 tests from scratch and get them right. Just keep in mind that ACL7 is untested and likely to be buggy!

What is the "easy helper module"? Do you have a pointer?

Unfortunately, I can't test your tests, see #800428: Some core tests just hang...

I've created the tests directory as well as some dummy placeholder files. This should make diffing much easier.

salvis’s picture

Status: Active » Needs work

I've finally been able to solve my problem with SimpleTest (#851058: DOMDocument::loadHTML() fails) and I committed #3 as an intermediate step. Thanks!

Setting back to NW because many of the tests are still empty.

good_man’s picture

subscribe

  • salvis committed 352cce1 on 8.x-1.x
    #761696: Simpletests: provide some dummy files.
    
    
  • salvis committed e516035 on 8.x-1.x
    #761696: Add tests (work in progress), by Nick_vh.
    
    
salvis’s picture

salvis’s picture

Status: Needs work » Closed (outdated)