From a3cd7ddc0978685687d10b01085767bafc163bb5 Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Fri, 23 Nov 2012 11:12:03 +0100 Subject: [PATCH 1/3] BUG Force SapphireTest schema reset for extension changes Previously changes to $requiredExtensions and $illegalExtensions didn't cause a reset unless there was also other schema-altering settings like $extensionsToRemove or $extraDataObjects --- dev/SapphireTest.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dev/SapphireTest.php b/dev/SapphireTest.php index 58fd71133..69ea3539b 100644 --- a/dev/SapphireTest.php +++ b/dev/SapphireTest.php @@ -272,6 +272,8 @@ class SapphireTest extends PHPUnit_Framework_TestCase { * for tearing down the state again. */ public function setUpOnce() { + $isAltered = false; + // Remove any illegal extensions that are present foreach($this->illegalExtensions as $class => $extensions) { foreach($extensions as $extension) { @@ -298,7 +300,7 @@ class SapphireTest extends PHPUnit_Framework_TestCase { } // If we have made changes to the extensions present, then migrate the database schema. - if($this->extensionsToReapply || $this->extensionsToRemove || $this->extraDataObjects) { + if($isAltered || $this->extensionsToReapply || $this->extensionsToRemove || $this->extraDataObjects) { if(!self::using_temp_db()) self::create_temp_db(); $this->resetDBSchema(true); } From 453d04e4ba05bec467a2dcea139e75edb1abcab5 Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Fri, 23 Nov 2012 11:14:02 +0100 Subject: [PATCH 2/3] BUG Reset DataObject caches in SapphireTest->resetDBSchema() This became a problem with fdcd7a2e where $custom_database_fields were cached, but never reset. It lead to extensions not applying correctly in SapphireTest->setUpOnce(). --- dev/SapphireTest.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dev/SapphireTest.php b/dev/SapphireTest.php index 69ea3539b..4ca8a370e 100644 --- a/dev/SapphireTest.php +++ b/dev/SapphireTest.php @@ -801,6 +801,8 @@ class SapphireTest extends PHPUnit_Framework_TestCase { */ public function resetDBSchema($includeExtraDataObjects = false) { if(self::using_temp_db()) { + DataObject::reset(); + // clear singletons, they're caching old extension info which is used in DatabaseAdmin->doBuild() Injector::inst()->unregisterAllObjects(); From aa72425e846e2bd1735e7041f6944aacd2187212 Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Fri, 23 Nov 2012 14:55:19 +0100 Subject: [PATCH 3/3] Fixed PHPUnit assertions for incomplete tests in core Avoid PHPUnit throwing "test didn't run any assertions" notices in PHP. If nothing else, it keeps test output looking less broken by default, making it more likely that actual errors do get noticed. --- dev/FunctionalTest.php | 56 ++++++------- dev/SapphireTest.php | 78 +++++++++--------- model/DataObject.php | 2 +- tests/control/RequestHandlingTest.php | 2 +- tests/core/ConfigTest.php | 2 +- tests/core/ObjectTest.php | 1 + tests/dev/DeprecationTest.php | 8 +- tests/filesystem/FileTest.php | 7 +- tests/forms/EmailFieldTest.php | 5 +- tests/forms/GridFieldTest.php | 16 ++-- tests/forms/RequirementsTest.php | 89 ++++++++++++--------- tests/model/AggregateTest.php | 2 +- tests/model/DataListTest.php | 4 +- tests/model/DataObjectTest.php | 17 ++-- tests/model/DatabaseTest.php | 19 +++-- tests/security/PasswordValidatorTest.php | 2 +- tests/security/SecurityDefaultAdminTest.php | 9 ++- tests/view/SSViewerCacheBlockTest.php | 6 +- 18 files changed, 163 insertions(+), 162 deletions(-) diff --git a/dev/FunctionalTest.php b/dev/FunctionalTest.php index a9058eba3..c95f4919b 100644 --- a/dev/FunctionalTest.php +++ b/dev/FunctionalTest.php @@ -191,14 +191,13 @@ class FunctionalTest extends SapphireTest { if($items) foreach($items as $item) $actuals[trim(preg_replace("/[ \n\r\t]+/", " ", $item. ''))] = true; foreach($expectedMatches as $match) { - if(!isset($actuals[$match])) { - throw new PHPUnit_Framework_AssertionFailedError( - "Failed asserting the CSS selector '$selector' has a partial match to the expected elements:\n'" - . implode("'\n'", $expectedMatches) . "'\n\n" - . "Instead the following elements were found:\n'" . implode("'\n'", array_keys($actuals)) . "'" - ); - return false; - } + $this->assertTrue( + isset($actuals[$match]), + "Failed asserting the CSS selector '$selector' has a partial match to the expected elements:\n'" + . implode("'\n'", $expectedMatches) . "'\n\n" + . "Instead the following elements were found:\n'" . implode("'\n'", array_keys($actuals)) . "'" + ); + return false; } return true; @@ -224,14 +223,12 @@ class FunctionalTest extends SapphireTest { $actuals = array(); if($items) foreach($items as $item) $actuals[] = trim(preg_replace("/[ \n\r\t]+/", " ", $item. '')); - if($expectedMatches != $actuals) { - throw new PHPUnit_Framework_AssertionFailedError( - "Failed asserting the CSS selector '$selector' has an exact match to the expected elements:\n'" - . implode("'\n'", $expectedMatches) . "'\n\n" - . "Instead the following elements were found:\n'" . implode("'\n'", $actuals) . "'" - ); - return false; - } + $this->assertTrue( + $expectedMatches == $actuals, + "Failed asserting the CSS selector '$selector' has an exact match to the expected elements:\n'" + . implode("'\n'", $expectedMatches) . "'\n\n" + . "Instead the following elements were found:\n'" . implode("'\n'", $actuals) . "'" + ); return true; } @@ -257,14 +254,12 @@ class FunctionalTest extends SapphireTest { if($items) foreach($items as $item) $actuals[$item->asXML()] = true; foreach($expectedMatches as $match) { - if(!isset($actuals[$match])) { - throw new PHPUnit_Framework_AssertionFailedError( - "Failed asserting the CSS selector '$selector' has a partial match to the expected elements:\n'" - . implode("'\n'", $expectedMatches) . "'\n\n" - . "Instead the following elements were found:\n'" . implode("'\n'", array_keys($actuals)) . "'" - ); - return false; - } + $this->assertTrue( + isset($actuals[$match]), + "Failed asserting the CSS selector '$selector' has a partial match to the expected elements:\n'" + . implode("'\n'", $expectedMatches) . "'\n\n" + . "Instead the following elements were found:\n'" . implode("'\n'", array_keys($actuals)) . "'" + ); } return true; @@ -288,13 +283,12 @@ class FunctionalTest extends SapphireTest { $actuals = array(); if($items) foreach($items as $item) $actuals[] = $item->asXML(); - if($expectedMatches != $actuals) { - throw new PHPUnit_Framework_AssertionFailedError( - "Failed asserting the CSS selector '$selector' has an exact match to the expected elements:\n'" - . implode("'\n'", $expectedMatches) . "'\n\n" - . "Instead the following elements were found:\n'" . implode("'\n'", $actuals) . "'" - ); - } + $this->assertTrue( + $expectedMatches == $actuals, + "Failed asserting the CSS selector '$selector' has an exact match to the expected elements:\n'" + . implode("'\n'", $expectedMatches) . "'\n\n" + . "Instead the following elements were found:\n'" . implode("'\n'", $actuals) . "'" + ); } /** diff --git a/dev/SapphireTest.php b/dev/SapphireTest.php index 4ca8a370e..531508ff7 100644 --- a/dev/SapphireTest.php +++ b/dev/SapphireTest.php @@ -542,21 +542,20 @@ class SapphireTest extends PHPUnit_Framework_TestCase { * 'customHeaders', 'htmlContent', inlineImages' */ public function assertEmailSent($to, $from = null, $subject = null, $content = null) { - // To do - this needs to be turned into a "real" PHPUnit ass - if(!$this->findEmail($to, $from, $subject, $content)) { - - $infoParts = ""; - $withParts = array(); - if($to) $infoParts .= " to '$to'"; - if($from) $infoParts .= " from '$from'"; - if($subject) $withParts[] = "subject '$subject'"; - if($content) $withParts[] = "content '$content'"; - if($withParts) $infoParts .= " with " . implode(" and ", $withParts); - - throw new PHPUnit_Framework_AssertionFailedError( - "Failed asserting that an email was sent$infoParts." - ); - } + $found = (bool)$this->findEmail($to, $from, $subject, $content); + + $infoParts = ""; + $withParts = array(); + if($to) $infoParts .= " to '$to'"; + if($from) $infoParts .= " from '$from'"; + if($subject) $withParts[] = "subject '$subject'"; + if($content) $withParts[] = "content '$content'"; + if($withParts) $infoParts .= " with " . implode(" and ", $withParts); + + $this->assertTrue( + $found, + "Failed asserting that an email was sent$infoParts." + ); } @@ -596,14 +595,12 @@ class SapphireTest extends PHPUnit_Framework_TestCase { } // We couldn't find a match - assertion failed - if(!$matched) { - throw new PHPUnit_Framework_AssertionFailedError( - "Failed asserting that the SS_List contains an item matching " - . var_export($match, true) . "\n\nIn the following SS_List:\n" - . $this->DOSSummaryForMatch($dataObjectSet, $match) - ); - } - + $this->assertTrue( + $matched, + "Failed asserting that the SS_List contains an item matching " + . var_export($match, true) . "\n\nIn the following SS_List:\n" + . $this->DOSSummaryForMatch($dataObjectSet, $match) + ); } } @@ -642,23 +639,21 @@ class SapphireTest extends PHPUnit_Framework_TestCase { } // We couldn't find a match - assertion failed - if(!$matched) { - throw new PHPUnit_Framework_AssertionFailedError( - "Failed asserting that the SS_List contains an item matching " - . var_export($match, true) . "\n\nIn the following SS_List:\n" - . $this->DOSSummaryForMatch($dataObjectSet, $match) - ); - } + $this->assertTrue( + $matched, + "Failed asserting that the SS_List contains an item matching " + . var_export($match, true) . "\n\nIn the following SS_List:\n" + . $this->DOSSummaryForMatch($dataObjectSet, $match) + ); } // If we have leftovers than the DOS has extra data that shouldn't be there - if($extracted) { + $this->assertTrue( + (count($extracted) == 0), // If we didn't break by this point then we couldn't find a match - throw new PHPUnit_Framework_AssertionFailedError( - "Failed asserting that the SS_List contained only the given items, the " - . "following items were left over:\n" . var_export($extracted, true) - ); - } + "Failed asserting that the SS_List contained only the given items, the " + . "following items were left over:\n" . var_export($extracted, true) + ); } /** @@ -678,12 +673,11 @@ class SapphireTest extends PHPUnit_Framework_TestCase { foreach($dataObjectSet as $item) $extracted[] = $item->toMap(); foreach($extracted as $i => $item) { - if(!$this->dataObjectArrayMatch($item, $match)) { - throw new PHPUnit_Framework_AssertionFailedError( - "Failed asserting that the the following item matched " - . var_export($match, true) . ": " . var_export($item, true) - ); - } + $this->assertTrue( + $this->dataObjectArrayMatch($item, $match), + "Failed asserting that the the following item matched " + . var_export($match, true) . ": " . var_export($item, true) + ); } } diff --git a/model/DataObject.php b/model/DataObject.php index e3418aba8..b4c73f639 100644 --- a/model/DataObject.php +++ b/model/DataObject.php @@ -1239,7 +1239,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity } // Deleting a record without an ID shouldn't do anything - if(!$this->ID) throw new Exception("DataObject::delete() called on a DataObject without an ID"); + if(!$this->ID) throw new LogicException("DataObject::delete() called on a DataObject without an ID"); // TODO: This is quite ugly. To improve: // - move the details of the delete code in the DataQuery system diff --git a/tests/control/RequestHandlingTest.php b/tests/control/RequestHandlingTest.php index 5214c1614..40fa57d22 100644 --- a/tests/control/RequestHandlingTest.php +++ b/tests/control/RequestHandlingTest.php @@ -26,7 +26,7 @@ class RequestHandlingTest extends FunctionalTest { } public function testRequestHandlerChainingAllParams() { - // TODO + $this->markTestIncomplete(); } public function testMethodCallingOnController() { diff --git a/tests/core/ConfigTest.php b/tests/core/ConfigTest.php index 00e6b6274..2598bd404 100644 --- a/tests/core/ConfigTest.php +++ b/tests/core/ConfigTest.php @@ -161,7 +161,7 @@ class ConfigTest extends SapphireTest { } public function testFragmentOrder() { - // $manifest = new SS_ConfigManifest(BASE_PATH, false, true); + $this->markTestIncomplete(); } } diff --git a/tests/core/ObjectTest.php b/tests/core/ObjectTest.php index ef81faec9..d62f45bc4 100644 --- a/tests/core/ObjectTest.php +++ b/tests/core/ObjectTest.php @@ -296,6 +296,7 @@ class ObjectTest extends SapphireTest { } public function testCacheToFile() { + $this->markTestIncomplete(); /* // This doesn't run properly on our build slave. $obj = new ObjectTest_CacheTest(); diff --git a/tests/dev/DeprecationTest.php b/tests/dev/DeprecationTest.php index 3282177d9..5a57f36a7 100644 --- a/tests/dev/DeprecationTest.php +++ b/tests/dev/DeprecationTest.php @@ -25,7 +25,7 @@ class DeprecationTest extends SapphireTest { public function testLesserVersionTriggersNoNotice() { Deprecation::notification_version('1.0.0'); - Deprecation::notice('2.0', 'Deprecation test failed'); + $this->assertNull(Deprecation::notice('2.0', 'Deprecation test failed')); } /** @@ -38,8 +38,8 @@ class DeprecationTest extends SapphireTest { public function testBetaVersionDoesntTriggerNoticeWhenDeprecationDoesntSpecifyReleasenum() { Deprecation::notification_version('2.0.0-beta1'); - Deprecation::notice('2.0', 'Deprecation test failed'); - Deprecation::notice('2.0.0', 'Deprecation test failed'); + $this->assertNull(Deprecation::notice('2.0', 'Deprecation test failed')); + $this->assertNull(Deprecation::notice('2.0.0', 'Deprecation test failed')); } /** @@ -99,7 +99,7 @@ class DeprecationTest extends SapphireTest { protected function callThatOriginatesFromFramework() { $this->assertEquals(DeprecationTest_Deprecation::get_module(), FRAMEWORK_DIR); - Deprecation::notice('2.0', 'Deprecation test passed'); + $this->assertNull(Deprecation::notice('2.0', 'Deprecation test passed')); } } diff --git a/tests/filesystem/FileTest.php b/tests/filesystem/FileTest.php index f7a5a583c..64c4d8cb9 100644 --- a/tests/filesystem/FileTest.php +++ b/tests/filesystem/FileTest.php @@ -164,6 +164,8 @@ class FileTest extends SapphireTest { /** * @see http://open.silverstripe.org/ticket/5693 + * + * @expectedException ValidationException */ public function testSetNameWithInvalidExtensionDoesntChangeFilesystem() { $origExts = File::$allowed_extensions; @@ -177,11 +179,8 @@ class FileTest extends SapphireTest { $file->write(); } catch(ValidationException $e) { File::$allowed_extensions = $origExts; - return; + throw $e; } - - $this->fail('Expected ValidationException not raised'); - File::$allowed_extensions = $origExts; } public function testLinkAndRelativeLink() { diff --git a/tests/forms/EmailFieldTest.php b/tests/forms/EmailFieldTest.php index 3927541a5..ea909f959 100644 --- a/tests/forms/EmailFieldTest.php +++ b/tests/forms/EmailFieldTest.php @@ -30,9 +30,8 @@ class EmailFieldTest extends FunctionalTest { $val = new EmailFieldTest_Validator(); try { $field->validate($val); - if (!$expectSuccess) { - $this->assertTrue(false,$checkText . " (/$email/ passed validation, but not expected to)"); - } + // If we expect failure and processing gets here without an exception, the test failed + $this->assertTrue($expectSuccess,$checkText . " (/$email/ passed validation, but not expected to)"); } catch (Exception $e) { if ($e instanceof PHPUnit_Framework_AssertionFailedError) throw $e; // re-throw assertion failure else if ($expectSuccess) { diff --git a/tests/forms/GridFieldTest.php b/tests/forms/GridFieldTest.php index 191fac94f..4d0676afc 100644 --- a/tests/forms/GridFieldTest.php +++ b/tests/forms/GridFieldTest.php @@ -273,13 +273,15 @@ class GridFieldTest extends SapphireTest { * @covers GridField::gridFieldAlterAction */ public function testGridFieldAlterAction() { - $config = GridFieldConfig::create()->addComponent(new GridFieldTest_Component); - $obj = new GridField('testfield', 'testfield', ArrayList::create(), $config); - $id = 'testGridStateActionField'; - Session::set($id, array('grid'=>'', 'actionName'=>'jump')); - $form = new Form(new Controller(), 'mockform', new FieldList(array($obj)), new FieldList()); - $request = new SS_HTTPRequest('POST', 'url'); - $obj->gridFieldAlterAction(array('StateID'=>$id), $form, $request); + $this->markTestIncomplete(); + + // $config = GridFieldConfig::create()->addComponent(new GridFieldTest_Component); + // $obj = new GridField('testfield', 'testfield', ArrayList::create(), $config); + // $id = 'testGridStateActionField'; + // Session::set($id, array('grid'=>'', 'actionName'=>'jump')); + // $form = new Form(new Controller(), 'mockform', new FieldList(array($obj)), new FieldList()); + // $request = new SS_HTTPRequest('POST', 'url'); + // $obj->gridFieldAlterAction(array('StateID'=>$id), $form, $request); } /** diff --git a/tests/forms/RequirementsTest.php b/tests/forms/RequirementsTest.php index 12348b990..597bbff23 100644 --- a/tests/forms/RequirementsTest.php +++ b/tests/forms/RequirementsTest.php @@ -11,7 +11,7 @@ class RequirementsTest extends SapphireTest { static $html_template = ''; static $old_requirements = null; - + public function testExternalUrls() { $backend = new Requirements_Backend; $backend->set_combined_files_enabled(true); @@ -281,29 +281,29 @@ class RequirementsTest extends SapphireTest { // to something else $basePath = 'framework' . substr($basePath, strlen(FRAMEWORK_DIR)); - $backend = new RequirementsTest_Backend(); + $backend = new Requirements_Backend(); $holder = Requirements::backend(); Requirements::set_backend($backend); $data = new ArrayData(array( 'FailTest' => true, )); $data->renderWith('RequirementsTest_Conditionals'); - $backend->assertFileIncluded('css', $basePath .'/RequirementsTest_a.css'); - $backend->assertFileIncluded('js', + $this->assertFileIncluded($backend, 'css', $basePath .'/RequirementsTest_a.css'); + $this->assertFileIncluded($backend, 'js', array($basePath .'/RequirementsTest_b.js', $basePath .'/RequirementsTest_c.js')); - $backend->assertFileNotIncluded('js', $basePath .'/RequirementsTest_a.js'); - $backend->assertFileNotIncluded('css', + $this->assertFileNotIncluded($backend, 'js', $basePath .'/RequirementsTest_a.js'); + $this->assertFileNotIncluded($backend, 'css', array($basePath .'/RequirementsTest_b.css', $basePath .'/RequirementsTest_c.css')); $backend->clear(); $data = new ArrayData(array( 'FailTest' => false, )); $data->renderWith('RequirementsTest_Conditionals'); - $backend->assertFileNotIncluded('css', $basePath .'/RequirementsTest_a.css'); - $backend->assertFileNotIncluded('js', + $this->assertFileNotIncluded($backend, 'css', $basePath .'/RequirementsTest_a.css'); + $this->assertFileNotIncluded($backend, 'js', array($basePath .'/RequirementsTest_b.js', $basePath .'/RequirementsTest_c.js')); - $backend->assertFileIncluded('js', $basePath .'/RequirementsTest_a.js'); - $backend->assertFileIncluded('css', + $this->assertFileIncluded($backend, 'js', $basePath .'/RequirementsTest_a.js'); + $this->assertFileIncluded($backend, 'css', array($basePath .'/RequirementsTest_b.css', $basePath .'/RequirementsTest_c.css')); Requirements::set_backend($holder); } @@ -323,82 +323,93 @@ class RequirementsTest extends SapphireTest { $html = $backend->includeInHTML(false, $template); $this->assertNotContains('assertContains('', $html); - } -} -class RequirementsTest_Backend extends Requirements_Backend implements TestOnly { - public function assertFileIncluded($type, $files) { + public function assertFileIncluded($backend, $type, $files) { $type = strtolower($type); switch (strtolower($type)) { case 'css': - $var = 'css'; + $method = 'get_css'; $type = 'CSS'; break; case 'js': case 'javascript': case 'script': - $var = 'javascript'; + $method = 'get_javascript'; $type = 'JavaScript'; break; } + $includedFiles = $backend->$method(); + + // Workaround for inconsistent return formats + if($method == 'get_javascript') { + $includedFiles = array_combine(array_values($includedFiles), array_values($includedFiles)); + } + if(is_array($files)) { $failedMatches = array(); foreach ($files as $file) { - if(!array_key_exists($file, $this->$var)) { + if(!array_key_exists($file, $includedFiles)) { $failedMatches[] = $file; } } - if(count($failedMatches) > 0) throw new PHPUnit_Framework_AssertionFailedError( + $this->assertTrue( + (count($failedMatches) == 0), "Failed asserting the $type files '" . implode("', '", $failedMatches) . "' have exact matches in the required elements:\n'" - . implode("'\n'", array_keys($this->$var)) . "'" + . implode("'\n'", array_keys($includedFiles)) . "'" ); } else { - if(!array_key_exists($files, $this->$var)) { - throw new PHPUnit_Framework_AssertionFailedError( - "Failed asserting the $type file '$files' has an exact match in the required elements:\n'" - . implode("'\n'", array_keys($this->$var)) . "'" - ); - } + $this->assertTrue( + (array_key_exists($files, $includedFiles)), + "Failed asserting the $type file '$files' has an exact match in the required elements:\n'" + . implode("'\n'", array_keys($includedFiles)) . "'" + ); } } - public function assertFileNotIncluded($type, $files) { + public function assertFileNotIncluded($backend, $type, $files) { $type = strtolower($type); switch ($type) { case 'css': - $var = 'css'; + $method = 'get_css'; $type = 'CSS'; break; case 'js': - case 'javascript': + case 'get_javascript': case 'script': - $var = 'javascript'; + $method = 'get_javascript'; $type = 'JavaScript'; break; } + $includedFiles = $backend->$method(); + + // Workaround for inconsistent return formats + if($method == 'get_javascript') { + $includedFiles = array_combine(array_values($includedFiles), array_values($includedFiles)); + } + if(is_array($files)) { $failedMatches = array(); foreach ($files as $file) { - if(array_key_exists($file, $this->$var)) { + if(array_key_exists($file, $includedFiles)) { $failedMatches[] = $file; } } - if(count($failedMatches) > 0) throw new PHPUnit_Framework_AssertionFailedError( + $this->assertTrue( + (count($failedMatches) == 0), "Failed asserting the $type files '" . implode("', '", $failedMatches) . "' do not have exact matches in the required elements:\n'" - . implode("'\n'", array_keys($this->$var)) . "'" + . implode("'\n'", array_keys($includedFiles)) . "'" ); } else { - if(array_key_exists($files, $this->$var)) { - throw new PHPUnit_Framework_AssertionFailedError( - "Failed asserting the $type file '$files' does not have an exact match in the required elements:" - . "\n'" . implode("'\n'", array_keys($this->$var)) . "'" - ); - } + $this->assertFalse( + (array_key_exists($files, $includedFiles)), + "Failed asserting the $type file '$files' does not have an exact match in the required elements:" + . "\n'" . implode("'\n'", array_keys($includedFiles)) . "'" + ); } } -} +} \ No newline at end of file diff --git a/tests/model/AggregateTest.php b/tests/model/AggregateTest.php index 80bbdad6c..2d71cbf0c 100644 --- a/tests/model/AggregateTest.php +++ b/tests/model/AggregateTest.php @@ -148,7 +148,7 @@ class AggregateTest extends SapphireTest { * Test aggregates are cached properly */ public function testCache() { - + $this->markTestIncomplete(); } /* */ diff --git a/tests/model/DataListTest.php b/tests/model/DataListTest.php index 4e17fb4ca..6a5830e8e 100644 --- a/tests/model/DataListTest.php +++ b/tests/model/DataListTest.php @@ -159,8 +159,8 @@ class DataListTest extends SapphireTest { } public function testFilter() { - // coming soon! - } + $this->markTestIncomplete(); + } public function testWhere() { // We can use raw SQL queries with where. This is only recommended for advanced uses; diff --git a/tests/model/DataObjectTest.php b/tests/model/DataObjectTest.php index fe0889584..df4d59e0a 100644 --- a/tests/model/DataObjectTest.php +++ b/tests/model/DataObjectTest.php @@ -1039,23 +1039,18 @@ class DataObjectTest extends SapphireTest { ); } + /** + * @expectedException LogicException + */ public function testInvalidate() { $do = new DataObjectTest_Fixture(); $do->write(); $do->delete(); - try { - // Prohibit invalid object manipulation - $do->delete(); - $do->write(); - $do->duplicate(); - } - catch(Exception $e) { - return; - } - - $this->fail('Should throw an exception'); + $do->delete(); // Prohibit invalid object manipulation + $do->write(); + $do->duplicate(); } public function testToMap() { diff --git a/tests/model/DatabaseTest.php b/tests/model/DatabaseTest.php index ff8832956..7a246a7e4 100644 --- a/tests/model/DatabaseTest.php +++ b/tests/model/DatabaseTest.php @@ -51,15 +51,18 @@ class DatabaseTest extends SapphireTest { } public function testMySQLCreateTableOptions() { - if(DB::getConn() instanceof MySQLDatabase) { - $ret = DB::query(sprintf( - 'SHOW TABLE STATUS WHERE "Name" = \'%s\'', - 'DatabaseTest_MyObject' - ))->first(); - $this->assertEquals($ret['Engine'],'InnoDB', - "MySQLDatabase tables can be changed to InnoDB through DataObject::\$create_table_options" - ); + if(!(DB::getConn() instanceof MySQLDatabase)) { + $this->markTestSkipped('MySQL only'); } + + + $ret = DB::query(sprintf( + 'SHOW TABLE STATUS WHERE "Name" = \'%s\'', + 'DatabaseTest_MyObject' + ))->first(); + $this->assertEquals($ret['Engine'],'InnoDB', + "MySQLDatabase tables can be changed to InnoDB through DataObject::\$create_table_options" + ); } function testIsSchemaUpdating() { diff --git a/tests/security/PasswordValidatorTest.php b/tests/security/PasswordValidatorTest.php index 3c98546b0..79dc7b915 100644 --- a/tests/security/PasswordValidatorTest.php +++ b/tests/security/PasswordValidatorTest.php @@ -39,6 +39,6 @@ class PasswordValidatorTest extends SapphireTest { } public function testHistoricalPasswordCount() { - // TODO + $this->markTestIncomplete(); } } diff --git a/tests/security/SecurityDefaultAdminTest.php b/tests/security/SecurityDefaultAdminTest.php index 04bdfe8a1..734e42f0e 100644 --- a/tests/security/SecurityDefaultAdminTest.php +++ b/tests/security/SecurityDefaultAdminTest.php @@ -11,9 +11,12 @@ class SecurityDefaultAdminTest extends SapphireTest { } public function testCheckDefaultAdmin() { - // TODO There's currently no way to inspect default admin state, - // hence we don't override existing settings - if(Security::has_default_admin()) return; + if(Security::has_default_admin()) { + $this->markTestSkipped( + 'Default admin present. There\'s no way to inspect default admin state, ' . + 'so we don\'t override existing settings' + ); + } Security::setDefaultAdmin('admin', 'password'); diff --git a/tests/view/SSViewerCacheBlockTest.php b/tests/view/SSViewerCacheBlockTest.php index 6ad5f4ce1..7350da29e 100644 --- a/tests/view/SSViewerCacheBlockTest.php +++ b/tests/view/SSViewerCacheBlockTest.php @@ -206,7 +206,7 @@ class SSViewerCacheBlockTest extends SapphireTest { public function testNoErrorMessageForControlWithinCached() { $this->_reset(true); - $this->_runtemplate('<% cached %><% control Foo %>$Bar<% end_control %><% end_cached %>'); + $this->assertNotNull($this->_runtemplate('<% cached %><% control Foo %>$Bar<% end_control %><% end_cached %>')); } /** @@ -220,8 +220,8 @@ class SSViewerCacheBlockTest extends SapphireTest { public function testNoErrorMessageForCachedWithinControlWithinUncached() { $this->_reset(true); - $this->_runtemplate( - '<% uncached %><% control Foo %><% cached %>$Bar<% end_cached %><% end_control %><% end_uncached %>'); + $this->assertNotNull($this->_runtemplate( + '<% uncached %><% control Foo %><% cached %>$Bar<% end_cached %><% end_control %><% end_uncached %>')); } /**