387 lines
18 KiB
HTML
387 lines
18 KiB
HTML
<!DOCTYPE html>
|
||
<html class="no-js" lang="en-GB">
|
||
<head>
|
||
<title>Updating Override Node Options Tests | Oliver Davies</title>
|
||
|
||
<meta charset="UTF-8">
|
||
<meta http-equiv="X-UA-Compatible" content="IE=edge">
|
||
<meta name="viewport" content="width=device-width, initial-scale=1">
|
||
|
||
|
||
<meta property="og:url" content="https://opdavies.github.io/oliverdavies.uk/blog/2017/05/05/updating-override-node-options-tests">
|
||
<meta property="og:title" content="Updating Override Node Options Tests"/>
|
||
|
||
|
||
|
||
<meta property="og:image" content="https://opdavies.github.io/oliverdavies.uk/assets/images/me-precedent.jpg"/>
|
||
<meta property="og:image:height" content="327"/>
|
||
<meta property="og:image:type" content="image/jpg">
|
||
<meta property="og:image:width" content="327"/>
|
||
|
||
|
||
|
||
<link rel="stylesheet" href="https://opdavies.github.io/oliverdavies.uk/assets/css/main.css">
|
||
<link rel="stylesheet" href="https://opdavies.github.io/oliverdavies.uk/assets/css/blog-post.css">
|
||
|
||
<link rel="apple-touch-icon" href="/assets/images/me-precedent.jpg?s=57" sizes="57x57">
|
||
<link rel="apple-touch-icon" href="/assets/images/me-precedent.jpg?s=114" sizes="114x114">
|
||
<link rel="apple-touch-icon" href="/assets/images/me-precedent.jpg?s=72" sizes="72x72">
|
||
<link rel="apple-touch-icon" href="/assets/images/me-precedent.jpg?s=144" sizes="144x144">
|
||
<link rel="apple-touch-icon" href="/assets/images/me-precedent.jpg?s=60" sizes="60x60">
|
||
<link rel="apple-touch-icon" href="/assets/images/me-precedent.jpg?s=120" sizes="120x120">
|
||
<link rel="apple-touch-icon" href="/assets/images/me-precedent.jpg?s=76" sizes="76x76">
|
||
<link rel="apple-touch-icon" href="/assets/images/me-precedent.jpg?s=152" sizes="152x152">
|
||
|
||
<link rel="icon" href="/assets/images/me-precedent.jpg?s=160" sizes="160x160">
|
||
<link rel="icon" href="/assets/images/me-precedent.jpg?s=96" sizes="96x96">
|
||
<link rel="icon" href="/assets/images/me-precedent.jpg?s=32" sizes="32x32">
|
||
<link rel="icon" href="/assets/images/me-precedent.jpg?s=16" sizes="16x16">
|
||
</head>
|
||
<body class="">
|
||
<nav class="navbar navbar-inverse navbar-fixed-top">
|
||
<div class="container">
|
||
<div class="navbar-header">
|
||
<button type="button" class="navbar-toggle collapsed" data-toggle="collapse" data-target="#navbar" aria-expanded="false" aria-controls="navbar">
|
||
<span class="sr-only">Toggle navigation</span>
|
||
<span class="icon-bar"></span>
|
||
<span class="icon-bar"></span>
|
||
<span class="icon-bar"></span>
|
||
</button>
|
||
<a class="navbar-brand" href="https://opdavies.github.io/oliverdavies.uk/">Oliver Davies</a>
|
||
</div>
|
||
|
||
<div id="navbar" class="collapse navbar-collapse" role="navigation">
|
||
<ul class="nav navbar-nav">
|
||
<li class="">
|
||
<a href="/">About</a>
|
||
</li>
|
||
|
||
<li class="">
|
||
<a href="/experience">Experience</a>
|
||
</li>
|
||
|
||
<li class="">
|
||
<a href="/testimonials">Testimonials</a>
|
||
</li>
|
||
|
||
<li class="">
|
||
<a href="/talks">Talks</a>
|
||
</li>
|
||
|
||
<li class="active">
|
||
<a href="/blog">Blog</a>
|
||
</li>
|
||
|
||
<li class="">
|
||
<a href="/contact">Contact</a>
|
||
</li>
|
||
</ul>
|
||
</div> </div>
|
||
</nav>
|
||
|
||
<div class="container">
|
||
<div class="row">
|
||
<main class="col-md-9">
|
||
<h1>Updating Override Node Options Tests</h1>
|
||
|
||
<p class="posted text-light">5th May 2017</p>
|
||
|
||
<p>Recently, I reviewed <a href="https://www.drupal.org/node/974730">a patch</a> in the <a href="https://www.drupal.org/project/override_node_options">Override Node Options</a> 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 <code>administer nodes</code> 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.</p>
|
||
|
||
<p>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.</p>
|
||
|
||
<p>This post documents the steps that I took to update the tests to add the proposed functionality.</p>
|
||
|
||
<h2 id="reviewing-the-existing-tests">Reviewing the Existing Tests</h2>
|
||
|
||
<p>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 <code>php scripts/run-tests.sh --class OverrideNodeOptionsTestCase</code>.</p>
|
||
|
||
<pre><code class="language-markup">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
|
||
</code></pre>
|
||
|
||
<p>After confirming that the existing tests still passed, I reviewed them to see what could be re-used.</p>
|
||
|
||
<p>This is one of the original tests:</p>
|
||
|
||
<pre><code class="language-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));
|
||
}
|
||
</code></pre>
|
||
|
||
<p>The first part of the test is creating and logging in a user with some content type specific override permissions (<code>$this->adminUser</code>), 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 (<code>$this->normalUser</code>), which is created in the <code>setUp()</code> class' method.</p>
|
||
|
||
<p>To test the new "all types" permissions, I created another user to test against called <code>$generalUser</code> and run the first part of the tests in a loop.</p>
|
||
|
||
<h2 id="beginning-to-refactor-the-tests">Beginning to Refactor the Tests</h2>
|
||
|
||
<p>With the tests passing, I was able to start refactoring.</p>
|
||
|
||
<pre><code class="language-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.
|
||
...
|
||
}
|
||
</code></pre>
|
||
|
||
<p>I started with a small change, renaming <code>$this->adminUser</code> to <code>$specificUser</code> 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.</p>
|
||
|
||
<p>After that change, I ran the tests again to check that everything still worked.</p>
|
||
|
||
<h2 id="adding-failing-tests">Adding Failing Tests</h2>
|
||
|
||
<p>The next step is to start testing the new permissions.</p>
|
||
|
||
<pre><code class="language-php">...
|
||
|
||
$generalUser = $this->drupalCreateUser(array());
|
||
|
||
foreach (array($specificUser, $generalUser) as $account) {
|
||
$this->drupalLogin($account);
|
||
|
||
// Test all the things.
|
||
}
|
||
</code></pre>
|
||
|
||
<p>I added a new <code>$generalUser</code> 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.</p>
|
||
|
||
<pre><code class="language-markup">Test summary
|
||
------------
|
||
|
||
Override node options 183 passes, 6 fails, 0 exceptions, and 49 debug messages
|
||
|
||
Test run duration: 28 sec
|
||
</code></pre>
|
||
|
||
<p>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.</p>
|
||
|
||
<p>TODO: Add another example.</p>
|
||
|
||
<h2 id="gotchas">Gotchas</h2>
|
||
|
||
<p>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 <code>$generalUser</code> and the test still failed, I noticed that the</p>
|
||
|
||
<p>This was fixed by adding the extra code into <code>override_node_options.module</code>.</p>
|
||
|
||
<pre><code class="language-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');
|
||
</code></pre>
|
||
|
||
<p>The other issue that I found was within <code>testNodeRevisions</code>. <code>assertNodeFieldsUpdated()</code> was failing after being put in a loop as the <code>vid</code> was not the same as what was expected.</p>
|
||
|
||
<p>Note: You can get more verbose output from <code>run-tests.sh</code> by adding the <code>--verbose</code> option.</p>
|
||
|
||
<blockquote>
|
||
<p>Node vid was updated to '3', expected 2.</p>
|
||
</blockquote>
|
||
|
||
<pre><code class="language-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));
|
||
+ }
|
||
</code></pre>
|
||
|
||
<p>The crucial part of this change was the addition of <code>$node = node_load($this->node->nid, NULL, TRUE);</code> to ensure that the latest version of the node was loaded during each loop.</p>
|
||
|
||
<h2 id="conclusion">Conclusion</h2>
|
||
|
||
<ul>
|
||
<li>Ensure that the existing tests were passing before starting to refactor.</li>
|
||
<li>Start with small changes and continue to run the tests to ensure that nothing has broken.</li>
|
||
<li>After the first change, I committed it as <code>WIP: Refactoring tests</code>, and used <code>git commit --amend --no-edit</code> to amend that commit each time I had refactored another test. After the last refactor, I updated the commit message.</li>
|
||
<li>It’s important to see tests failing before making them pass. This was achieved by initially assigning no permissions to <code>$generalUser</code> 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.</li>
|
||
</ul>
|
||
|
||
<p>With the refactoring complete, the number of passing tests increased from 142 to 213.</p>
|
||
|
||
<pre><code class="language-markup">Override node options 213 passes, 0 fails, 0 exceptions, and 60 debug messages
|
||
|
||
Test run duration: 25 sec
|
||
</code></pre>
|
||
|
||
<p><img src="/assets/images/blog/override-node-options-refactor-tests-new-passing.png" alt=""></p>
|
||
|
||
<p><a href="https://www.drupal.org/files/issues/interdiff_25712.txt">Here</a> are my full changes from the previous patch, where I added the new tests as well as some small refactors.</p>
|
||
|
||
<p class="tags">
|
||
Tags:
|
||
<a href="https://opdavies.github.io/oliverdavies.uk/blog/tags/drupal">drupal</a>, <a href="https://opdavies.github.io/oliverdavies.uk/blog/tags/drupal-modules">drupal-modules</a>, <a href="https://opdavies.github.io/oliverdavies.uk/blog/tags/drupal-planet">drupal-planet</a>, <a href="https://opdavies.github.io/oliverdavies.uk/blog/tags/testing">testing</a>, <a href="https://opdavies.github.io/oliverdavies.uk/blog/tags/drafts">drafts</a> </p>
|
||
<div class="post-pager is-flex">
|
||
<div class="is-half">
|
||
<a href="/blog/2017/05/05/fixing-drupal-simpletest-docker">
|
||
« Fixing Drupal SimpleTest issues inside Docker Containers
|
||
</a>
|
||
</div>
|
||
|
||
<div class="is-half text-right">
|
||
<a href="/blog/2017/05/15/drupalcamp-bristol-early-bird-tickets-sessions-sponsors">
|
||
DrupalCamp Bristol 2017 - Early Bird Tickets, Call for Sessions, Sponsors »
|
||
</a>
|
||
</div>
|
||
</div>
|
||
<div class="about-author">
|
||
<h2>About the Author</h2>
|
||
|
||
<img src="https://opdavies.github.io/oliverdavies.uk/assets/images/me-precedent.jpg" alt="Picture of Oliver" class="img-circle">
|
||
|
||
<p>Oliver Davies is a Web Developer, System Administrator and Drupal specialist based in the UK. He is a Senior Developer at <a href="https://microserve.io">Microserve</a> and also provides freelance consultancy services for Drupal websites, PHP applications and Linux servers.</p>
|
||
</div>
|
||
</main>
|
||
|
||
<div class="col-md-3">
|
||
<div class="panel badges text-center">
|
||
<a class="badge--da-member" href="https://assoc.drupal.org/membership" title="I’m a Drupal Association member.">
|
||
<img
|
||
src="https://opdavies.github.io/oliverdavies.uk/assets/images/da-individual-member.png"
|
||
alt="Drupal Association Individual Member"
|
||
width="152"
|
||
>
|
||
</a>
|
||
|
||
<a href="http://drupalcores.com/#opdavies">
|
||
<img
|
||
alt="I built Drupal 8 with hand holding a wrench on blue background"
|
||
src="https://opdavies.github.io/oliverdavies.uk/assets/images/drupal-8.jpg"
|
||
/>
|
||
</a>
|
||
|
||
<img
|
||
src="https://opdavies.github.io/oliverdavies.uk/assets/images/badges/acquia-certified-developer-drupal-8.png"
|
||
alt="Acquia Certified Developer - Drupal 8 Exam Badge"
|
||
height="147" width="147"
|
||
/>
|
||
|
||
<a href="http://conference.phpnw.org.uk/phpnw17">
|
||
<img src="https://opdavies.github.io/oliverdavies.uk/assets/images/badges/phpnw17.png" alt="">
|
||
</a>
|
||
</div>
|
||
<div class="availability panel panel-default">
|
||
<div class="panel-heading">Availability</div>
|
||
|
||
<div class="panel-body">
|
||
<p>
|
||
<i class="fa fa-thumbs-o-up text-warning"></i>
|
||
|
||
|
||
Currently have limited part-time capacity
|
||
</p>
|
||
<p>
|
||
<i class="fa fa-thumbs-o-down text-danger"></i>
|
||
|
||
Currently no spare full-time capacity.
|
||
|
||
</p>
|
||
</div>
|
||
</div>
|
||
</div>
|
||
|
||
</div> </div>
|
||
<footer class="container">
|
||
<p class="copyright">
|
||
© 2010-2017 Oliver Davies. Built with <a href="https://sculpin.io">Sculpin</a>.
|
||
</p>
|
||
|
||
<div class="meetups">
|
||
<h2>Things that I organise</h2>
|
||
<ul>
|
||
<li class="meetups--drupal-bristol">
|
||
<a href="http://www.drupalbristol.org.uk" title="Drupal Bristol">
|
||
<img
|
||
src="https://opdavies.github.io/oliverdavies.uk/assets/images/meetups/drupal-bristol.jpeg"
|
||
alt="Drupal Bristol logo"
|
||
>
|
||
</a>
|
||
</li>
|
||
<li class="meetups--drupalcamp-bristol">
|
||
<a href="http://www.drupalcampbristol.co.uk" title="DrupalCamp Bristol">
|
||
<img
|
||
src="https://opdavies.github.io/oliverdavies.uk/assets/images/meetups/drupalcamp-bristol.png"
|
||
alt="DrupalCamp Bristol logo"
|
||
>
|
||
</a>
|
||
</li>
|
||
<li class="meetups--phpsw">
|
||
<a href="http://phpsw.uk" title="PHPSW">
|
||
<img
|
||
src="https://opdavies.github.io/oliverdavies.uk/assets/images/meetups/phpsw.jpeg"
|
||
alt="PHPSW logo"
|
||
>
|
||
</a>
|
||
</li>
|
||
</ul>
|
||
</div>
|
||
</footer>
|
||
|
||
<script src="https://opdavies.github.io/oliverdavies.uk/assets/js/site.js"></script>
|
||
|
||
<script>(function(i,s,o,g,r,a,m){i['GoogleAnalyticsObject']=r;i[r]=i[r]||function(){ (i[r].q=i[r].q||[]).push(arguments)},i[r].l=1*new Date();a=s.createElement(o),m=s.getElementsByTagName(o)[0];a.async=1;a.src=g;m.parentNode.insertBefore(a,m) })(window,document,'script','//www.google-analytics.com/analytics.js','ga'); ga('create', 'UA-11967257-1', 'auto'); ga('send', 'pageview');</script>
|
||
|
||
</body>
|
||
</html>
|