Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add unit testing capabilities for templates #23708

Merged
merged 2 commits into from
Apr 12, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions core/templates/403.php
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
<?php
// @codeCoverageIgnoreStart
if(!isset($_)) {//also provide standalone error page
require_once '../../lib/base.php';

$tmpl = new OC_Template( '', '403', 'guest' );
$tmpl->printPage();
exit;
}
// @codeCoverageIgnoreEnd
?>
<ul>
<li class='error'>
Expand Down
3 changes: 3 additions & 0 deletions core/templates/404.php
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
<?php
/** @var $_ array */
/** @var $l OC_L10N */
/** @var $theme OC_Theme */
// @codeCoverageIgnoreStart
if(!isset($_)) {//also provide standalone error page
require_once '../../lib/base.php';

$tmpl = new OC_Template( '', '404', 'guest' );
$tmpl->printPage();
exit;
}
// @codeCoverageIgnoreEnd
?>
<?php if (isset($_['content'])): ?>
<?php print_unescaped($_['content']) ?>
Expand Down
8 changes: 5 additions & 3 deletions lib/private/template.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@
*
*/

use OC\TemplateLayout;

require_once __DIR__.'/template/functions.php';

/**
Expand Down Expand Up @@ -218,11 +220,11 @@ public function addHeader($tag, $attributes, $text=null) {
* This function process the template. If $this->renderAs is set, it
* will produce a full page.
*/
public function fetchPage() {
$data = parent::fetchPage();
public function fetchPage($additionalParams = null) {
$data = parent::fetchPage($additionalParams);

if( $this->renderAs ) {
$page = new \OC\TemplateLayout($this->renderAs, $this->app);
$page = new TemplateLayout($this->renderAs, $this->app);

// Add custom headers
$headers = '';
Expand Down
58 changes: 34 additions & 24 deletions lib/private/template/base.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,17 @@
class Base {
private $template; // The template
private $vars; // Vars
private $l10n; // The l10n-Object
private $theme; // theme defaults

/** @var \OCP\IL10N */
private $l10n;

/** @var \OC_Defaults */
private $theme;

/**
* @param string $template
* @param string $requestToken
* @param \OC_L10N $l10n
* @param \OCP\IL10N $l10n
* @param \OC_Defaults $theme
*/
public function __construct($template, $requestToken, $l10n, $theme ) {
Expand All @@ -47,34 +51,36 @@ public function __construct($template, $requestToken, $l10n, $theme ) {
}

/**
* @param string $serverroot
* @param string $serverRoot
* @param string|false $app_dir
* @param string $theme
* @param string $app
* @return array
*/
protected function getAppTemplateDirs($theme, $app, $serverroot, $app_dir) {
protected function getAppTemplateDirs($theme, $app, $serverRoot, $app_dir) {
// Check if the app is in the app folder or in the root
if( file_exists($app_dir.'/templates/' )) {
return array(
$serverroot.'/themes/'.$theme.'/apps/'.$app.'/templates/',
return [
$serverRoot.'/themes/'.$theme.'/apps/'.$app.'/templates/',
$app_dir.'/templates/',
);
];
}
return array(
$serverroot.'/themes/'.$theme.'/'.$app.'/templates/',
$serverroot.'/'.$app.'/templates/',
);
return [
$serverRoot.'/themes/'.$theme.'/'.$app.'/templates/',
$serverRoot.'/'.$app.'/templates/',
];
}

/**
* @param string $serverroot
* @param string $serverRoot
* @param string $theme
* @return array
*/
protected function getCoreTemplateDirs($theme, $serverroot) {
return array(
$serverroot.'/themes/'.$theme.'/core/templates/',
$serverroot.'/core/templates/',
);
protected function getCoreTemplateDirs($theme, $serverRoot) {
return [
$serverRoot.'/themes/'.$theme.'/core/templates/',
$serverRoot.'/core/templates/',
];
}

/**
Expand Down Expand Up @@ -131,29 +137,33 @@ public function printPage() {

/**
* Process the template
* @return string
*
* @param array|null $additionalParams
* @return string This function processes the template.
*
* This function processes the template.
*/
public function fetchPage() {
return $this->load($this->template);
public function fetchPage($additionalParams = null) {
return $this->load($this->template, $additionalParams);
}

/**
* doing the actual work
*
* @param string $file
* @param array|null $additionalParams
* @return string content
*
* Includes the template file, fetches its output
*/
protected function load( $file, $additionalparams = null ) {
protected function load($file, $additionalParams = null) {
// Register the variables
$_ = $this->vars;
$l = $this->l10n;
$theme = $this->theme;

if( !is_null($additionalparams)) {
$_ = array_merge( $additionalparams, $this->vars );
if( !is_null($additionalParams)) {
$_ = array_merge( $additionalParams, $this->vars );
}

// Include
Expand Down
20 changes: 20 additions & 0 deletions tests/core/templates/templates.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<?php

namespace Tests\Core\Templates;

class Templates extends \Test\TestCase {

public function test403() {
$template = \OC::$SERVERROOT . '/core/templates/403.php';
$expectedHtml = "<ul><li class='error'>\n\t\tAccess forbidden<br><p class='hint'></p></li></ul>";
$this->assertTemplate($expectedHtml, $template);
}

public function test404() {
$template = \OC::$SERVERROOT . '/core/templates/404.php';
$href = \OC::$server->getURLGenerator()->linkTo('', 'index.php');
$expectedHtml = "<ul><li class='error'>\n\t\t\tFile not found<br><p class='hint'>The specified document has not been found on the server.</p>\n<p class='hint'><a href='$href'>You can click here to return to ownCloud.</a></p>\n\t\t</li></ul>";
$this->assertTemplate($expectedHtml, $template);
}

}
68 changes: 68 additions & 0 deletions tests/lib/testcase.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,15 @@

namespace Test;

use DOMDocument;
use DOMNode;
use OC\Command\QueueBus;
use OC\Files\Filesystem;
use OC\Template\Base;
use OC_Defaults;
use OCP\DB\QueryBuilder\IQueryBuilder;
use OCP\IDBConnection;
use OCP\IL10N;
use OCP\Security\ISecureRandom;

abstract class TestCase extends \PHPUnit_Framework_TestCase {
Expand All @@ -34,6 +39,8 @@ abstract class TestCase extends \PHPUnit_Framework_TestCase {

/** @var IDBConnection */
static protected $realDatabase = null;

/** @var bool */
static private $wasDatabaseAllowed = false;

/** @var array */
Expand Down Expand Up @@ -408,4 +415,65 @@ private function IsDatabaseAccessAllowed() {

return false;
}

/**
* @param string $expectedHtml
* @param string $template
* @param array $vars
*/
protected function assertTemplate($expectedHtml, $template, $vars = []) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't those methods live in tests/core/templates/templates.php

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to use this function in very app and all templates here in core - moving them into the base class made most sense to me - objections?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well my "objection" was that it is now available in all apps and tests. but I guess that is intended then, so fine

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so fine

is that a 👍 ❓


require_once __DIR__.'/../../lib/private/template/functions.php';

$requestToken = 12345;
$theme = new OC_Defaults();
/** @var IL10N | \PHPUnit_Framework_MockObject_MockObject $l10n */
$l10n = $this->getMockBuilder('\OCP\IL10N')
->disableOriginalConstructor()->getMock();
$l10n
->expects($this->any())
->method('t')
->will($this->returnCallback(function($text, $parameters = array()) {
return vsprintf($text, $parameters);
}));

$t = new Base($template, $requestToken, $l10n, $theme);
$buf = $t->fetchPage($vars);
$this->assertHtmlStringEqualsHtmlString($expectedHtml, $buf);
}

/**
* @param string $expectedHtml
* @param string $actualHtml
* @param string $message
*/
protected function assertHtmlStringEqualsHtmlString($expectedHtml, $actualHtml, $message = '') {
$expected = new DOMDocument();
$expected->preserveWhiteSpace = false;
$expected->formatOutput = true;
$expected->loadHTML($expectedHtml);

$actual = new DOMDocument();
$actual->preserveWhiteSpace = false;
$actual->formatOutput = true;
$actual->loadHTML($actualHtml);
$this->removeWhitespaces($actual);

$expectedHtml1 = $expected->saveHTML();
$actualHtml1 = $actual->saveHTML();
self::assertEquals($expectedHtml1, $actualHtml1, $message);
}


private function removeWhitespaces(DOMNode $domNode) {
foreach ($domNode->childNodes as $node) {
if($node->hasChildNodes()) {
$this->removeWhitespaces($node);
} else {
if ($node instanceof \DOMText && $node->isWhitespaceInElementContent() ) {
$domNode->removeChild($node);
}
}
}
}
}