Skip to content

Commit

Permalink
Make assertTags() run much faster.
Browse files Browse the repository at this point in the history
Generating the various permutations a priori is incredibly expensive
with sets of attributes. Using nested loops that look for matches is
more efficient.

Add replacments for `.*` and `.+` in preg:/ prefixed attribute matchers
so they do not greedily eat all content. This also requires that preg:/
based attribute matchers *must* be quoted.

Fixes #3072
  • Loading branch information
markstory committed Mar 22, 2014
1 parent c1b2b56 commit af68f61
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 41 deletions.
48 changes: 41 additions & 7 deletions lib/Cake/Test/Case/TestSuite/CakeTestCaseTest.php
Expand Up @@ -62,40 +62,47 @@ public function tearDown() {
}

/**
* testAssertGoodTags
* testAssertTags
*
* @return void
*/
public function testAssertTagsQuotes() {
public function testAssertTagsBasic() {
$test = new AssertTagsTestCase('testAssertTagsQuotes');
$result = $test->run();
$this->assertEquals(0, $result->errorCount());
$this->assertTrue($result->wasSuccessful());
$this->assertEquals(0, $result->failureCount());
}

/**
* test assertTags works with single and double quotes
*
* @return void
*/
public function testAssertTagsQuoting() {
$input = '<a href="/test.html" class="active">My link</a>';
$pattern = array(
'a' => array('href' => '/test.html', 'class' => 'active'),
'My link',
'/a'
);
$this->assertTrue($test->assertTags($input, $pattern), 'Double quoted attributes %s');
$this->assertTags($input, $pattern);

$input = "<a href='/test.html' class='active'>My link</a>";
$pattern = array(
'a' => array('href' => '/test.html', 'class' => 'active'),
'My link',
'/a'
);
$this->assertTrue($test->assertTags($input, $pattern), 'Single quoted attributes %s');
$this->assertTags($input, $pattern);

$input = "<a href='/test.html' class='active'>My link</a>";
$pattern = array(
'a' => array('href' => 'preg:/.*\.html/', 'class' => 'active'),
'My link',
'/a'
);
$this->assertTrue($test->assertTags($input, $pattern), 'Single quoted attributes %s');
$this->assertTags($input, $pattern);

$input = "<span><strong>Text</strong></span>";
$pattern = array(
Expand All @@ -105,7 +112,7 @@ public function testAssertTagsQuotes() {
'/strong',
'/span'
);
$this->assertTrue($test->assertTags($input, $pattern), 'Tags with no attributes');
$this->assertTags($input, $pattern);

$input = "<span class='active'><strong>Text</strong></span>";
$pattern = array(
Expand All @@ -115,7 +122,34 @@ public function testAssertTagsQuotes() {
'/strong',
'/span'
);
$this->assertTrue($test->assertTags($input, $pattern), 'Test attribute presence');
$this->assertTags($input, $pattern);
}

/**
* Test that assertTags runs quickly.
*
* @return void
*/
public function testAssertTagsRuntimeComplexity() {
$pattern = array(
'div' => array(
'attr1' => 'val1',
'attr2' => 'val2',
'attr3' => 'val3',
'attr4' => 'val4',
'attr5' => 'val5',
'attr6' => 'val6',
'attr7' => 'val7',
'attr8' => 'val8',
),
'My div',
'/div'
);
$input = '<div attr8="val8" attr6="val6" attr4="val4" attr2="val2"' .
' attr1="val1" attr3="val3" attr5="val5" attr7="val7" />' .
'My div' .
'</div>';
$this->assertTags($input, $pattern);
}

/**
Expand Down
70 changes: 36 additions & 34 deletions lib/Cake/TestSuite/CakeTestCase.php
Expand Up @@ -448,8 +448,12 @@ public function assertTags($string, $expected, $fullDebug = false) {
$val = '.+?';
$explanations[] = sprintf('Attribute "%s" present', $attr);
} elseif (!empty($val) && preg_match('/^preg\:\/(.+)\/$/i', $val, $matches)) {
$quotes = '["\']?';
$val = $matches[1];
$quotes = '["\']';
$val = str_replace(
array('.*', '.+'),
array('.*?', '.+?'),
$matches[1]
);
$explanations[] = sprintf('Attribute "%s" matches "%s"', $attr, $val);
} else {
$explanations[] = sprintf('Attribute "%s" == "%s"', $attr, $val);
Expand All @@ -460,16 +464,9 @@ public function assertTags($string, $expected, $fullDebug = false) {
$i++;
}
if ($attrs) {
$permutations = $this->_arrayPermute($attrs);

$permutationTokens = array();
foreach ($permutations as $permutation) {
$permutationTokens[] = implode('', $permutation);
}
$regex[] = array(
sprintf('%s', implode(', ', $explanations)),
$permutationTokens,
$i,
'explains' => $explanations,
'attrs' => $attrs,
);
}
$regex[] = array(
Expand All @@ -479,9 +476,14 @@ public function assertTags($string, $expected, $fullDebug = false) {
);
}
}
foreach ($regex as $i => $assertation) {
list($description, $expressions, $itemNum) = $assertation;
foreach ($regex as $i => $assertion) {
$matches = false;
if (isset($assertion['attrs'])) {
$string = $this->_assertAttributes($assertion, $string);
continue;
}

list($description, $expressions, $itemNum) = $assertion;
foreach ((array)$expressions as $expression) {
if (preg_match(sprintf('/^%s/s', $expression), $string, $match)) {
$matches = true;
Expand All @@ -504,31 +506,31 @@ public function assertTags($string, $expected, $fullDebug = false) {
}

/**
* Generates all permutation of an array $items and returns them in a new array.
* Check the attributes as part of an assertTags() check.
*
* @param array $items An array of items
* @param array $perms
* @return array
* @param array $assertions Assertions to run.
* @param string $string The HTML string to check.
* @return void
*/
protected function _arrayPermute($items, $perms = array()) {
static $permuted;
if (empty($perms)) {
$permuted = array();
}

if (empty($items)) {
$permuted[] = $perms;
} else {
$numItems = count($items) - 1;
for ($i = $numItems; $i >= 0; --$i) {
$newItems = $items;
$newPerms = $perms;
list($tmp) = array_splice($newItems, $i, 1);
array_unshift($newPerms, $tmp);
$this->_arrayPermute($newItems, $newPerms);
protected function _assertAttributes($assertions, $string) {
$asserts = $assertions['attrs'];
$explains = $assertions['explains'];
while (count($asserts) > 0) {

This comment has been minimized.

Copy link
@ADmad

ADmad Mar 23, 2014

Member

Code sniffer doesn't like this.

This comment has been minimized.

Copy link
@markstory

markstory Mar 23, 2014

Author Member

Ok, I can write it a different way.

$matches = false;
foreach ($asserts as $j => $assert) {
if (preg_match(sprintf('/^%s/s', $assert), $string, $match)) {
$matches = true;
$string = substr($string, strlen($match[0]));
array_splice($asserts, $j, 1);
array_splice($explains, $j, 1);
break;
}
}
if ($matches === false) {
$this->assertTrue(false, 'Attribute did not match. Was expecting ' . $explains[$j]);
}
return $permuted;
}
return $string;
}

// @codingStandardsIgnoreStart
Expand Down

0 comments on commit af68f61

Please sign in to comment.