From 0aeb2293bbc78422d6d546cffc6635bd8800185a Mon Sep 17 00:00:00 2001 From: Cam Spiers Date: Mon, 1 Jul 2013 14:51:32 +1200 Subject: [PATCH] Allow module directories to be named with more valid characters ensuring that module names in fragment meta-data are correct. Unit tests for ConfigManifest reference path parsing --- core/manifest/ConfigManifest.php | 2 +- tests/core/manifest/ConfigManifestTest.php | 158 ++++++++++++++++++ .../mysite/_config/addyamlconfigfile.yml | 59 +++++++ 3 files changed, 218 insertions(+), 1 deletion(-) create mode 100644 tests/core/manifest/fixtures/configmanifest/mysite/_config/addyamlconfigfile.yml diff --git a/core/manifest/ConfigManifest.php b/core/manifest/ConfigManifest.php index 1398518aa..7b58455c9 100644 --- a/core/manifest/ConfigManifest.php +++ b/core/manifest/ConfigManifest.php @@ -291,7 +291,7 @@ class SS_ConfigManifest { // For each, parse out into module/file#name, and set any missing to "*" $header[$order] = array(); foreach($orderparts as $part) { - preg_match('! (?P\*|\w+)? (\/ (?P\*|\w+))? (\# (?P\*|\w+))? !x', + preg_match('! (?P\*|[^\/#]+)? (\/ (?P\*|\w+))? (\# (?P\*|\w+))? !x', $part, $match); $header[$order][] = array( diff --git a/tests/core/manifest/ConfigManifestTest.php b/tests/core/manifest/ConfigManifestTest.php index 27a2da60f..5f0895ef0 100644 --- a/tests/core/manifest/ConfigManifestTest.php +++ b/tests/core/manifest/ConfigManifestTest.php @@ -13,6 +13,164 @@ class ConfigManifestTest extends SapphireTest { return $manifest->get('ConfigManifestTest', $name); } + /** + * This is a helper method for displaying a relevant message about a parsing failure + */ + protected function getParsedAsMessage($path) { + return sprintf('Reference path "%s" failed to parse correctly', $path); + } + + /** + * This test checks the processing of before and after reference paths (module-name/filename#fragment) + * This method uses fixture/configmanifest/mysite/_config/addyamlconfigfile.yml as a fixture + */ + public function testAddYAMLConfigFileReferencePathParsing() { + // Use a mock to avoid testing unrelated functionality + $manifest = $this->getMockBuilder('SS_ConfigManifest') + ->disableOriginalConstructor() + ->setMethods(array('addModule')) + ->getMock(); + + // This tests that the addModule method is called with the correct value + $manifest->expects($this->once()) + ->method('addModule') + ->with($this->equalTo(dirname(__FILE__).'/fixtures/configmanifest/mysite')); + + // Call the method to be tested + $manifest->addYAMLConfigFile( + 'addyamlconfigfile.yml', + dirname(__FILE__).'/fixtures/configmanifest/mysite/_config/addyamlconfigfile.yml', + false + ); + + // There is no getter for yamlConfigFragments + $property = new ReflectionProperty('SS_ConfigManifest', 'yamlConfigFragments'); + $property->setAccessible(true); + + // Get the result back from the parsing + $result = $property->getValue($manifest); + + $this->assertEquals( + array( + array( + 'module' => 'mysite', + 'file' => 'testfile', + 'name' => 'fragment', + ), + ), + @$result[0]['after'], + $this->getParsedAsMessage('mysite/testfile#fragment') + ); + + $this->assertEquals( + array( + array( + 'module' => 'test-module', + 'file' => 'testfile', + 'name' => 'fragment', + ), + ), + @$result[1]['after'], + $this->getParsedAsMessage('test-module/testfile#fragment') + ); + + $this->assertEquals( + array( + array( + 'module' => '*', + 'file' => '*', + 'name' => '*', + ), + ), + @$result[2]['after'], + $this->getParsedAsMessage('*') + ); + + $this->assertEquals( + array( + array( + 'module' => '*', + 'file' => 'testfile', + 'name' => '*' + ), + ), + @$result[3]['after'], + $this->getParsedAsMessage('*/testfile') + ); + + $this->assertEquals( + array( + array( + 'module' => '*', + 'file' => '*', + 'name' => 'fragment' + ), + ), + @$result[4]['after'], + $this->getParsedAsMessage('*/*#fragment') + ); + + $this->assertEquals( + array( + array( + 'module' => '*', + 'file' => '*', + 'name' => 'fragment' + ), + ), + @$result[5]['after'], + $this->getParsedAsMessage('#fragment') + ); + + $this->assertEquals( + array( + array( + 'module' => 'test-module', + 'file' => '*', + 'name' => 'fragment' + ), + ), + @$result[6]['after'], + $this->getParsedAsMessage('test-module#fragment') + ); + + $this->assertEquals( + array( + array( + 'module' => 'test-module', + 'file' => '*', + 'name' => '*' + ), + ), + @$result[7]['after'], + $this->getParsedAsMessage('test-module') + ); + + $this->assertEquals( + array( + array( + 'module' => 'test-module', + 'file' => '*', + 'name' => '*' + ), + ), + @$result[8]['after'], + $this->getParsedAsMessage('test-module/*') + ); + + $this->assertEquals( + array( + array( + 'module' => 'test-module', + 'file' => '*', + 'name' => '*' + ), + ), + @$result[9]['after'], + $this->getParsedAsMessage('test-module/*/#*') + ); + } + public function testClassRules() { $config = $this->getConfigFixtureValue('Class'); diff --git a/tests/core/manifest/fixtures/configmanifest/mysite/_config/addyamlconfigfile.yml b/tests/core/manifest/fixtures/configmanifest/mysite/_config/addyamlconfigfile.yml new file mode 100644 index 000000000..936a28d6d --- /dev/null +++ b/tests/core/manifest/fixtures/configmanifest/mysite/_config/addyamlconfigfile.yml @@ -0,0 +1,59 @@ +--- +After: 'mysite/testfile#fragment' +--- +ClassManifestTest: + testParam: false + +--- +After: 'test-module/testfile#fragment' +--- +ClassManifestTest: + testParam: false + +--- +After: '*' +--- +ClassManifestTest: + testParam: false + +--- +After: '*/testfile' +--- +ClassManifestTest: + testParam: false + +--- +After: '*/*#fragment' +--- +ClassManifestTest: + testParam: false + +--- +After: '#fragment' +--- +ClassManifestTest: + testParam: false + +--- +After: 'test-module#fragment' +--- +ClassManifestTest: + testParam: false + +--- +After: 'test-module' +--- +ClassManifestTest: + testParam: false + +--- +After: 'test-module/*' +--- +ClassManifestTest: + testParam: false + +--- +After: 'test-module/*#*' +--- +ClassManifestTest: + testParam: false \ No newline at end of file