240 lines
7.6 KiB
Markdown
240 lines
7.6 KiB
Markdown
---
|
||
title: Updating Override Node Options Tests
|
||
date: 2017-05-05
|
||
excerpt: ~
|
||
tags:
|
||
- drupal
|
||
- drupal-modules
|
||
- drupal-planet
|
||
- testing
|
||
draft: true
|
||
---
|
||
|
||
Recently, I reviewed [a patch][1] in the [Override Node Options][2] module issue
|
||
queue. For those not familiar with it, the module adds extra permissions for
|
||
node options like "authored by" and "published on" which are normally only
|
||
available to users with the `administer nodes` permission. What the patch does
|
||
is to optionally add another set of permissions that enable options for all
|
||
content types - e.g. "override published option for all node types", in addition
|
||
to or instead of the content type specific ones.
|
||
|
||
It was quite an old issue and the latest patch needed to be re-rolled due to
|
||
merge conflicts, but the existing tests still passed. Though as no new tests
|
||
were added for the new functionality, these needed to be added before I
|
||
committed it.
|
||
|
||
## Reviewing the Existing Tests
|
||
|
||
The first thing to do was to run the existing tests and check that they still
|
||
passed. I do this on the command line by typing
|
||
`php scripts/run-tests.sh --class OverrideNodeOptionsTestCase`.
|
||
|
||
```
|
||
Drupal test run
|
||
---------------
|
||
|
||
Tests to be run:
|
||
- Override node options (OverrideNodeOptionsTestCase)
|
||
|
||
Test run started:
|
||
Saturday, April 29, 2017 - 14:44
|
||
|
||
Test summary
|
||
------------
|
||
|
||
Override node options 142 passes, 0 fails, 0 exceptions, and 38 debug messages
|
||
|
||
Test run duration: 32 sec
|
||
```
|
||
|
||
After confirming that the existing tests still passed, I reviewed them to see
|
||
what could be re-used.
|
||
|
||
This is one of the original tests:
|
||
|
||
```php
|
||
/**
|
||
* Test the 'Authoring information' fieldset.
|
||
*/
|
||
protected function testNodeOptions() {
|
||
$this->adminUser = $this->drupalCreateUser(array(
|
||
'create page content',
|
||
'edit any page content',
|
||
'override page published option',
|
||
'override page promote to front page option',
|
||
'override page sticky option',
|
||
'override page comment setting option',
|
||
));
|
||
$this->drupalLogin($this->adminUser);
|
||
|
||
$fields = array(
|
||
'status' => (bool) !$this->node->status,
|
||
'promote' => (bool) !$this->node->promote,
|
||
'sticky' => (bool) !$this->node->sticky,
|
||
'comment' => COMMENT_NODE_OPEN,
|
||
);
|
||
$this->drupalPost('node/' . $this->node->nid . '/edit', $fields, t('Save'));
|
||
$this->assertNodeFieldsUpdated($this->node, $fields);
|
||
|
||
$this->drupalLogin($this->normalUser);
|
||
$this->assertNodeFieldsNoAccess($this->node, array_keys($fields));
|
||
}
|
||
```
|
||
|
||
The first part of the test is creating and logging in a user with some content
|
||
type specific override permissions (`$this->adminUser`), and then testing that
|
||
the fields were updated when the node is saved. The second part is testing that
|
||
the fields are not visible for a normal user without the extra permissions
|
||
(`$this->normalUser`), which is created in the `setUp()` class' method.
|
||
|
||
To test the new "all types" permissions, I created another user to test against
|
||
called `$generalUser` and run the first part of the tests in a loop.
|
||
|
||
## Beginning to Refactor the Tests
|
||
|
||
With the tests passing, I was able to start refactoring.
|
||
|
||
```php
|
||
// Create a new user with content type specific permissions.
|
||
$specificUser = $this->drupalCreateUser(array(
|
||
'create page content',
|
||
'edit any page content',
|
||
'override page published option',
|
||
'override page promote to front page option',
|
||
'override page sticky option',
|
||
'override page comment setting option',
|
||
));
|
||
|
||
foreach (array($specificUser) as $account) {
|
||
$this->drupalLogin($account);
|
||
|
||
// Test all the things.
|
||
...
|
||
}
|
||
```
|
||
|
||
I started with a small change, renaming `$this->adminUser` to `$specificUser` to
|
||
make it clearer what permissions it had, and moving the tests into a loop so
|
||
that the tests can be repeated for both users.
|
||
|
||
After that change, I ran the tests again to check that everything still worked.
|
||
|
||
## Adding Failing Tests
|
||
|
||
The next step is to start testing the new permissions.
|
||
|
||
```php
|
||
...
|
||
|
||
$generalUser = $this->drupalCreateUser(array());
|
||
|
||
foreach (array($specificUser, $generalUser) as $account) {
|
||
$this->drupalLogin($account);
|
||
|
||
// Test all the things.
|
||
}
|
||
```
|
||
|
||
I added a new `$generalUser` to test the general permissions and added to the
|
||
loop, but in order to see the tests failing intially I assigned it no
|
||
permissions. When running the tests again, 6 tests have failed.
|
||
|
||
```
|
||
Test summary
|
||
------------
|
||
|
||
Override node options 183 passes, 6 fails, 0 exceptions, and 49 debug messages
|
||
|
||
Test run duration: 28 sec
|
||
```
|
||
|
||
Then it was a case of re-adding more permissions to the user and seeing the
|
||
number of failures decrease, confirming that the functionality was working
|
||
correctly.
|
||
|
||
TODO: Add another example.
|
||
|
||
## Gotchas
|
||
|
||
There was a bug that I found where a permission was added, but wasn't used
|
||
within the implementation code. After initially expecting the test to pass after
|
||
adding the permission to `$generalUser` and the test still failed, I noticed
|
||
that the
|
||
|
||
This was fixed by adding the extra code into `override_node_options.module`.
|
||
|
||
```diff
|
||
- $form['comment_settings']['#access'] |= user_access('override ' . $node->type . ' comment setting option');
|
||
+ $form['comment_settings']['#access'] |= user_access('override ' . $node->type . ' comment setting option') || user_access('override all comment setting option');
|
||
```
|
||
|
||
The other issue that I found was within `testNodeRevisions`.
|
||
`assertNodeFieldsUpdated()` was failing after being put in a loop as the `vid`
|
||
was not the same as what was expected.
|
||
|
||
Note: You can get more verbose output from `run-tests.sh` by adding the
|
||
`--verbose` option.
|
||
|
||
> Node vid was updated to '3', expected 2.
|
||
|
||
```diff
|
||
- $fields = array(
|
||
- 'revision' => TRUE,
|
||
- );
|
||
- $this->drupalPost('node/' . $this->node->nid . '/edit', $fields, t('Save'));
|
||
- $this->assertNodeFieldsUpdated($this->node, array('vid' => $this->node->vid + 1));
|
||
+ $generalUser = $this->drupalCreateUser(array(
|
||
+ 'create page content',
|
||
+ 'edit any page content',
|
||
+ 'override all revision option',
|
||
+ ));
|
||
+
|
||
+ foreach (array($specificUser, $generalUser) as $account) {
|
||
+ $this->drupalLogin($account);
|
||
+
|
||
+ // Ensure that we have the latest node data.
|
||
+ $node = node_load($this->node->nid, NULL, TRUE);
|
||
+
|
||
+ $fields = array(
|
||
+ 'revision' => TRUE,
|
||
+ );
|
||
+ $this->drupalPost('node/' . $node->nid . '/edit', $fields, t('Save'));
|
||
+ $this->assertNodeFieldsUpdated($node, array('vid' => $node->vid + 1));
|
||
+ }
|
||
```
|
||
|
||
The crucial part of this change was the addition of
|
||
`$node = node_load($this->node->nid, NULL, TRUE);` to ensure that the latest
|
||
version of the node was loaded during each loop.
|
||
|
||
## Conclusion
|
||
|
||
- Ensure that the existing tests were passing before starting to refactor.
|
||
- Start with small changes and continue to run the tests to ensure that nothing
|
||
has broken.
|
||
- After the first change, I committed it as `WIP: Refactoring tests`, and used
|
||
`git commit --amend --no-edit` to amend that commit each time I had refactored
|
||
another test. After the last refactor, I updated the commit message.
|
||
- It’s important to see tests failing before making them pass. This was achieved
|
||
by initially assigning no permissions to `$generalUser` so that the fails
|
||
failed and then added permissions and re-run the tests to ensure that the
|
||
failure count decreased with each new permission.
|
||
|
||
With the refactoring complete, the number of passing tests increased from 142
|
||
to 213.
|
||
|
||
```
|
||
Override node options 213 passes, 0 fails, 0 exceptions, and 60 debug messages
|
||
|
||
Test run duration: 25 sec
|
||
```
|
||
|
||
<img src="/images/blog/override-node-options-refactor-tests-new-passing.png" alt="">
|
||
|
||
[Here][3] are my full changes from the previous patch, where I added the new
|
||
tests as well as some small refactors.
|
||
|
||
[1]: https://www.drupal.org/node/974730
|
||
[2]: https://www.drupal.org/project/override_node_options
|
||
[3]: https://www.drupal.org/files/issues/interdiff_25712.txt
|