Re-add old blog posts from Astro
This commit is contained in:
parent
076239fa25
commit
051e154c65
178 changed files with 13479 additions and 7 deletions
239
source/_posts/updating-override-node-options-tests.md
Normal file
239
source/_posts/updating-override-node-options-tests.md
Normal file
|
@ -0,0 +1,239 @@
|
|||
---
|
||||
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
|
Loading…
Add table
Add a link
Reference in a new issue