Merge remote-tracking branch 'origin/4.0' into 4

# Conflicts:
#	src/Core/TempFolder.php
#	src/ORM/DataObject.php
#	src/View/ThemeResourceLoader.php
#	src/includes/constants.php
#	tests/php/Control/SimpleResourceURLGeneratorTest.php
#	tests/php/Forms/HTMLEditor/HTMLEditorFieldTest.php
#	tests/php/View/RequirementsTest.php

docs/en/02_Developer_Guides/15_Customising_the_Admin_Interface/05_Typography.md

@@ -4,25 +4,34 @@ summary: Add custom CSS properties to the rich-text editor.
 # WYSIWYG Styles
 
 SilverStripe lets you customise the style of content in the CMS. This is done by setting up a CSS file called
-`editor.css` in either your theme or in your `mysite` folder. This is set through
+`editor.css` in either your theme or in your `mysite` folder. This is set through yaml config:
 
-
+```yaml
-```php
+---
-use SilverStripe\Forms\HTMLEditor\HtmlEditorConfig;
+name: MyCSS
-
+---
-HtmlEditorConfig::get('cms')->setOption('content_css', project() . '/css/editor.css');
+SilverStripe\Forms\HTMLEditor\TinyMCEConfig:
+  editor_css:
+    - 'mysite/css/editor.css'
 ```
 
 Will load the `mysite/css/editor.css` file.
 
-If using this config option in `mysite/_config.php`, you will have to instead call:
+## Custom style dropdown
 
+The custom style dropdown can be enabled via the `importcss` plugin bundled with admin module. ([Doc](https://www.tinymce.com/docs/plugins/importcss/))  
+Use the below code in `mysite/_config.php`:
 
 ```php
-HtmlEditorConfig::get('cms')->setOption('content_css', project() . '/css/editor.css');
+use SilverStripe\Forms\HTMLEditor\TinyMCEConfig;
+
+TinyMCEConfig::get('cms')
+    ->addButtonsToLine(1, 'styleselect')
+    ->setOption('importcss_append', true);
 ```
 
-Any CSS classes within this file will be automatically added to the `WYSIWYG` editors 'style' dropdown. For instance, to
+Any CSS classes within this file will be automatically added to the `WYSIWYG` editors 'style' dropdown.  
+For instance, to
 add the color 'red' as an option within the `WYSIWYG` add the following to the `editor.css`
 
 
@@ -31,10 +40,64 @@ add the color 'red' as an option within the `WYSIWYG` add the following to the `
     color: red;
 }
 ```
+Adding a tag to the selector will automatically wrap with this tag. For example : 
+```css
+h4.red {
+    color: red;
+}
+```
+will add an `h4` tag to the selected block.
+
+For further customisation, customize the `style_formats` option.  
+`style_formats` won't be applied if you do not enable `importcss_append`.  
+Here is a working example to get you started.  
+See related [tinymce doc](https://www.tinymce.com/docs/configure/content-formatting/#style_formats).
+
+```php
+use SilverStripe\Forms\HTMLEditor\TinyMCEConfig;
+
+$formats = [
+    [ 'title' => 'Headings', 'items' => [
+            ['title' => 'Heading 1', 'block' => 'h1' ],
+            ['title' => 'Heading 2', 'block' => 'h2' ],
+            ['title' => 'Heading 3', 'block' => 'h3' ],
+            ['title' => 'Heading 4', 'block' => 'h4' ],
+            ['title' => 'Heading 5', 'block' => 'h5' ],
+            ['title' => 'Heading 6', 'block' => 'h6' ],
+            [
+                'title'           => 'Subtitle',
+                'selector'        => 'p',
+                'classes'         => 'title-sub',
+            ],
+        ]
+    ],
+    [
+        'title' => 'Misc Styles', 'items' => [
+            [
+                'title' => 'Style 1',
+                'selector' => 'ul',
+                'classes' => 'style1',
+                'wrapper' => true,
+                'merge_siblings' => false,
+            ],
+            [
+                'title' => 'Button red',
+                'inline' => 'span',
+                'classes' => 'btn-red',
+                'merge_siblings' => true,
+            ],
+        ]
+    ],
+];
+
+TinyMCEConfig::get('cms')
+    ->addButtonsToLine(1, 'styleselect')
+    ->setOptions([
+        'importcss_append' => true,
+        'style_formats' => $formats,
+    ]);
+```
 
-<div class="notice" markdown="1">
-After you have defined the `editor.css` make sure you clear your SilverStripe cache for it to take effect.
-</div>
 
 ## API Documentation
 

phpcs.xml.dist

@@ -18,6 +18,12 @@
 		<exclude name="Generic.Files.LineLength.TooLong" />
 		<exclude name="PEAR.Functions.ValidDefaultValue.NotAtEnd" />
 	</rule>
+	<rule phpcbf-only="true" ref="Squiz.Strings.ConcatenationSpacing">
+		<properties>
+			<property name="spacing" value="1" />
+			<property name="ignoreNewlines" value="true"/>
+		</properties>
+	</rule>
 
 	<!-- include php files only -->
 	<arg name="extensions" value="php,lib,inc,php5"/>

src/Control/HTTPResponse.php

@@ -245,7 +245,7 @@ class HTTPResponse
      */
     public function removeHeader($header)
     {
-        strtolower($header);
+        $header = strtolower($header);
         unset($this->headers[$header]);
         return $this;
     }

src/Control/RSS/RSSFeed.php

@@ -157,8 +157,7 @@ class RSSFeed extends ViewableData
     {
         $title = Convert::raw2xml($title);
         Requirements::insertHeadTags(
-            '<link rel="alternate" type="application/rss+xml" title="' . $title .
+            '<link rel="alternate" type="application/rss+xml" title="' . $title . '" href="' . $url . '" />'
-            '" href="' . $url . '" />'
         );
     }
 

src/Control/RSS/RSSFeed_Entry.php

@@ -130,9 +130,7 @@ class RSSFeed_Entry extends ViewableData
         }
 
         throw new BadMethodCallException(
-            get_class($this->failover) .
+            get_class($this->failover) . " object has neither an AbsoluteLink nor a Link method." . " Can't put a link in the RSS feed",
-            " object has neither an AbsoluteLink nor a Link method." .
-            " Can't put a link in the RSS feed",
             E_USER_WARNING
         );
     }

src/Control/RequestHandler.php

@@ -266,8 +266,7 @@ class RequestHandler extends ViewableData
                             $class = static::class;
                             $latestParams = var_export($request->latestParams(), true);
                             Debug::message(
-                                "Rule '{$rule}' matched to action '{$action}' on {$class}. ".
+                                "Rule '{$rule}' matched to action '{$action}' on {$class}. " . "Latest request params: {$latestParams}"
-                                "Latest request params: {$latestParams}"
                             );
                         }
 
@@ -566,8 +565,7 @@ class RequestHandler extends ViewableData
 
         // no link defined by default
         trigger_error(
-            'Request handler '.static::class. ' does not have a url_segment defined. '.
+            'Request handler ' . static::class . ' does not have a url_segment defined. ' . 'Relying on this link may be an application error',
-            'Relying on this link may be an application error',
             E_USER_WARNING
         );
         return null;

src/Core/TempFolder.php

@@ -108,9 +108,7 @@ class TempFolder
 
         if (!$worked) {
             throw new Exception(
-                'Permission problem gaining access to a temp folder. ' .
+                'Permission problem gaining access to a temp folder. ' . 'Please create a folder named silverstripe-cache in the base folder ' . 'of the installation and ensure it has the correct permissions'
-                'Please create a folder named silverstripe-cache in the base folder ' .
-                'of the installation and ensure it has the correct permissions'
             );
         }
 

src/Dev/Install/MySQLDatabaseConfigurationHelper.php

@@ -247,8 +247,7 @@ class MySQLDatabaseConfigurationHelper implements DatabaseConfigurationHelper
             preg_quote('"%".*'),
             preg_quote('*.*')
         );
-        $expression = '/GRANT[ ,\w]+((ALL PRIVILEGES)|('.$permission.'(?! ((VIEW)|(ROUTINE)))))[ ,\w]+ON '.
+        $expression = '/GRANT[ ,\w]+((ALL PRIVILEGES)|(' . $permission . '(?! ((VIEW)|(ROUTINE)))))[ ,\w]+ON ' . $dbPattern . '/i';
-            $dbPattern.'/i';
         return preg_match($expression, $grant);
     }
 

src/Forms/FormRequestHandler.php

@@ -144,8 +144,7 @@ class FormRequestHandler extends RequestHandler
             if (empty($vars[$securityID])) {
                 $this->httpError(400, _t(
                     "SilverStripe\\Forms\\Form.CSRF_FAILED_MESSAGE",
-                    "There seems to have been a technical problem. Please click the back button, ".
+                    "There seems to have been a technical problem. Please click the back button, " . "refresh your browser, and try again."
-                    "refresh your browser, and try again."
                 ));
             } else {
                 // Clear invalid token on refresh
@@ -228,6 +227,7 @@ class FormRequestHandler extends RequestHandler
             // First, try a handler method on the controller (has been checked for allowed_actions above already)
             $controller = $this->form->getController();
             if ($controller && $controller->hasMethod($funcName)) {
+                $controller->setRequest($request);
                 return $controller->$funcName($vars, $this->form, $request, $this);
             }
 
@@ -257,8 +257,7 @@ class FormRequestHandler extends RequestHandler
         $legacyActions = $this->form->config()->get('allowed_actions');
         if ($legacyActions) {
             throw new BadMethodCallException(
-                "allowed_actions are not valid on Form class " . get_class($this->form) .
+                "allowed_actions are not valid on Form class " . get_class($this->form) . ". Implement these in subclasses of " . static::class . " instead"
-                ". Implement these in subclasses of " . static::class . " instead"
             );
         }
 

src/Forms/GridField/GridField.php

@@ -897,8 +897,7 @@ class GridField extends FormField
         if (!$token->checkRequest($request)) {
             $this->httpError(400, _t(
                 "SilverStripe\\Forms\\Form.CSRF_FAILED_MESSAGE",
-                "There seems to have been a technical problem. Please click the back button, ".
+                "There seems to have been a technical problem. Please click the back button, " . "refresh your browser, and try again."
-                "refresh your browser, and try again."
             ));
         }
 

src/Forms/GridField/GridFieldConfig_RecordEditor.php

@@ -1,6 +1,8 @@
 <?php
 namespace SilverStripe\Forms\GridField;
 
+use SilverStripe\Versioned\Versioned;
+
 /**
  * Allows editing of records contained within the GridField, instead of only allowing the ability to view records in
  * the GridField.
@@ -21,7 +23,10 @@ class GridFieldConfig_RecordEditor extends GridFieldConfig
         $this->addComponent($sort = new GridFieldSortableHeader());
         $this->addComponent($filter = new GridFieldFilterHeader());
         $this->addComponent(new GridFieldDataColumns());
+        // @todo Move to versioned module, add via extension instead
+        if (class_exists(Versioned::class)) {
             $this->addComponent(new GridFieldVersionedState(['Name', 'Title']));
+        }
         $this->addComponent(new GridFieldEditButton());
         $this->addComponent(new GridFieldDeleteAction());
         $this->addComponent(new GridFieldPageCount('toolbar-header-right'));

src/Forms/GridField/GridFieldPageCount.php

@@ -51,8 +51,7 @@ class GridFieldPageCount implements GridField_HTMLProvider
 
         if (!$paginator && GridFieldPageCount::config()->uninherited('require_paginator')) {
             throw new LogicException(
-                static::class . " relies on a GridFieldPaginator to be added " .
+                static::class . " relies on a GridFieldPaginator to be added " . "to the same GridField, but none are present."
-                "to the same GridField, but none are present."
             );
         }
 

src/Forms/GridField/GridFieldVersionedState.php

@@ -1,12 +1,21 @@
 <?php
+
 namespace SilverStripe\Forms\GridField;
 
 use SilverStripe\ORM\DataObject;
 use SilverStripe\Versioned\Versioned;
 use SilverStripe\Core\Convert;
 
+/**
+ * @todo Move to siverstripe/versioned module
+ */
 class GridFieldVersionedState implements GridField_ColumnProvider
 {
+    /**
+     * Column name for versioned state
+     *
+     * @var string
+     */
     protected $column = null;
 
     /**
@@ -33,9 +42,12 @@ class GridFieldVersionedState implements GridField_ColumnProvider
      */
     public function augmentColumns($gridField, &$columns)
     {
+        if (!class_exists(Versioned::class)) {
+            return;
+        }
+
         $model = $gridField->getModelClass();
         $isModelVersioned = $model::has_extension(Versioned::class);
-
         if (!$isModelVersioned) {
             return;
         }
@@ -61,7 +73,7 @@ class GridFieldVersionedState implements GridField_ColumnProvider
      */
     public function getColumnsHandled($gridField)
     {
-        return [$this->column];
+        return $this->column ? [$this->column] : [];
     }
 
     /**
@@ -74,7 +86,6 @@ class GridFieldVersionedState implements GridField_ColumnProvider
      */
     public function getColumnContent($gridField, $record, $columnName)
     {
-
         $flagContent = '';
         $flags = $this->getStatusFlags($record);
         foreach ($flags as $class => $data) {
@@ -140,13 +151,16 @@ class GridFieldVersionedState implements GridField_ColumnProvider
      *   )
      * ```
      *
-     * @param DataObject $record - the record to check status for
+     * @param Versioned|DataObject $record - the record to check status for
      * @return array
      */
     protected function getStatusFlags($record)
     {
-        $flags = array();
+        if (!$record->hasExtension(Versioned::class)) {
+            return [];
+        }
 
+        $flags = [];
         if ($record->isOnLiveOnly()) {
             $flags['removedfromdraft'] = array(
                 'text' => _t(__CLASS__ . '.ONLIVEONLYSHORT', 'On live only'),

src/Forms/HTMLEditor/TinyMCECombinedGenerator.php

@@ -168,8 +168,7 @@ SCRIPT;
         // Join list of paths
         $filesList = Convert::raw2js(implode(',', $fileURLS));
         // Mark all themes, plugins and languages as done
-        $buffer[] = "window.tinymce.each('$filesList'.split(',')," .
+        $buffer[] = "window.tinymce.each('$filesList'.split(',')," . "function(f){tinymce.ScriptLoader.markDone(baseURL+f);});";
-            "function(f){tinymce.ScriptLoader.markDone(baseURL+f);});";
 
         $buffer[] = '})();';
         return implode("\n", $buffer) . "\n";

src/ORM/FieldType/DBCurrency.php

@@ -17,7 +17,6 @@ namespace SilverStripe\ORM\FieldType;
  */
 class DBCurrency extends DBDecimal
 {
-
     /**
      * @config
      * @var string
@@ -38,9 +37,9 @@ class DBCurrency extends DBDecimal
         $val = $this->config()->currency_symbol . number_format(abs($this->value), 2);
         if ($this->value < 0) {
             return "($val)";
-        } else {
-            return $val;
         }
+
+        return $val;
     }
 
     /**
@@ -51,9 +50,8 @@ class DBCurrency extends DBDecimal
         $val = $this->config()->currency_symbol . number_format(abs($this->value), 0);
         if ($this->value < 0) {
             return "($val)";
-        } else {
-            return $val;
         }
+        return $val;
     }
 
     public function setValue($value, $record = null, $markChanged = true)
@@ -62,9 +60,11 @@ class DBCurrency extends DBDecimal
         if (is_numeric($value)) {
             $this->value = $value;
         } elseif (preg_match('/-?\$?[0-9,]+(.[0-9]+)?([Ee][0-9]+)?/', $value, $matches)) {
-            $this->value = str_replace(array('$',',',$this->config()->currency_symbol), '', $matches[0]);
+            $this->value = str_replace(['$', ',', $this->config()->currency_symbol], '', $matches[0]);
         } else {
             $this->value = 0;
         }
+
+        return $this;
     }
 }

src/Security/BasicAuth.php

@@ -110,10 +110,6 @@ class BasicAuth
             return true;
         }
 
-        if ($member instanceof Member) {
-            Security::setCurrentUser($member);
-        }
-
         if (!$member && $tryUsingSessionLogin) {
             $member = Security::getCurrentUser();
         }

src/Security/CMSSecurity.php

@@ -198,8 +198,7 @@ PHP
         $controller = $controller->customise(array(
             'Content' => DBField::create_field(DBHTMLText::class, _t(
                 __CLASS__ . '.SUCCESSCONTENT',
-                '<p>Login success. If you are not automatically redirected ' .
+                '<p>Login success. If you are not automatically redirected ' . '<a target="_top" href="{link}">click here</a></p>',
-                '<a target="_top" href="{link}">click here</a></p>',
                 'Login message displayed in the cms popup once a user has re-authenticated themselves',
                 array('link' => Convert::raw2att($backURL))
             ))

src/Security/InheritedPermissions.php

@@ -360,8 +360,7 @@ class InheritedPermissions implements PermissionChecker, MemberCacheFlusher
             $baseTable = DataObject::getSchema()->baseDataTable($this->getBaseClass());
             $uninheritedPermissions = $stageRecords
                 ->where([
-                    "(\"$typeField\" IN (?, ?) OR " .
+                    "(\"$typeField\" IN (?, ?) OR " . "(\"$typeField\" = ? AND \"$groupJoinTable\".\"{$baseTable}ID\" IS NOT NULL))"
-                    "(\"$typeField\" = ? AND \"$groupJoinTable\".\"{$baseTable}ID\" IS NOT NULL))"
                     => [
                         self::ANYONE,
                         self::LOGGED_IN_USERS,
@@ -370,8 +369,7 @@ class InheritedPermissions implements PermissionChecker, MemberCacheFlusher
                 ])
                 ->leftJoin(
                     $groupJoinTable,
-                    "\"$groupJoinTable\".\"{$baseTable}ID\" = \"{$baseTable}\".\"ID\" AND " .
+                    "\"$groupJoinTable\".\"{$baseTable}ID\" = \"{$baseTable}\".\"ID\" AND " . "\"$groupJoinTable\".\"GroupID\" IN ($groupIDsSQLList)"
-                    "\"$groupJoinTable\".\"GroupID\" IN ($groupIDsSQLList)"
                 )->column('ID');
         } else {
             // Only view pages with ViewType = Anyone if not logged in

src/Security/Member.php

@@ -351,8 +351,7 @@ class Member extends DataObject
             $result->addError(
                 _t(
                     __CLASS__ . '.ERRORLOCKEDOUT2',
-                    'Your account has been temporarily disabled because of too many failed attempts at ' .
+                    'Your account has been temporarily disabled because of too many failed attempts at ' . 'logging in. Please try again in {count} minutes.',
-                    'logging in. Please try again in {count} minutes.',
                     null,
                     array('count' => static::config()->get('lock_out_delay_mins'))
                 )

src/View/Parsers/HTML4Value.php

@@ -22,8 +22,7 @@ class HTML4Value extends HTMLValue
 
         $errorState = libxml_use_internal_errors(true);
         $result = $this->getDocument()->loadHTML(
-            '<html><head><meta http-equiv="content-type" content="text/html; charset=utf-8"></head>' .
+            '<html><head><meta http-equiv="content-type" content="text/html; charset=utf-8"></head>' . "<body>$content</body></html>"
-            "<body>$content</body></html>"
         );
         libxml_clear_errors();
         libxml_use_internal_errors($errorState);

src/View/Parsers/ShortcodeParser.php

@@ -374,11 +374,9 @@ class ShortcodeParser
                 if ($i == 0) {
                     $err = 'Close tag "' . $tags[$i]['close'] . '" is the first found tag, so has no related open tag';
                 } elseif (!$tags[$i-1]['open']) {
-                    $err = 'Close tag "'.$tags[$i]['close'].'" preceded by another close tag "'.
+                    $err = 'Close tag "' . $tags[$i]['close'] . '" preceded by another close tag "' . $tags[$i-1]['close'] . '"';
-                            $tags[$i-1]['close'].'"';
                 } elseif ($tags[$i]['close'] != $tags[$i-1]['open']) {
-                    $err = 'Close tag "'.$tags[$i]['close'].'" doesn\'t match preceding open tag "'.
+                    $err = 'Close tag "' . $tags[$i]['close'] . '" doesn\'t match preceding open tag "' . $tags[$i-1]['open'] . '"';
-                            $tags[$i-1]['open'].'"';
                 }
 
                 if ($err) {
@@ -599,8 +597,7 @@ class ShortcodeParser
         elseif ($location == self::INLINE) {
             if (in_array(strtolower($node->tagName), self::$block_level_elements)) {
                 user_error(
-                    'Requested to insert block tag '.$node->tagName.
+                    'Requested to insert block tag ' . $node->tagName . ' inline - probably this will break HTML compliance',
-                    ' inline - probably this will break HTML compliance',
                     E_USER_WARNING
                 );
             }

src/View/ViewableData_Debugger.php

@@ -44,9 +44,7 @@ class ViewableData_Debugger extends ViewableData
         // debugging info for a specific field
         $class = get_class($this->object);
         if ($field) {
-            return "<b>Debugging Information for {$class}->{$field}</b><br/>" .
+            return "<b>Debugging Information for {$class}->{$field}</b><br/>" . ($this->object->hasMethod($field) ? "Has method '$field'<br/>" : null) . ($this->object->hasField($field) ? "Has field '$field'<br/>" : null);
-                ($this->object->hasMethod($field) ? "Has method '$field'<br/>" : null) .
-                ($this->object->hasField($field) ? "Has field '$field'<br/>" : null);
         }
 
         // debugging information for the entire class

src/includes/constants.php

@@ -9,7 +9,6 @@ use SilverStripe\Core\TempFolder;
  * This file is the Framework constants bootstrap. It will prepare some basic common constants.
  *
  * It takes care of:
- *  - Normalisation of $_SERVER values
  *  - Initialisation of necessary constants (mostly paths)
  *
  * Initialized constants:
@@ -24,6 +23,8 @@ use SilverStripe\Core\TempFolder;
  * - THEMES_PATH: Absolute filepath, e.g. "/var/www/my-webroot/themes"
  * - FRAMEWORK_DIR: Path relative to webroot, e.g. "framework"
  * - FRAMEWORK_PATH:Absolute filepath, e.g. "/var/www/my-webroot/framework"
+ * - PUBLIC_DIR: Webroot path relative to project root, e.g. "public" or ""
+ * - PUBLIC_PATH: Absolute path to webroot, e.g. "/var/www/project/public"
  * - THIRDPARTY_DIR: Path relative to webroot, e.g. "framework/thirdparty"
  * - THIRDPARTY_PATH: Absolute filepath, e.g. "/var/www/my-webroot/framework/thirdparty"
  */

tests/bootstrap/init.php

@@ -1,9 +1,7 @@
 <?php
 
 if (!defined('BASE_PATH')) {
-    echo "BASE_PATH hasn't been defined. This probably means that framework/Core/Constants.php hasn't been " .
+    echo "BASE_PATH hasn't been defined. This probably means that framework/Core/Constants.php hasn't been " . "included by Composer's autoloader.\n" . "Make sure the you are running your tests via vendor/bin/phpunit and your autoloader is up to date.\n";
-        "included by Composer's autoloader.\n" .
-        "Make sure the you are running your tests via vendor/bin/phpunit and your autoloader is up to date.\n";
     exit(1);
 }
 

tests/php/Control/ControllerTest.php

@@ -108,40 +108,35 @@ class ControllerTest extends FunctionalTest
         $this->assertEquals(
             200,
             $response->getStatusCode(),
-            'Access granted on index action without $allowed_actions on defining controller, ' .
+            'Access granted on index action without $allowed_actions on defining controller, ' . 'when called without an action in the URL'
-            'when called without an action in the URL'
         );
 
         $response = $this->get("UnsecuredController/index");
         $this->assertEquals(
             200,
             $response->getStatusCode(),
-            'Access denied on index action without $allowed_actions on defining controller, ' .
+            'Access denied on index action without $allowed_actions on defining controller, ' . 'when called with an action in the URL'
-            'when called with an action in the URL'
         );
 
         $response = $this->get("UnsecuredController/method1");
         $this->assertEquals(
             403,
             $response->getStatusCode(),
-            'Access denied on action without $allowed_actions on defining controller, ' .
+            'Access denied on action without $allowed_actions on defining controller, ' . 'when called without an action in the URL'
-            'when called without an action in the URL'
         );
 
         $response = $this->get("AccessBaseController/");
         $this->assertEquals(
             200,
             $response->getStatusCode(),
-            'Access granted on index with empty $allowed_actions on defining controller, ' .
+            'Access granted on index with empty $allowed_actions on defining controller, ' . 'when called without an action in the URL'
-            'when called without an action in the URL'
         );
 
         $response = $this->get("AccessBaseController/index");
         $this->assertEquals(
             200,
             $response->getStatusCode(),
-            'Access granted on index with empty $allowed_actions on defining controller, ' .
+            'Access granted on index with empty $allowed_actions on defining controller, ' . 'when called with an action in the URL'
-            'when called with an action in the URL'
         );
 
         $response = $this->get("AccessBaseController/method1");
@@ -155,48 +150,42 @@ class ControllerTest extends FunctionalTest
         $this->assertEquals(
             403,
             $response->getStatusCode(),
-            'Access denied on action with empty $allowed_actions on defining controller, ' .
+            'Access denied on action with empty $allowed_actions on defining controller, ' . 'even when action is allowed in subclasses (allowed_actions don\'t inherit)'
-            'even when action is allowed in subclasses (allowed_actions don\'t inherit)'
         );
 
         $response = $this->get("AccessSecuredController/");
         $this->assertEquals(
             200,
             $response->getStatusCode(),
-            'Access granted on index with non-empty $allowed_actions on defining controller, ' .
+            'Access granted on index with non-empty $allowed_actions on defining controller, ' . 'even when index isn\'t specifically mentioned in there'
-            'even when index isn\'t specifically mentioned in there'
         );
 
         $response = $this->get("AccessSecuredController/method1");
         $this->assertEquals(
             403,
             $response->getStatusCode(),
-            'Access denied on action which is only defined in parent controller, ' .
+            'Access denied on action which is only defined in parent controller, ' . 'even when action is allowed in currently called class (allowed_actions don\'t inherit)'
-            'even when action is allowed in currently called class (allowed_actions don\'t inherit)'
         );
 
         $response = $this->get("AccessSecuredController/method2");
         $this->assertEquals(
             200,
             $response->getStatusCode(),
-            'Access granted on action originally defined with empty $allowed_actions on parent controller, ' .
+            'Access granted on action originally defined with empty $allowed_actions on parent controller, ' . 'because it has been redefined in the subclass'
-            'because it has been redefined in the subclass'
         );
 
         $response = $this->get("AccessSecuredController/templateaction");
         $this->assertEquals(
             403,
             $response->getStatusCode(),
-            'Access denied on action with $allowed_actions on defining controller, ' .
+            'Access denied on action with $allowed_actions on defining controller, ' . 'if action is not a method but rather a template discovered by naming convention'
-            'if action is not a method but rather a template discovered by naming convention'
         );
 
         $response = $this->get("AccessSecuredController/templateaction");
         $this->assertEquals(
             403,
             $response->getStatusCode(),
-            'Access denied on action with $allowed_actions on defining controller, ' .
+            'Access denied on action with $allowed_actions on defining controller, ' . 'if action is not a method but rather a template discovered by naming convention'
-            'if action is not a method but rather a template discovered by naming convention'
         );
 
         Security::setCurrentUser($adminUser);
@@ -204,8 +193,7 @@ class ControllerTest extends FunctionalTest
         $this->assertEquals(
             200,
             $response->getStatusCode(),
-            'Access granted for logged in admin on action with $allowed_actions on defining controller, ' .
+            'Access granted for logged in admin on action with $allowed_actions on defining controller, ' . 'if action is not a method but rather a template discovered by naming convention'
-            'if action is not a method but rather a template discovered by naming convention'
         );
 
         Security::setCurrentUser(null);
@@ -213,16 +201,14 @@ class ControllerTest extends FunctionalTest
         $this->assertEquals(
             403,
             $response->getStatusCode(),
-            'Access denied on action with $allowed_actions on defining controller, ' .
+            'Access denied on action with $allowed_actions on defining controller, ' . 'when restricted by unmatched permission code'
-            'when restricted by unmatched permission code'
         );
 
         $response = $this->get("AccessSecuredController/aDmiNOnlY");
         $this->assertEquals(
             403,
             $response->getStatusCode(),
-            'Access denied on action with $allowed_actions on defining controller, ' .
+            'Access denied on action with $allowed_actions on defining controller, ' . 'regardless of capitalization'
-            'regardless of capitalization'
         );
 
         $response = $this->get('AccessSecuredController/protectedmethod');
@@ -245,40 +231,35 @@ class ControllerTest extends FunctionalTest
         $this->assertEquals(
             200,
             $response->getStatusCode(),
-            "Access granted to method defined in allowed_actions on extension, " .
+            "Access granted to method defined in allowed_actions on extension, " . "where method is also defined on extension"
-            "where method is also defined on extension"
         );
 
         $response = $this->get('AccessSecuredController/extensionmethod1');
         $this->assertEquals(
             200,
             $response->getStatusCode(),
-            "Access granted to method defined in allowed_actions on extension, " .
+            "Access granted to method defined in allowed_actions on extension, " . "where method is also defined on extension, even when called in a subclass"
-            "where method is also defined on extension, even when called in a subclass"
         );
 
         $response = $this->get('AccessBaseController/extensionmethod2');
         $this->assertEquals(
             404,
             $response->getStatusCode(),
-            "Access denied to method not defined in allowed_actions on extension, " .
+            "Access denied to method not defined in allowed_actions on extension, " . "where method is also defined on extension"
-            "where method is also defined on extension"
         );
 
         $response = $this->get('IndexSecuredController/');
         $this->assertEquals(
             403,
             $response->getStatusCode(),
-            "Access denied when index action is limited through allowed_actions, " .
+            "Access denied when index action is limited through allowed_actions, " . "and doesn't satisfy checks, and action is empty"
-            "and doesn't satisfy checks, and action is empty"
         );
 
         $response = $this->get('IndexSecuredController/index');
         $this->assertEquals(
             403,
             $response->getStatusCode(),
-            "Access denied when index action is limited through allowed_actions, " .
+            "Access denied when index action is limited through allowed_actions, " . "and doesn't satisfy checks"
-            "and doesn't satisfy checks"
         );
 
         Security::setCurrentUser($adminUser);
@@ -286,8 +267,7 @@ class ControllerTest extends FunctionalTest
         $this->assertEquals(
             200,
             $response->getStatusCode(),
-            "Access granted when index action is limited through allowed_actions, " .
+            "Access granted when index action is limited through allowed_actions, " . "and does satisfy checks"
-            "and does satisfy checks"
         );
         Security::setCurrentUser(null);
     }
@@ -433,8 +413,7 @@ class ControllerTest extends FunctionalTest
 
         $this->assertFalse(
             $securedController->hasAction('protectedextensionmethod'),
-            'Method is not visible when defined on an extension, part of allowed_actions, ' .
+            'Method is not visible when defined on an extension, part of allowed_actions, ' . 'but with protected visibility'
-            'but with protected visibility'
         );
     }
 

tests/php/Control/HTTPResponseTest.php

@@ -47,4 +47,15 @@ class HTTPResponseTest extends SapphireTest
         // Fail if we get to here
         $this->assertFalse(true, 'Something went wrong with our test exception');
     }
+
+    public function testRemoveHeader()
+    {
+        $response = new HTTPResponse();
+
+        $response->addHeader('X-Animal', 'Monkey');
+        $this->assertSame('Monkey', $response->getHeader('X-Animal'));
+
+        $response->removeHeader('X-Animal');
+        $this->assertEmpty($response->getHeader('X-Animal'));
+    }
 }

tests/php/Control/HTTPTest.php

@@ -226,8 +226,7 @@ class HTTPTest extends FunctionalTest
                 // background-image
                 // Note that using /./ in urls is absolutely acceptable
                 $this->assertEquals(
-                    '<div style="background-image: url(\'http://www.silverstripe.org/./images/mybackground.gif\');">'.
+                    '<div style="background-image: url(\'http://www.silverstripe.org/./images/mybackground.gif\');">' . 'Content</div>',
-                    'Content</div>',
                     HTTP::absoluteURLs('<div style="background-image: url(\'./images/mybackground.gif\');">Content</div>')
                 );
 
@@ -290,8 +289,7 @@ class HTTPTest extends FunctionalTest
                 // background
                 // Note that using /./ in urls is absolutely acceptable
                 $this->assertEquals(
-                    '<div background="http://www.silverstripe.org/./themes/silverstripe/images/nav-bg-repeat-2.png">'.
+                    '<div background="http://www.silverstripe.org/./themes/silverstripe/images/nav-bg-repeat-2.png">' . 'SS Blog</div>',
-                    'SS Blog</div>',
                     HTTP::absoluteURLs('<div background="./themes/silverstripe/images/nav-bg-repeat-2.png">SS Blog</div>')
                 );
 
@@ -342,11 +340,9 @@ class HTTPTest extends FunctionalTest
 
                 // data uri
                 $this->assertEquals(
-                    '<img src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAUAAAAFCAYAAACNbyblAAAAHElEQVQI12P4//8/w38'.
+                    '<img src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAUAAAAFCAYAAACNbyblAAAAHElEQVQI12P4//8/w38' . 'GIAXDIBKE0DHxgljNBAAO9TXL0Y4OHwAAAABJRU5ErkJggg==" alt="Red dot" />',
-                    'GIAXDIBKE0DHxgljNBAAO9TXL0Y4OHwAAAABJRU5ErkJggg==" alt="Red dot" />',
                     HTTP::absoluteURLs(
-                        '<img src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAUAAAAFCAYAAACNbyblAAAAH'.
+                        '<img src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAUAAAAFCAYAAACNbyblAAAAH' . 'ElEQVQI12P4//8/w38GIAXDIBKE0DHxgljNBAAO9TXL0Y4OHwAAAABJRU5ErkJggg==" alt="Red dot" />'
-                        'ElEQVQI12P4//8/w38GIAXDIBKE0DHxgljNBAAO9TXL0Y4OHwAAAABJRU5ErkJggg==" alt="Red dot" />'
                     ),
                     'Data URI links are not rewritten'
                 );

tests/php/Core/ConvertTest.php

@@ -136,12 +136,7 @@ class ConvertTest extends SapphireTest
             "Single quotes are decoded correctly"
         );
 
-        $val8 = 'Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor '.
+        $val8 = 'Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor ' . 'incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud ' . 'exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute ' . 'irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla ' . 'pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia ' . 'deserunt mollit anim id est laborum.';
-          'incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud '.
-          'exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute '.
-          'irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla '.
-          'pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia '.
-          'deserunt mollit anim id est laborum.';
         $this->assertEquals($val8, Convert::html2raw($val8), 'Test long text is unwrapped');
         $this->assertEquals(
             <<<PHP

tests/php/Core/CoreTest.php

@@ -29,8 +29,7 @@ class CoreTest extends SapphireTest
             $this->assertEquals(TempFolder::getTempFolder(BASE_PATH), $this->tempPath . DIRECTORY_SEPARATOR . $user);
         } else {
             $user = TempFolder::getTempFolderUsername();
-            $base = sys_get_temp_dir() . DIRECTORY_SEPARATOR . 'silverstripe-cache-php' .
+            $base = sys_get_temp_dir() . DIRECTORY_SEPARATOR . 'silverstripe-cache-php' . preg_replace('/[^\w-\.+]+/', '-', PHP_VERSION);
-                preg_replace('/[^\w-\.+]+/', '-', PHP_VERSION);
 
             // A typical Windows location for where sites are stored on IIS
             $this->assertEquals(
@@ -56,8 +55,7 @@ class CoreTest extends SapphireTest
     {
         parent::tearDown();
         $user = TempFolder::getTempFolderUsername();
-        $base = sys_get_temp_dir() . DIRECTORY_SEPARATOR . 'silverstripe-cache-php' .
+        $base = sys_get_temp_dir() . DIRECTORY_SEPARATOR . 'silverstripe-cache-php' . preg_replace('/[^\w-\.+]+/', '-', PHP_VERSION);
-            preg_replace('/[^\w-\.+]+/', '-', PHP_VERSION);
         foreach (array(
             'C--inetpub-wwwroot-silverstripe-test-project',
             '-Users-joebloggs-Sites-silverstripe-test-project',

tests/php/Dev/FixtureBlueprintTest.php

@@ -109,8 +109,7 @@ class FixtureBlueprintTest extends SapphireTest
             'one',
             array(
                 'ManyManyRelation' =>
-                    '=>'.DataObjectRelation::class.'.relation1,' .
+                    '=>' . DataObjectRelation::class . '.relation1,' . '=>' . DataObjectRelation::class . '.relation2'
-                    '=>'.DataObjectRelation::class.'.relation2'
             ),
             array(
                 DataObjectRelation::class => array(

tests/php/Dev/SapphireTestTest.php

@@ -16,25 +16,38 @@ class SapphireTestTest extends SapphireTest
     public function provideResolveFixturePath()
     {
         return [
-            [__DIR__ . '/CsvBulkLoaderTest.yml', './CsvBulkLoaderTest.yml'],
+            'sameDirectory' => [
-            //same dir
+                __DIR__ . '/CsvBulkLoaderTest.yml',
-            [__DIR__ . '/CsvBulkLoaderTest.yml', 'CsvBulkLoaderTest.yml'],
+                './CsvBulkLoaderTest.yml',
-            // Filename only
+                'Could not resolve fixture path relative from same directory',
-            [dirname(__DIR__) . '/ORM/DataObjectTest.yml', '../ORM/DataObjectTest.yml'],
+            ],
-            // Parent path
+            'filenameOnly' => [
-            [dirname(__DIR__) . '/ORM/DataObjectTest.yml', dirname(__DIR__) . '/ORM/DataObjectTest.yml'],
+                __DIR__ . '/CsvBulkLoaderTest.yml',
-            // Absolute path
+                'CsvBulkLoaderTest.yml',
+                'Could not resolve fixture path from filename only',
+            ],
+            'parentPath' => [
+                dirname(__DIR__) . '/ORM/DataObjectTest.yml',
+                '../ORM/DataObjectTest.yml',
+                'Could not resolve fixture path from parent path',
+            ],
+            'absolutePath' => [
+                dirname(__DIR__) . '/ORM/DataObjectTest.yml',
+                dirname(__DIR__) . '/ORM/DataObjectTest.yml',
+                'Could not relsolve fixture path from absolute path',
+            ],
         ];
     }
 
     /**
      * @dataProvider provideResolveFixturePath
      */
-    public function testResolveFixturePath($expected, $path)
+    public function testResolveFixturePath($expected, $path, $message)
     {
         $this->assertEquals(
             $expected,
-            $this->resolveFixturePath($path)
+            $this->resolveFixturePath($path),
+            $message
         );
     }
 
@@ -46,10 +59,10 @@ class SapphireTestTest extends SapphireTest
         $this->logOut();
         $this->assertFalse(Permission::check('ADMIN'));
         $this->actWithPermission('ADMIN', function () {
-            $this->assertTrue(Permission::check('ADMIN'));
+            $this->assertTrue(Permission::check('ADMIN'), 'Member should now have ADMIN role');
             // check nested actAs calls work as expected
             Member::actAs(null, function () {
-                $this->assertFalse(Permission::check('ADMIN'));
+                $this->assertFalse(Permission::check('ADMIN'), 'Member should not act as ADMIN any more after reset');
             });
         });
     }
@@ -59,9 +72,16 @@ class SapphireTestTest extends SapphireTest
      */
     public function testCreateMemberWithPermission()
     {
-        $this->assertCount(0, Member::get()->filter(['Email' => 'TESTPERM@example.org']));
+        $this->assertEmpty(
+            Member::get()->filter(['Email' => 'TESTPERM@example.org']),
+            'DB should not have the test member created when the test starts'
+        );
         $this->createMemberWithPermission('TESTPERM');
-        $this->assertCount(1, Member::get()->filter(['Email' => 'TESTPERM@example.org']));
+        $this->assertCount(
+            1,
+            Member::get()->filter(['Email' => 'TESTPERM@example.org']),
+            'Database should now contain the test member'
+        );
     }
 
     /**
@@ -69,13 +89,30 @@ class SapphireTestTest extends SapphireTest
      *
      * @param $match
      * @param $itemsForList
+     *
      * @testdox Has assertion assertListAllMatch
      */
-    public function testAssertListAllMatch($match, $itemsForList)
+    public function testAssertListAllMatch($match, $itemsForList, $message)
     {
         $list = $this->generateArrayListFromItems($itemsForList);
 
-        $this->assertListAllMatch($match, $list);
+        $this->assertListAllMatch($match, $list, $message);
+    }
+
+    /**
+     * generate SS_List as this is not possible in dataProvider
+     *
+     * @param array $itemsForList
+     *
+     * @return ArrayList
+     */
+    private function generateArrayListFromItems($itemsForList)
+    {
+        $list = ArrayList::create();
+        foreach ($itemsForList as $data) {
+            $list->push(Member::create($data));
+        }
+        return $list;
     }
 
     /**
@@ -100,6 +137,7 @@ class SapphireTestTest extends SapphireTest
      *
      * @param $matches
      * @param $itemsForList
+     *
      * @testdox Has assertion assertListContains
      */
     public function testAssertListContains($matches, $itemsForList)
@@ -109,7 +147,7 @@ class SapphireTestTest extends SapphireTest
         $list->push(Member::create(['FirstName' => 'Bar', 'Surname' => 'Bar']));
         $list->push(Member::create(['FirstName' => 'Baz', 'Surname' => 'Baz']));
 
-        $this->assertListContains($matches, $list);
+        $this->assertListContains($matches, $list, 'The list does not contain the expected items');
     }
 
     /**
@@ -143,7 +181,7 @@ class SapphireTestTest extends SapphireTest
     {
         $list = $this->generateArrayListFromItems($itemsForList);
 
-        $this->assertListNotContains($matches, $list);
+        $this->assertListNotContains($matches, $list, 'List contains forbidden items');
     }
 
     /**
@@ -151,6 +189,7 @@ class SapphireTestTest extends SapphireTest
      *
      * @param $matches
      * @param $itemsForList
+     *
      * @testdox assertion assertListNotContains throws a exception when a matching item is found in the list
      *
      * @expectedException \PHPUnit_Framework_ExpectationFailedException
@@ -165,7 +204,6 @@ class SapphireTestTest extends SapphireTest
         $this->assertListNotContains($matches, $list);
     }
 
-
     /**
      * @dataProvider \SilverStripe\Dev\Tests\SapphireTestTest\DataProvider::provideEqualListsWithEmptyList()
      * @testdox Has assertion assertListEquals
@@ -177,7 +215,7 @@ class SapphireTestTest extends SapphireTest
     {
         $list = $this->generateArrayListFromItems($itemsForList);
 
-        $this->assertListEquals($matches, $list);
+        $this->assertListEquals($matches, $list, 'Lists do not equal');
     }
 
     /**
@@ -195,19 +233,4 @@ class SapphireTestTest extends SapphireTest
 
         $this->assertListEquals($matches, $list);
     }
-
-    /**
-     * generate SS_List as this is not possible in dataProvider
-     *
-     * @param $itemsForList array
-     * @return ArrayList
-     */
-    private function generateArrayListFromItems($itemsForList)
-    {
-        $list = ArrayList::create();
-        foreach ($itemsForList as $data) {
-            $list->push(Member::create($data));
-        }
-        return $list;
-    }
 }

tests/php/Dev/SapphireTestTest/DataProvider.php

@@ -7,12 +7,17 @@ use SilverStripe\Dev\TestOnly;
 class DataProvider implements TestOnly
 {
     protected static $oneItemList = [
-        ['FirstName' => 'Ingo', 'Surname' => 'Schommer']
+        ['FirstName' => 'Ingo', 'Surname' => 'Schommer'],
     ];
 
     protected static $twoItemList = [
         ['FirstName' => 'Ingo', 'Surname' => 'Schommer'],
-        ['FirstName' => 'Sam', 'Surname' => 'Minnee']
+        ['FirstName' => 'Sam', 'Surname' => 'Minnee'],
+    ];
+
+    protected static $memberList = [
+        ['FirstName' => 'Ingo', 'Surname' => 'Schommer', 'Locale' => 'en_US'],
+        ['FirstName' => 'Sam', 'Surname' => 'Minnee', 'Locale' => 'en_US'],
     ];
 
     /**
@@ -21,11 +26,11 @@ class DataProvider implements TestOnly
     public static function provideEqualListsWithEmptyList()
     {
         return array_merge(
-            [ //empty list
             [
+                'emptyLists' => [
                     [],
-                    []
+                    [],
-                ]
+                ],
             ],
             self::provideEqualLists()
         );
@@ -38,37 +43,37 @@ class DataProvider implements TestOnly
     {
         return [
             [
-                [ //one param
+                'oneParameterOneItem' => [
-                    ['FirstName' => 'Ingo']
-                ],
-                self::$oneItemList
-            ],
-            [
-                [ //two params
-                    ['FirstName' => 'Ingo', 'Surname' => 'Schommer']
-                ],
-                self::$oneItemList
-            ],
-            [ //only one param
-                [
                     ['FirstName' => 'Ingo'],
-                    ['FirstName' => 'Sam']
                 ],
-                self::$twoItemList
+                self::$oneItemList,
             ],
             [
-                [ //two params
+                'twoParametersOneItem' => [
                     ['FirstName' => 'Ingo', 'Surname' => 'Schommer'],
-                    ['FirstName' => 'Sam', 'Surname' => 'Minnee']
                 ],
-                self::$twoItemList
+                self::$oneItemList,
             ],
             [
-                [ //mixed
+                'oneParameterTwoItems' => [
-                    ['FirstName' => 'Ingo', 'Surname' => 'Schommer'],
+                    ['FirstName' => 'Ingo'],
-                    ['FirstName' => 'Sam']
+                    ['FirstName' => 'Sam'],
                 ],
-                self::$twoItemList
+                self::$twoItemList,
+            ],
+            [
+                'twoParametersTwoItems' => [
+                    ['FirstName' => 'Ingo', 'Surname' => 'Schommer'],
+                    ['FirstName' => 'Sam', 'Surname' => 'Minnee'],
+                ],
+                self::$twoItemList,
+            ],
+            [
+                'mixedParametersTwoItems' => [
+                    ['FirstName' => 'Ingo', 'Surname' => 'Schommer'],
+                    ['FirstName' => 'Sam'],
+                ],
+                self::$twoItemList,
             ],
         ];
     }
@@ -80,40 +85,38 @@ class DataProvider implements TestOnly
     {
 
         return [
-            [ //empty list
             [
-                    ['FirstName' => 'Ingo']
+                'checkAgainstEmptyList' => [
+                    ['FirstName' => 'Ingo'],
                 ],
-                []
+                [],
             ],
             [
-                [ //one item expected
+                'oneItemExpectedListContainsMore' => [
-                    ['FirstName' => 'Ingo']
+                    ['FirstName' => 'Ingo'],
-                ]
+                ],
-                ,
+                self::$twoItemList,
-                self::$twoItemList
             ],
-            [ //one item with wrong param
             [
+                'oneExpectationHasWrontParamter' => [
                     ['FirstName' => 'IngoXX'],
-                    ['FirstName' => 'Sam']
+                    ['FirstName' => 'Sam'],
-                ]
+                ],
-                ,
+                self::$twoItemList,
-                self::$twoItemList
             ],
             [
-                [ //two params wrong
+                'differentParametersInDifferentItemsAreWrong' => [
                     ['FirstName' => 'IngoXXX', 'Surname' => 'Schommer'],
-                    ['FirstName' => 'Sam', 'Surname' => 'MinneeXXX']
+                    ['FirstName' => 'Sam', 'Surname' => 'MinneeXXX'],
                 ],
-                self::$twoItemList
+                self::$twoItemList,
             ],
             [
-                [ //mixed
+                'differentParametersNotMatching' => [
                     ['FirstName' => 'Daniel', 'Surname' => 'Foo'],
-                    ['FirstName' => 'Dan']
+                    ['FirstName' => 'Dan'],
                 ],
-                self::$twoItemList
+                self::$twoItemList,
             ],
         ];
     }
@@ -124,32 +127,31 @@ class DataProvider implements TestOnly
     public static function provideNotContainingList()
     {
         return [
-            [ //empty list
+            'listIsEmpty' => [
                 [
-                    ['FirstName' => 'Ingo']
+                    ['FirstName' => 'Ingo'],
                 ],
-                []
+                [],
             ],
+            'oneItemIsExpected' => [
                 [
-                [ //one item expected
+                    ['FirstName' => 'Sam'],
-                    ['FirstName' => 'Sam']
-                ]
-                ,
-                self::$oneItemList
                 ],
+                self::$oneItemList,
+            ],
+            'twoParametersAreWrong' => [
                 [
-                [ //two params wrong
                     ['FirstName' => 'IngoXXX', 'Surname' => 'Schommer'],
-                    ['FirstName' => 'Sam', 'Surname' => 'MinneeXXX']
+                    ['FirstName' => 'Sam', 'Surname' => 'MinneeXXX'],
                 ],
-                self::$twoItemList
+                self::$twoItemList,
             ],
+            'mixedList' => [
                 [
-                [ //mixed
                     ['FirstName' => 'Daniel', 'Surname' => 'Foo'],
-                    ['FirstName' => 'Dan']
+                    ['FirstName' => 'Dan'],
                 ],
-                self::$twoItemList
+                self::$twoItemList,
             ],
         ];
     }
@@ -159,14 +161,17 @@ class DataProvider implements TestOnly
      */
     public static function provideAllMatchingList()
     {
-        $list = [
-            ['FirstName' => 'Ingo', 'Surname' => 'Schommer', 'Locale' => 'en_US'],
-            ['FirstName' => 'Sam', 'Surname' => 'Minnee', 'Locale' => 'en_US']
-        ];
-
         return [
-            [[], $list], //empty match
+            'emptyMatch' => [
-            [['Locale' => 'en_US'], $list] //all items have this field set
+                [],
+                self::$memberList,
+                'empty list did not match',
+            ],
+            'allItemsWithLocaleSet' => [
+                ['Locale' => 'en_US'],
+                self::$memberList,
+                'list with Locale set in all items did not match',
+            ],
         ];
     }
 
@@ -175,13 +180,8 @@ class DataProvider implements TestOnly
      */
     public static function provideNotMatchingList()
     {
-        $list = [
-            ['FirstName' => 'Ingo', 'Surname' => 'Schommer', 'Locale' => 'en_US'],
-            ['FirstName' => 'Sam', 'Surname' => 'Minnee', 'Locale' => 'en_US']
-        ];
-
         return [
-            [['FirstName' => 'Ingo'], $list] //not all items have this field set
+            'notAllItemsHaveLocaleSet' => [['FirstName' => 'Ingo'], self::$memberList],
         ];
     }
 }

tests/php/Forms/GridField/GridFieldDeleteActionTest.php

@@ -95,8 +95,7 @@ class GridFieldDeleteActionTest extends SapphireTest
         $this->expectException(HTTPResponse_Exception::class);
         $this->expectExceptionMessage(_t(
             "SilverStripe\\Forms\\Form.CSRF_FAILED_MESSAGE",
-            "There seems to have been a technical problem. Please click the back button, ".
+            "There seems to have been a technical problem. Please click the back button, " . "refresh your browser, and try again."
-            "refresh your browser, and try again."
         ));
         $this->expectExceptionCode(400);
         $stateID = 'testGridStateActionField';

tests/php/Forms/GridField/GridFieldExportButtonTest.php

@@ -66,9 +66,7 @@ class GridFieldExportButtonTest extends SapphireTest
         $button->setExportColumns(['Name' => 'My Name']);
 
         $this->assertEquals(
-            '"My Name"'."\n".
+            '"My Name"' . "\n" . 'Test' . "\n" . 'Test2' . "\n",
-            'Test'."\n".
-            'Test2'."\n",
             $button->generateExportFileData($this->gridField)
         );
     }
@@ -101,9 +99,7 @@ class GridFieldExportButtonTest extends SapphireTest
         ]);
 
         $this->assertEquals(
-            'Name,City'."\n".
+            'Name,City' . "\n" . 'Test,"City city"' . "\n" . 'Test2,"Quoted ""City"" 2 city"' . "\n",
-            'Test,"City city"'."\n".
-            'Test2,"Quoted ""City"" 2 city"'."\n",
             $button->generateExportFileData($this->gridField)
         );
     }
@@ -117,9 +113,7 @@ class GridFieldExportButtonTest extends SapphireTest
         ]);
 
         $this->assertEquals(
-            'Name,strtolower'."\n".
+            'Name,strtolower' . "\n" . 'Test,City' . "\n" . 'Test2,"Quoted ""City"" 2"' . "\n",
-            'Test,City'."\n".
-            'Test2,"Quoted ""City"" 2"'."\n",
             $button->generateExportFileData($this->gridField)
         );
     }
@@ -134,8 +128,7 @@ class GridFieldExportButtonTest extends SapphireTest
         $button->setCsvHasHeader(false);
 
         $this->assertEquals(
-            'Test,City'."\n".
+            'Test,City' . "\n" . 'Test2,"Quoted ""City"" 2"' . "\n",
-            'Test2,"Quoted ""City"" 2"'."\n",
             $button->generateExportFileData($this->gridField)
         );
     }
@@ -154,23 +147,7 @@ class GridFieldExportButtonTest extends SapphireTest
         $this->gridField->setList($arrayList);
 
         $this->assertEquals(
-            "ID\n".
+            "ID\n" . "1\n" . "2\n" . "3\n" . "4\n" . "5\n" . "6\n" . "7\n" . "8\n" . "9\n" . "10\n" . "11\n" . "12\n" . "13\n" . "14\n" . "15\n" . "16\n",
-            "1\n".
-            "2\n".
-            "3\n".
-            "4\n".
-            "5\n".
-            "6\n".
-            "7\n".
-            "8\n".
-            "9\n".
-            "10\n".
-            "11\n".
-            "12\n".
-            "13\n".
-            "14\n".
-            "15\n".
-            "16\n",
             $button->generateExportFileData($this->gridField)
         );
     }

tests/php/Forms/TreeDropdownFieldTest.php

@@ -111,8 +111,7 @@ class TreeDropdownFieldTest extends SapphireTest
         $folder1Subfolder1 = $this->objFromFixture(Folder::class, 'folder1-subfolder1');
 
         $parser = new CSSContentParser($tree);
-        $cssPath = 'ul.tree li#selector-TestTree-'.$folder1->ID.' li#selector-TestTree-'.
+        $cssPath = 'ul.tree li#selector-TestTree-' . $folder1->ID . ' li#selector-TestTree-' . $folder1Subfolder1->ID . ' a span.item';
-            $folder1Subfolder1->ID.' a span.item';
         $firstResult = $parser->getBySelector($cssPath);
         $this->assertEquals(
             $folder1Subfolder1->Name,
@@ -149,8 +148,7 @@ class TreeDropdownFieldTest extends SapphireTest
         $parser = new CSSContentParser($tree);
 
         // Even if we used File as the source object, folders are still returned because Folder is a File
-        $cssPath = 'ul.tree li#selector-TestTree-'.$folder1->ID.' li#selector-TestTree-'.
+        $cssPath = 'ul.tree li#selector-TestTree-' . $folder1->ID . ' li#selector-TestTree-' . $folder1Subfolder1->ID . ' a span.item';
-            $folder1Subfolder1->ID.' a span.item';
         $firstResult = $parser->getBySelector($cssPath);
         $this->assertEquals(
             $folder1Subfolder1->Name,

tests/php/ORM/DataObjectLazyLoadingTest.php

@@ -28,14 +28,7 @@ class DataObjectLazyLoadingTest extends SapphireTest
         $db = DB::get_conn();
         $playerList = new DataList(SubTeam::class);
         $playerList = $playerList->setQueriedColumns(array('ID'));
-        $expected = 'SELECT DISTINCT "DataObjectTest_Team"."ClassName", "DataObjectTest_Team"."LastEdited", ' .
+        $expected = 'SELECT DISTINCT "DataObjectTest_Team"."ClassName", "DataObjectTest_Team"."LastEdited", ' . '"DataObjectTest_Team"."Created", "DataObjectTest_Team"."ID", CASE WHEN ' . '"DataObjectTest_Team"."ClassName" IS NOT NULL THEN "DataObjectTest_Team"."ClassName" ELSE ' . $db->quoteString(Team::class) . ' END AS "RecordClassName", "DataObjectTest_Team"."Title" ' . 'FROM "DataObjectTest_Team" ' . 'LEFT JOIN "DataObjectTest_SubTeam" ON "DataObjectTest_SubTeam"."ID" = "DataObjectTest_Team"."ID" ' . 'WHERE ("DataObjectTest_Team"."ClassName" IN (?))' . ' ORDER BY "DataObjectTest_Team"."Title" ASC';
-            '"DataObjectTest_Team"."Created", "DataObjectTest_Team"."ID", CASE WHEN '.
-            '"DataObjectTest_Team"."ClassName" IS NOT NULL THEN "DataObjectTest_Team"."ClassName" ELSE ' .
-            $db->quoteString(Team::class).' END AS "RecordClassName", "DataObjectTest_Team"."Title" '.
-            'FROM "DataObjectTest_Team" ' .
-            'LEFT JOIN "DataObjectTest_SubTeam" ON "DataObjectTest_SubTeam"."ID" = "DataObjectTest_Team"."ID" ' .
-            'WHERE ("DataObjectTest_Team"."ClassName" IN (?))' .
-            ' ORDER BY "DataObjectTest_Team"."Title" ASC';
         $this->assertSQLEquals($expected, $playerList->sql($parameters));
     }
 
@@ -44,14 +37,7 @@ class DataObjectLazyLoadingTest extends SapphireTest
         $db = DB::get_conn();
         $playerList = new DataList(SubTeam::class);
         $playerList = $playerList->setQueriedColumns(array('Title', 'SubclassDatabaseField'));
-        $expected = 'SELECT DISTINCT "DataObjectTest_Team"."ClassName", "DataObjectTest_Team"."LastEdited", ' .
+        $expected = 'SELECT DISTINCT "DataObjectTest_Team"."ClassName", "DataObjectTest_Team"."LastEdited", ' . '"DataObjectTest_Team"."Created", "DataObjectTest_Team"."Title", ' . '"DataObjectTest_SubTeam"."SubclassDatabaseField", "DataObjectTest_Team"."ID", CASE WHEN ' . '"DataObjectTest_Team"."ClassName" IS NOT NULL THEN "DataObjectTest_Team"."ClassName" ELSE ' . $db->quoteString(Team::class) . ' END AS "RecordClassName" FROM "DataObjectTest_Team" ' . 'LEFT JOIN "DataObjectTest_SubTeam" ON "DataObjectTest_SubTeam"."ID" = "DataObjectTest_Team"."ID" WHERE ' . '("DataObjectTest_Team"."ClassName" IN (?)) ' . 'ORDER BY "DataObjectTest_Team"."Title" ASC';
-            '"DataObjectTest_Team"."Created", "DataObjectTest_Team"."Title", ' .
-            '"DataObjectTest_SubTeam"."SubclassDatabaseField", "DataObjectTest_Team"."ID", CASE WHEN ' .
-            '"DataObjectTest_Team"."ClassName" IS NOT NULL THEN "DataObjectTest_Team"."ClassName" ELSE ' .
-            $db->quoteString(Team::class).' END AS "RecordClassName" FROM "DataObjectTest_Team" ' .
-            'LEFT JOIN "DataObjectTest_SubTeam" ON "DataObjectTest_SubTeam"."ID" = "DataObjectTest_Team"."ID" WHERE ' .
-            '("DataObjectTest_Team"."ClassName" IN (?)) ' .
-            'ORDER BY "DataObjectTest_Team"."Title" ASC';
         $this->assertSQLEquals($expected, $playerList->sql($parameters));
     }
 
@@ -60,13 +46,7 @@ class DataObjectLazyLoadingTest extends SapphireTest
         $db = DB::get_conn();
         $playerList = new DataList(SubTeam::class);
         $playerList = $playerList->setQueriedColumns(array('Title'));
-        $expected = 'SELECT DISTINCT "DataObjectTest_Team"."ClassName", "DataObjectTest_Team"."LastEdited", ' .
+        $expected = 'SELECT DISTINCT "DataObjectTest_Team"."ClassName", "DataObjectTest_Team"."LastEdited", ' . '"DataObjectTest_Team"."Created", "DataObjectTest_Team"."Title", "DataObjectTest_Team"."ID", ' . 'CASE WHEN "DataObjectTest_Team"."ClassName" IS NOT NULL THEN "DataObjectTest_Team"."ClassName" ELSE ' . $db->quoteString(Team::class) . ' END AS "RecordClassName" FROM "DataObjectTest_Team" ' . 'LEFT JOIN "DataObjectTest_SubTeam" ON "DataObjectTest_SubTeam"."ID" = "DataObjectTest_Team"."ID" WHERE ' . '("DataObjectTest_Team"."ClassName" IN (?)) ' . 'ORDER BY "DataObjectTest_Team"."Title" ASC';
-            '"DataObjectTest_Team"."Created", "DataObjectTest_Team"."Title", "DataObjectTest_Team"."ID", ' .
-            'CASE WHEN "DataObjectTest_Team"."ClassName" IS NOT NULL THEN "DataObjectTest_Team"."ClassName" ELSE ' .
-            $db->quoteString(Team::class).' END AS "RecordClassName" FROM "DataObjectTest_Team" ' .
-            'LEFT JOIN "DataObjectTest_SubTeam" ON "DataObjectTest_SubTeam"."ID" = "DataObjectTest_Team"."ID" WHERE ' .
-            '("DataObjectTest_Team"."ClassName" IN (?)) ' .
-            'ORDER BY "DataObjectTest_Team"."Title" ASC';
         $this->assertSQLEquals($expected, $playerList->sql($parameters));
     }
 
@@ -75,14 +55,7 @@ class DataObjectLazyLoadingTest extends SapphireTest
         $db = DB::get_conn();
         $playerList = new DataList(SubTeam::class);
         $playerList = $playerList->setQueriedColumns(array('SubclassDatabaseField'));
-        $expected = 'SELECT DISTINCT "DataObjectTest_Team"."ClassName", "DataObjectTest_Team"."LastEdited", ' .
+        $expected = 'SELECT DISTINCT "DataObjectTest_Team"."ClassName", "DataObjectTest_Team"."LastEdited", ' . '"DataObjectTest_Team"."Created", "DataObjectTest_SubTeam"."SubclassDatabaseField", ' . '"DataObjectTest_Team"."ID", CASE WHEN "DataObjectTest_Team"."ClassName" IS NOT NULL THEN ' . '"DataObjectTest_Team"."ClassName" ELSE ' . $db->quoteString(Team::class) . ' END ' . 'AS "RecordClassName", "DataObjectTest_Team"."Title" ' . 'FROM "DataObjectTest_Team" LEFT JOIN "DataObjectTest_SubTeam" ON "DataObjectTest_SubTeam"."ID" = ' . '"DataObjectTest_Team"."ID" WHERE ("DataObjectTest_Team"."ClassName" IN (?)) ' . 'ORDER BY "DataObjectTest_Team"."Title" ASC';
-            '"DataObjectTest_Team"."Created", "DataObjectTest_SubTeam"."SubclassDatabaseField", ' .
-            '"DataObjectTest_Team"."ID", CASE WHEN "DataObjectTest_Team"."ClassName" IS NOT NULL THEN ' .
-            '"DataObjectTest_Team"."ClassName" ELSE '.$db->quoteString(Team::class).' END ' .
-            'AS "RecordClassName", "DataObjectTest_Team"."Title" ' .
-            'FROM "DataObjectTest_Team" LEFT JOIN "DataObjectTest_SubTeam" ON "DataObjectTest_SubTeam"."ID" = ' .
-            '"DataObjectTest_Team"."ID" WHERE ("DataObjectTest_Team"."ClassName" IN (?)) ' .
-            'ORDER BY "DataObjectTest_Team"."Title" ASC';
         $this->assertSQLEquals($expected, $playerList->sql($parameters));
     }
 

tests/php/ORM/DataObjectTest.php

@@ -156,8 +156,7 @@ class DataObjectTest extends SapphireTest
             $helper = $obj->dbObject($field);
             $this->assertTrue(
                 ($helper instanceof DBField),
-                "for {$field} expected helper to be DBField, but was " .
+                "for {$field} expected helper to be DBField, but was " . (is_object($helper) ? get_class($helper) : "null")
-                (is_object($helper) ? get_class($helper) : "null")
             );
         }
     }

tests/php/Security/SecurityTest.php

@@ -554,8 +554,7 @@ class SecurityTest extends FunctionalTest
         }
         $msg = _t(
             'SilverStripe\\Security\\Member.ERRORLOCKEDOUT2',
-            'Your account has been temporarily disabled because of too many failed attempts at ' .
+            'Your account has been temporarily disabled because of too many failed attempts at ' . 'logging in. Please try again in {count} minutes.',
-            'logging in. Please try again in {count} minutes.',
             null,
             array('count' => 15)
         );

tests/php/View/Parsers/ShortcodeParserTest.php

@@ -83,8 +83,7 @@ class ShortcodeParserTest extends SapphireTest
     {
         $tests = array(
             '[test_shortcode]',
-            '[test_shortcode ]', '[test_shortcode,]', '[test_shortcode, ]'.
+            '[test_shortcode ]', '[test_shortcode,]', '[test_shortcode, ]' . '[test_shortcode/]', '[test_shortcode /]', '[test_shortcode,/]', '[test_shortcode, /]'
-            '[test_shortcode/]', '[test_shortcode /]', '[test_shortcode,/]', '[test_shortcode, /]'
         );
 
         foreach ($tests as $test) {