From 5035613157cd027cb4cbfebc88f1b05c12f231b0 Mon Sep 17 00:00:00 2001 From: Wouter van Dongen Date: Fri, 14 Mar 2014 15:20:12 +0100 Subject: [PATCH 01/11] * Fixed bug where select query in deleteAll could return wrong table name. PDOStatement::getColumnMeta (in mysql.php) sometimes returns the actual table name when using a MySQL view with the distinct select query, and not the alias table name. By returning the actual table name the records could not be removed. By the way PDOStatement::getColumnMeta is an experimental function, perhaps it's better to avoid it. --- lib/Cake/Model/Model.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/Cake/Model/Model.php b/lib/Cake/Model/Model.php index c990c87eb..209b3e7c9 100644 --- a/lib/Cake/Model/Model.php +++ b/lib/Cake/Model/Model.php @@ -2703,8 +2703,9 @@ class Model extends Object implements CakeEventListener { } $ids = $this->find('all', array_merge(array( - 'fields' => "DISTINCT {$this->alias}.{$this->primaryKey}", + 'fields' => "{$this->alias}.{$this->primaryKey}", 'order' => false, + 'group' => "{$this->alias}.{$this->primaryKey}", 'recursive' => 0), compact('conditions')) ); From 97c148a1706edea3e3363c18c705a0068b31e6fe Mon Sep 17 00:00:00 2001 From: euromark Date: Wed, 19 Mar 2014 12:47:31 +0100 Subject: [PATCH 02/11] Correct typo --- lib/Cake/TestSuite/ControllerTestCase.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Cake/TestSuite/ControllerTestCase.php b/lib/Cake/TestSuite/ControllerTestCase.php index 12701914d..23f03ce79 100644 --- a/lib/Cake/TestSuite/ControllerTestCase.php +++ b/lib/Cake/TestSuite/ControllerTestCase.php @@ -292,7 +292,7 @@ abstract class ControllerTestCase extends CakeTestCase { * ### Mocks: * * - `methods` Methods to mock on the controller. `_stop()` is mocked by default - * - `models` Models to mock. Models are added to the ClassRegistry so they any + * - `models` Models to mock. Models are added to the ClassRegistry so any * time they are instantiated the mock will be created. Pass as key value pairs * with the value being specific methods on the model to mock. If `true` or * no value is passed, the entire model will be mocked. From c6f723059c231c767f29ae3804973ee555461f90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc=20W=C3=BCrth?= Date: Wed, 19 Mar 2014 17:30:10 +0100 Subject: [PATCH 03/11] testcases => test cases --- CONTRIBUTING.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ab748c188..8fdfd430c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -2,8 +2,8 @@ CakePHP loves to welcome your contributions. There are several ways to help out: * Create an [issue](https://github.com/cakephp/cakephp/issues) on GitHub, if you have found a bug -* Write testcases for open bug issues -* Write patches for open bug/feature issues, preferably with testcases included +* Write test cases for open bug issues +* Write patches for open bug/feature issues, preferably with test cases included * Contribute to the [documentation](https://github.com/cakephp/docs) There are a few guidelines that we need contributors to follow so that we have a @@ -30,7 +30,7 @@ chance of keeping on top of things. * Make commits of logical units. * Check for unnecessary whitespace with `git diff --check` before committing. * Use descriptive commit messages and reference the #issue number. -* Core testcases should continue to pass. You can run tests locally or enable +* Core test cases should continue to pass. You can run tests locally or enable [travis-ci](https://travis-ci.org/) for your fork, so all tests and codesniffs will be executed. * Your work should apply the CakePHP coding standards. @@ -48,10 +48,10 @@ chance of keeping on top of things. * Submit a pull request to the repository in the cakephp organization, with the correct target branch. -## Testcases and codesniffer +## Test cases and codesniffer CakePHP tests requires [PHPUnit](http://www.phpunit.de/manual/current/en/installation.html) -3.5 or higher. To run the testcases locally use the following command: +3.5 or higher. To run the test cases locally use the following command: ./lib/Cake/Console/cake test core AllTests --stderr From 9a36ed5785da55463079ce2251537435e24ba03f Mon Sep 17 00:00:00 2001 From: wbkostan Date: Wed, 19 Mar 2014 19:12:44 -0400 Subject: [PATCH 04/11] Update Inflector.php Modified singularizer inside inflector to accurately inflect most words ending in -aves. Removed irregular cases solved by this fix. --- lib/Cake/Utility/Inflector.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/Cake/Utility/Inflector.php b/lib/Cake/Utility/Inflector.php index fb57413af..ef4685edf 100644 --- a/lib/Cake/Utility/Inflector.php +++ b/lib/Cake/Utility/Inflector.php @@ -117,7 +117,7 @@ class Inflector { '/(alumn|bacill|cact|foc|fung|nucle|radi|stimul|syllab|termin|viri?)i$/i' => '\1us', '/([ftw]ax)es/i' => '\1', '/(cris|ax|test)es$/i' => '\1is', - '/(shoe|slave)s$/i' => '\1', + '/(shoe)s$/i' => '\1', '/(o)es$/i' => '\1', '/ouses$/' => 'ouse', '/([^a])uses$/' => '\1us', @@ -130,7 +130,7 @@ class Inflector { '/(hive)s$/i' => '\1', '/(drive)s$/i' => '\1', '/([le])ves$/i' => '\1f', - '/([^rfo])ves$/i' => '\1fe', + '/([^rfoa])ves$/i' => '\1fe', '/(^analy)ses$/i' => '\1sis', '/(analy|diagno|^ba|(p)arenthe|(p)rogno|(s)ynop|(t)he)ses$/i' => '\1\2sis', '/([ti])a$/i' => '\1um', @@ -147,7 +147,6 @@ class Inflector { ), 'irregular' => array( 'foes' => 'foe', - 'waves' => 'wave', ) ); From fea60bfe5123a08e8959a225af6939c277cf369a Mon Sep 17 00:00:00 2001 From: wbkostan Date: Wed, 19 Mar 2014 19:17:52 -0400 Subject: [PATCH 05/11] Update InflectorTest.php Added test cases for changes to inflector which affected words ending -aves. Author acknowledges the homonym conflict with 'leaves' and 'leaves', but preferences the word whose singular avoids an exception to the inflection rule. --- lib/Cake/Test/Case/Utility/InflectorTest.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/Cake/Test/Case/Utility/InflectorTest.php b/lib/Cake/Test/Case/Utility/InflectorTest.php index 366a62635..1d269348b 100644 --- a/lib/Cake/Test/Case/Utility/InflectorTest.php +++ b/lib/Cake/Test/Case/Utility/InflectorTest.php @@ -92,6 +92,8 @@ class InflectorTest extends CakeTestCase { $this->assertEquals(Inflector::singularize('faxes'), 'fax'); $this->assertEquals(Inflector::singularize('waxes'), 'wax'); $this->assertEquals(Inflector::singularize('niches'), 'niche'); + $this->assertEquals(Inflector::singularize('caves'), 'cave'); + $this->assertEquals(Inflector::singularize('graves'), 'grave'); $this->assertEquals(Inflector::singularize('waves'), 'wave'); $this->assertEquals(Inflector::singularize('bureaus'), 'bureau'); $this->assertEquals(Inflector::singularize('genetic_analyses'), 'genetic_analysis'); From 2eadb89ff6fdc29754a8a0b7686852a245d66a8d Mon Sep 17 00:00:00 2001 From: James Watts Date: Fri, 21 Mar 2014 16:16:32 +0100 Subject: [PATCH 06/11] Update in_array() check to avoid fatal error Enabling the $strict parameter to true avoids PHP's default behavior when search for an array in an array, which throws a fatal error if circular references exist - http://php.net/in_array#refsect1-function.in-array-parameters --- lib/Cake/Model/Datasource/DboSource.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Cake/Model/Datasource/DboSource.php b/lib/Cake/Model/Datasource/DboSource.php index 68733daeb..cab2ba055 100644 --- a/lib/Cake/Model/Datasource/DboSource.php +++ b/lib/Cake/Model/Datasource/DboSource.php @@ -1561,7 +1561,7 @@ class DboSource extends DataSource { if (!empty($assocData['order'])) { $queryData['order'][] = $assocData['order']; } - if (!in_array($join, $queryData['joins'])) { + if (!in_array($join, $queryData['joins'], true)) { $queryData['joins'][] = $join; } return true; From a827b96804f3a0bf840b4ca54e63ebc56879c0a5 Mon Sep 17 00:00:00 2001 From: mark_story Date: Thu, 20 Mar 2014 23:17:46 -0400 Subject: [PATCH 07/11] Clean up doc blocks for assertTags. --- lib/Cake/TestSuite/CakeTestCase.php | 35 ++++++++++++++++++----------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/lib/Cake/TestSuite/CakeTestCase.php b/lib/Cake/TestSuite/CakeTestCase.php index 16b4ee57b..81f14c54f 100644 --- a/lib/Cake/TestSuite/CakeTestCase.php +++ b/lib/Cake/TestSuite/CakeTestCase.php @@ -340,24 +340,33 @@ abstract class CakeTestCase extends PHPUnit_Framework_TestCase { * * Checks for an input tag with a name attribute (contains any non-empty value) and an id * attribute that contains 'my-input': - * array('input' => array('name', 'id' => 'my-input')) + * + * {{{ + * array('input' => array('name', 'id' => 'my-input')) + * }}} * * Checks for two p elements with some text in them: - * array( - * array('p' => true), - * 'textA', - * '/p', - * array('p' => true), - * 'textB', - * '/p' - * ) + * + * {{{ + * array( + * array('p' => true), + * 'textA', + * '/p', + * array('p' => true), + * 'textB', + * '/p' + * ) + * }}} * * You can also specify a pattern expression as part of the attribute values, or the tag * being defined, if you prepend the value with preg: and enclose it with slashes, like so: - * array( - * array('input' => array('name', 'id' => 'preg:/FieldName\d+/')), - * 'preg:/My\s+field/' - * ) + * + * {{{ + * array( + * array('input' => array('name', 'id' => 'preg:/FieldName\d+/')), + * 'preg:/My\s+field/' + * ) + * }}} * * Important: This function is very forgiving about whitespace and also accepts any * permutation of attribute order. It will also allow whitespace between specified tags. From c1b2b560bba2ad83752053d3160ce577b6f9ef89 Mon Sep 17 00:00:00 2001 From: mark_story Date: Thu, 20 Mar 2014 23:18:21 -0400 Subject: [PATCH 08/11] Fix typo. --- lib/Cake/Test/Fixture/AssertTagsTestCase.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/Cake/Test/Fixture/AssertTagsTestCase.php b/lib/Cake/Test/Fixture/AssertTagsTestCase.php index 9391dd426..f0baeab3c 100644 --- a/lib/Cake/Test/Fixture/AssertTagsTestCase.php +++ b/lib/Cake/Test/Fixture/AssertTagsTestCase.php @@ -1,6 +1,6 @@ assertTags($input, $pattern); } + } From af68f61e7a0cc4c2d558daee12212db51beae4f9 Mon Sep 17 00:00:00 2001 From: mark_story Date: Fri, 21 Mar 2014 21:24:10 -0400 Subject: [PATCH 09/11] Make assertTags() run much faster. 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 --- .../Test/Case/TestSuite/CakeTestCaseTest.php | 48 +++++++++++-- lib/Cake/TestSuite/CakeTestCase.php | 70 ++++++++++--------- 2 files changed, 77 insertions(+), 41 deletions(-) diff --git a/lib/Cake/Test/Case/TestSuite/CakeTestCaseTest.php b/lib/Cake/Test/Case/TestSuite/CakeTestCaseTest.php index 33acd0efe..2d884b246 100644 --- a/lib/Cake/Test/Case/TestSuite/CakeTestCaseTest.php +++ b/lib/Cake/Test/Case/TestSuite/CakeTestCaseTest.php @@ -62,24 +62,31 @@ class CakeTestCaseTest extends CakeTestCase { } /** - * 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 = 'My link'; $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 = "My link"; $pattern = array( @@ -87,7 +94,7 @@ class CakeTestCaseTest extends CakeTestCase { 'My link', '/a' ); - $this->assertTrue($test->assertTags($input, $pattern), 'Single quoted attributes %s'); + $this->assertTags($input, $pattern); $input = "My link"; $pattern = array( @@ -95,7 +102,7 @@ class CakeTestCaseTest extends CakeTestCase { 'My link', '/a' ); - $this->assertTrue($test->assertTags($input, $pattern), 'Single quoted attributes %s'); + $this->assertTags($input, $pattern); $input = "Text"; $pattern = array( @@ -105,7 +112,7 @@ class CakeTestCaseTest extends CakeTestCase { '/strong', '/span' ); - $this->assertTrue($test->assertTags($input, $pattern), 'Tags with no attributes'); + $this->assertTags($input, $pattern); $input = "Text"; $pattern = array( @@ -115,7 +122,34 @@ class CakeTestCaseTest extends CakeTestCase { '/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 = '
' . + 'My div' . + '
'; + $this->assertTags($input, $pattern); } /** diff --git a/lib/Cake/TestSuite/CakeTestCase.php b/lib/Cake/TestSuite/CakeTestCase.php index 81f14c54f..fe3531bb2 100644 --- a/lib/Cake/TestSuite/CakeTestCase.php +++ b/lib/Cake/TestSuite/CakeTestCase.php @@ -448,8 +448,12 @@ abstract class CakeTestCase extends PHPUnit_Framework_TestCase { $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); @@ -460,16 +464,9 @@ abstract class CakeTestCase extends PHPUnit_Framework_TestCase { $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( @@ -479,9 +476,14 @@ abstract class CakeTestCase extends PHPUnit_Framework_TestCase { ); } } - 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; @@ -504,31 +506,31 @@ abstract class CakeTestCase extends PHPUnit_Framework_TestCase { } /** - * 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) { + $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 From f12b272758870283e42960987307275aab74b2dd Mon Sep 17 00:00:00 2001 From: mark_story Date: Fri, 21 Mar 2014 21:57:06 -0400 Subject: [PATCH 10/11] Fix a few flaky/bad attribute matchers. --- lib/Cake/Test/Case/View/Helper/FormHelperTest.php | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/lib/Cake/Test/Case/View/Helper/FormHelperTest.php b/lib/Cake/Test/Case/View/Helper/FormHelperTest.php index 4a7ba4946..73bb50f5a 100644 --- a/lib/Cake/Test/Case/View/Helper/FormHelperTest.php +++ b/lib/Cake/Test/Case/View/Helper/FormHelperTest.php @@ -862,6 +862,7 @@ class FormHelperTest extends CakeTestCase { '/div', ); $this->assertTags($result, $expected); + $result = $this->Form->submit('Cancel', array('name' => 'cancel')); $expected = array( 'div' => array('class' => 'submit'), @@ -1844,8 +1845,8 @@ class FormHelperTest extends CakeTestCase { 'preg:/[^<]+/', '/label', 'input' => array( - 'type' => 'text', 'name' => 'preg:/[^<]+/', - 'id' => 'preg:/[^<]+/', 'class' => 'form-error' + 'type' => 'text', 'name', 'id', + 'class' => 'form-error' ), array('div' => array('class' => 'error-message')), 'You must have a last name', @@ -1869,7 +1870,7 @@ class FormHelperTest extends CakeTestCase { 'label' => array('for'), 'Balance', '/label', - 'input' => array('name', 'type' => 'number', 'id'), + 'input' => array('name', 'type' => 'number', 'id', 'step'), '/div', ); $this->assertTags($result, $expected); @@ -5698,14 +5699,14 @@ class FormHelperTest extends CakeTestCase { $result = $this->Form->checkbox('Contact.field', array('id' => 'theID', 'value' => 'myvalue')); $expected = array( 'input' => array('type' => 'hidden', 'class' => 'form-error', 'name' => 'data[Contact][field]', 'value' => '0', 'id' => 'theID_'), - array('input' => array('preg:/[^<]+/', 'value' => 'myvalue', 'id' => 'theID', 'checked' => 'checked', 'class' => 'form-error')) + array('input' => array('type', 'name', 'value' => 'myvalue', 'id' => 'theID', 'checked' => 'checked', 'class' => 'form-error')) ); $this->assertTags($result, $expected); $result = $this->Form->checkbox('Contact.field', array('value' => 'myvalue')); $expected = array( 'input' => array('type' => 'hidden', 'class' => 'form-error', 'name' => 'data[Contact][field]', 'value' => '0', 'id' => 'ContactField_'), - array('input' => array('preg:/[^<]+/', 'value' => 'myvalue', 'id' => 'ContactField', 'checked' => 'checked', 'class' => 'form-error')) + array('input' => array('type', 'name', 'value' => 'myvalue', 'id' => 'ContactField', 'checked' => 'checked', 'class' => 'form-error')) ); $this->assertTags($result, $expected); From a801be30d9ed2af8ebe5818afb6eaf1c2d2a01b8 Mon Sep 17 00:00:00 2001 From: mark_story Date: Fri, 21 Mar 2014 22:53:25 -0400 Subject: [PATCH 11/11] Use alternate quoting in assertTags() When a preg pattern contains no `.` it probably doesn't need quote anchors. --- lib/Cake/TestSuite/CakeTestCase.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/Cake/TestSuite/CakeTestCase.php b/lib/Cake/TestSuite/CakeTestCase.php index fe3531bb2..4d8fd4050 100644 --- a/lib/Cake/TestSuite/CakeTestCase.php +++ b/lib/Cake/TestSuite/CakeTestCase.php @@ -448,12 +448,13 @@ abstract class CakeTestCase extends PHPUnit_Framework_TestCase { $val = '.+?'; $explanations[] = sprintf('Attribute "%s" present', $attr); } elseif (!empty($val) && preg_match('/^preg\:\/(.+)\/$/i', $val, $matches)) { - $quotes = '["\']'; $val = str_replace( array('.*', '.+'), array('.*?', '.+?'), $matches[1] ); + $quotes = $val !== $matches[1] ? '["\']' : '["\']?'; + $explanations[] = sprintf('Attribute "%s" matches "%s"', $attr, $val); } else { $explanations[] = sprintf('Attribute "%s" == "%s"', $attr, $val);