From 3416c841a5dabed7f222f85d120bdbf40e60e6ab Mon Sep 17 00:00:00 2001 From: ant-workaholic Date: Mon, 3 Jul 2017 17:44:37 +0300 Subject: [PATCH 1/3] Fixed #92: Issue duplicated in description - Created parser unit test. - Fixed issue duplication issue. --- .../Filter/ShortCommitMsg/Parser/Jira.php | 3 + .../Test/Filter/ShortCommitMsg/ParserTest.php | 175 ++++++++++++++---- 2 files changed, 138 insertions(+), 40 deletions(-) diff --git a/src/lib/PreCommit/Filter/ShortCommitMsg/Parser/Jira.php b/src/lib/PreCommit/Filter/ShortCommitMsg/Parser/Jira.php index c6b21b9..2761cf5 100644 --- a/src/lib/PreCommit/Filter/ShortCommitMsg/Parser/Jira.php +++ b/src/lib/PreCommit/Filter/ShortCommitMsg/Parser/Jira.php @@ -291,6 +291,9 @@ protected function interpretShortMessage($message) $userMessage = null; if ($m) { $userMessage = trim(array_pop($m)); + if (is_numeric($userMessage)) { + $userMessage = ""; + } } return [$commitVerb, $issueKey, $userMessage]; diff --git a/src/tests/testsuite/PreCommit/Test/Filter/ShortCommitMsg/ParserTest.php b/src/tests/testsuite/PreCommit/Test/Filter/ShortCommitMsg/ParserTest.php index c07c627..ff11260 100644 --- a/src/tests/testsuite/PreCommit/Test/Filter/ShortCommitMsg/ParserTest.php +++ b/src/tests/testsuite/PreCommit/Test/Filter/ShortCommitMsg/ParserTest.php @@ -4,7 +4,7 @@ */ namespace testsuite\PreCommit\Test\Filter\ShortCommitMsg; -use PreCommit\Filter\ShortCommitMsg; +use PreCommit\Filter\Explode; use PreCommit\Issue; use PreCommit\Message; @@ -31,37 +31,65 @@ public function dataProvider() 'Test summary!!!', //issue summary 'task', //issue type 'Task', //original issue type - "My test 1!\nTest 2.", //user message - "123 My test 1!\nTest 2.", //full commit message - 'Implemented', //verb - '', //short verb + "R 123 My new test header \nTest 2.", //full commit message + 'TEST', // project key + 'jira', // tracker type + "Refactored TEST-123: Test summary!!!\n - My new test header\nTest 2." ], + /** - * Test getting verb by short verb + * Test getting default verb by issue type */ [ - 'TEST-123', //issue key + 'TEST-1', //issue key 'Test summary!!!', //issue summary 'task', //issue type 'Task', //original issue type - "My test 1!\nTest 2.", //user message - "F 123 My test 1!\nTest 2.", //full commit message - 'Fixed', //verb - 'F', //short verb + "I 1 My new test header \nTest 2.", //full commit message + 'TEST', //project key + 'jira', //tracker type + "Implemented TEST-1: Test summary!!!\n - My new test header\nTest 2." ], + /** - * Test working with full issue key + * Test getting default verb by issue type */ [ - 'TEST-123', //issue key + 'TEST-1', // issue key + 'Test summary!!!', // issue summary + 'task', // issue type + 'Task', // original issue type + "1 \nTest 2.", // full commit message + 'TEST', // project key + 'jira', // tracker type + "Implemented TEST-1: Test summary!!!\n - Test 2." + ], + /** + * Test getting default verb by issue type + */ + [ + 'TEST-12', // issue key + 'Test summary!!!', // issue summary + 'task', // issue type + 'Task', // original issue type + "12 Test 1 \nTest 2.", // full commit message + 'TEST', // project key + 'jira', // tracker type + "Implemented TEST-12: Test summary!!!\n - Test 1\nTest 2." + ], + /** + * Test getting default verb by issue type + */ + [ + 'TEST-22', //issue key 'Test summary!!!', //issue summary 'task', //issue type 'Task', //original issue type - "My test 1!\nTest 2.", //user message - "F TEST-123 My test 1!\nTest 2.", //full commit message - 'Fixed', //verb - 'F', //short verb - ], + "C 22 My new test header \nTest 2.", //full commit message + 'TEST', // project key + 'jira', // tracker type + "CR Changes TEST-22: Test summary!!!\n - My new test header\nTest 2." + ] ]; } @@ -72,10 +100,10 @@ public function dataProvider() * @param string $summary * @param string $type * @param string $originalType - * @param string $userBody * @param string $commitMessage - * @param string $verb - * @param string $shortVerb + * @param string $projectKey + * @param string $trackerType + * @param string $resultValue * @dataProvider dataProvider */ public function testFilterShortMessageWithoutVerb( @@ -83,34 +111,98 @@ public function testFilterShortMessageWithoutVerb( $summary, $type, $originalType, - $userBody, $commitMessage, - $verb, - $shortVerb + $projectKey, + $trackerType, + $resultValue ) { - $this->markTestIncomplete(); + // Real message. + $message = new Message(); + + // Explode filter + $explodeFilter = new Explode(); - $message = new Message(); + // Prepare a commit message $message->body = $commitMessage; + // Filter commit message. + $message = $explodeFilter->filter($message); $issue = $this->getIssueMock($summary, $issueKey, $type, $originalType); - /** @var ShortCommitMsg\Parser|\PHPUnit_Framework_MockObject_MockObject $parser */ - $parser = $this->getMock('PreCommit\Filter\ShortCommitMsg\Parser', ['_initIssue', '_getIssue']); - $parser->method('_initIssue') - ->willReturnSelf(); - $parser->method('_getIssue') - ->willReturn($issue); + // Create a config object mock + $configMock = $this->getMockBuilder('PreCommit\Config') + ->getMock(); + + $map = [ + [ + "hooks/commit-msg/message/verb/list/", true, [ + "I" => "Implemented", + "R" => "Refactored", + "F" => "Fixed", + "C" => "CR Changes" + ] + ], + [ + "formatters/ShortCommitMsg/formatting/$trackerType", true, [ + "regular" => "~^__format__~", + "format" => "__verb__ __issue_key__: __summary__", + ], + ] + ]; + + $mapNode = [ + ["tracker/type", true, $trackerType], + ["tracker/jira/project", true, $projectKey], + ["tracker/jira/active_task", true, $issueKey], + ["filters/ShortCommitMsg/issue/default_type_verb/task", true, "I"], + ["filters/ShortCommitMsg/issue/default_type_verb/bug", true, "F"], + ]; + + // Configure the getNodeArray stub. + $configMock->method('getNodeArray') + ->will($this->returnValueMap($map)); + + // Configure the getNode stub. + $configMock->method('getNode') + ->will($this->returnValueMap($mapNode)); + + $reflector = new \ReflectionClass("\\PreCommit\\Filter\\ShortCommitMsg\\Parser\\Jira"); + + // A new parser object. Set a custom config mock to a config property. + + // Create a new instance without constructor. + $parser = $reflector->newInstanceWithoutConstructor(); + $configProperty = $reflector->getProperty("config"); + $configProperty->setAccessible(true); + $configProperty->setValue($parser, $configMock); + + // Specify an "issue" property. Add a custom mock object as an issue property. + $issueProperty = $reflector->getProperty("issue"); + $issueProperty->setAccessible(true); + $issueProperty->setValue($parser, $issue); + + // Interpret original commit message. $result = $parser->interpret($message); - $this->assertEquals($message, $result); - $this->assertEquals($issue, $message->issue); - $this->assertEquals($summary, $message->summary); - $this->assertEquals($issueKey, $message->issueKey); - $this->assertEquals($userBody, $message->userBody); - $this->assertEquals($verb, $message->verb); - $this->assertEquals($shortVerb, $message->shortVerb); + $reflector = new \ReflectionClass("\\PreCommit\\Filter\\ShortCommitMsg\\Formatter"); + + $formatter = $reflector->newInstanceWithoutConstructor(); + $configProperty = $reflector->getProperty("config"); + + // Get a formatter type property, set a tracker type for the Formatter. + $typeProperty = $reflector->getProperty("type"); + $typeProperty->setAccessible(true); + $typeProperty->setValue($formatter, $trackerType); + + // Get a formatter config property, set a config model for the Formatter. + $configProperty->setAccessible(true); + $configProperty->setValue($formatter, $configMock); + + $resultMessage = $formatter->filter($result); + + // Verify commit messages + $this->assertEquals($resultMessage->__toString(), $resultValue); } /** @@ -125,7 +217,10 @@ public function testFilterShortMessageWithoutVerb( protected function getIssueMock($summary, $key, $type, $originalType) { /** @var Issue\AbstractAdapter|\PHPUnit_Framework_MockObject_MockObject $filter */ - $issue = $this->getMock('PreCommit\Issue\AdapterAbstract', [], [], '', false); + $issue = $this->getMockBuilder('PreCommit\Issue\AdapterAbstract') + ->setMethods(['getSummary', 'getKey', 'getType', 'getOriginalType']) + ->getMock(); + $issue->method('getSummary') ->willReturn($summary); $issue->method('getKey') From 02141548d55d25467f2ed632b9c183bb384df85f Mon Sep 17 00:00:00 2001 From: ant-workaholic Date: Wed, 5 Jul 2017 17:20:27 +0300 Subject: [PATCH 2/3] Fixed #92: Issue duplicated in description. - Fixed tests. Created parser integration tests. - Refactored Jira class. Added verification for an issue initialization. --- .../Filter/ShortCommitMsg/Parser/Jira.php | 5 +- .../Test/Filter/ShortCommitMsg/ParserTest.php | 90 ++----------------- 2 files changed, 12 insertions(+), 83 deletions(-) diff --git a/src/lib/PreCommit/Filter/ShortCommitMsg/Parser/Jira.php b/src/lib/PreCommit/Filter/ShortCommitMsg/Parser/Jira.php index 2761cf5..750a4dd 100644 --- a/src/lib/PreCommit/Filter/ShortCommitMsg/Parser/Jira.php +++ b/src/lib/PreCommit/Filter/ShortCommitMsg/Parser/Jira.php @@ -337,8 +337,9 @@ protected function mergeComment($comment, $commentInline) */ protected function initIssue($issueKey) { - $this->issue = Issue::factory($issueKey); - + if (!$this->issue) { + $this->issue = Issue::factory($issueKey); + } return $this; } diff --git a/src/tests/testsuite/PreCommit/Test/Filter/ShortCommitMsg/ParserTest.php b/src/tests/testsuite/PreCommit/Test/Filter/ShortCommitMsg/ParserTest.php index ff11260..1920caf 100644 --- a/src/tests/testsuite/PreCommit/Test/Filter/ShortCommitMsg/ParserTest.php +++ b/src/tests/testsuite/PreCommit/Test/Filter/ShortCommitMsg/ParserTest.php @@ -5,6 +5,7 @@ namespace testsuite\PreCommit\Test\Filter\ShortCommitMsg; use PreCommit\Filter\Explode; +use PreCommit\Filter\ShortCommitMsg\Formatter; use PreCommit\Issue; use PreCommit\Message; @@ -32,8 +33,6 @@ public function dataProvider() 'task', //issue type 'Task', //original issue type "R 123 My new test header \nTest 2.", //full commit message - 'TEST', // project key - 'jira', // tracker type "Refactored TEST-123: Test summary!!!\n - My new test header\nTest 2." ], @@ -46,8 +45,6 @@ public function dataProvider() 'task', //issue type 'Task', //original issue type "I 1 My new test header \nTest 2.", //full commit message - 'TEST', //project key - 'jira', //tracker type "Implemented TEST-1: Test summary!!!\n - My new test header\nTest 2." ], @@ -60,8 +57,6 @@ public function dataProvider() 'task', // issue type 'Task', // original issue type "1 \nTest 2.", // full commit message - 'TEST', // project key - 'jira', // tracker type "Implemented TEST-1: Test summary!!!\n - Test 2." ], /** @@ -73,8 +68,6 @@ public function dataProvider() 'task', // issue type 'Task', // original issue type "12 Test 1 \nTest 2.", // full commit message - 'TEST', // project key - 'jira', // tracker type "Implemented TEST-12: Test summary!!!\n - Test 1\nTest 2." ], /** @@ -86,8 +79,6 @@ public function dataProvider() 'task', //issue type 'Task', //original issue type "C 22 My new test header \nTest 2.", //full commit message - 'TEST', // project key - 'jira', // tracker type "CR Changes TEST-22: Test summary!!!\n - My new test header\nTest 2." ] ]; @@ -101,8 +92,6 @@ public function dataProvider() * @param string $type * @param string $originalType * @param string $commitMessage - * @param string $projectKey - * @param string $trackerType * @param string $resultValue * @dataProvider dataProvider */ @@ -112,70 +101,25 @@ public function testFilterShortMessageWithoutVerb( $type, $originalType, $commitMessage, - $projectKey, - $trackerType, $resultValue ) { - // Real message. - $message = new Message(); - // Explode filter + $message = new Message(); $explodeFilter = new Explode(); + $formatter = new Formatter(); // Prepare a commit message $message->body = $commitMessage; - // Filter commit message. + // Filter commit message through the explode filter. $message = $explodeFilter->filter($message); $issue = $this->getIssueMock($summary, $issueKey, $type, $originalType); - - // Create a config object mock - $configMock = $this->getMockBuilder('PreCommit\Config') - ->getMock(); - - $map = [ - [ - "hooks/commit-msg/message/verb/list/", true, [ - "I" => "Implemented", - "R" => "Refactored", - "F" => "Fixed", - "C" => "CR Changes" - ] - ], - [ - "formatters/ShortCommitMsg/formatting/$trackerType", true, [ - "regular" => "~^__format__~", - "format" => "__verb__ __issue_key__: __summary__", - ], - ] - ]; - - $mapNode = [ - ["tracker/type", true, $trackerType], - ["tracker/jira/project", true, $projectKey], - ["tracker/jira/active_task", true, $issueKey], - ["filters/ShortCommitMsg/issue/default_type_verb/task", true, "I"], - ["filters/ShortCommitMsg/issue/default_type_verb/bug", true, "F"], - ]; - - // Configure the getNodeArray stub. - $configMock->method('getNodeArray') - ->will($this->returnValueMap($map)); - - // Configure the getNode stub. - $configMock->method('getNode') - ->will($this->returnValueMap($mapNode)); - + // Getting jira parser reflector. $reflector = new \ReflectionClass("\\PreCommit\\Filter\\ShortCommitMsg\\Parser\\Jira"); - // A new parser object. Set a custom config mock to a config property. - - // Create a new instance without constructor. - $parser = $reflector->newInstanceWithoutConstructor(); - $configProperty = $reflector->getProperty("config"); - $configProperty->setAccessible(true); - $configProperty->setValue($parser, $configMock); + // Create a new instance from reflector. + $parser = $reflector->newInstance(); // Specify an "issue" property. Add a custom mock object as an issue property. $issueProperty = $reflector->getProperty("issue"); @@ -185,23 +129,8 @@ public function testFilterShortMessageWithoutVerb( // Interpret original commit message. $result = $parser->interpret($message); - $reflector = new \ReflectionClass("\\PreCommit\\Filter\\ShortCommitMsg\\Formatter"); - - $formatter = $reflector->newInstanceWithoutConstructor(); - $configProperty = $reflector->getProperty("config"); - - // Get a formatter type property, set a tracker type for the Formatter. - $typeProperty = $reflector->getProperty("type"); - $typeProperty->setAccessible(true); - $typeProperty->setValue($formatter, $trackerType); - - // Get a formatter config property, set a config model for the Formatter. - $configProperty->setAccessible(true); - $configProperty->setValue($formatter, $configMock); - + // Filter result message through a formatter object. $resultMessage = $formatter->filter($result); - - // Verify commit messages $this->assertEquals($resultMessage->__toString(), $resultValue); } @@ -216,7 +145,7 @@ public function testFilterShortMessageWithoutVerb( */ protected function getIssueMock($summary, $key, $type, $originalType) { - /** @var Issue\AbstractAdapter|\PHPUnit_Framework_MockObject_MockObject $filter */ + /** @var Issue\AbstractAdapter|\PHPUnit_Framework_MockObject_MockObject $issue */ $issue = $this->getMockBuilder('PreCommit\Issue\AdapterAbstract') ->setMethods(['getSummary', 'getKey', 'getType', 'getOriginalType']) ->getMock(); @@ -229,7 +158,6 @@ protected function getIssueMock($summary, $key, $type, $originalType) ->willReturn($type); $issue->method('getOriginalType') ->willReturn($originalType); - return $issue; } } From 7da317a5817f49ac580be8c4e0c2fb7b19fbb960 Mon Sep 17 00:00:00 2001 From: ant-workaholic Date: Wed, 5 Jul 2017 17:48:09 +0300 Subject: [PATCH 3/3] Fixed #92: Issue duplicated in description. - Fixed logic pattern group issue. --- src/lib/PreCommit/Filter/ShortCommitMsg/Parser/Jira.php | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/lib/PreCommit/Filter/ShortCommitMsg/Parser/Jira.php b/src/lib/PreCommit/Filter/ShortCommitMsg/Parser/Jira.php index 750a4dd..34f94b9 100644 --- a/src/lib/PreCommit/Filter/ShortCommitMsg/Parser/Jira.php +++ b/src/lib/PreCommit/Filter/ShortCommitMsg/Parser/Jira.php @@ -289,11 +289,8 @@ protected function interpretShortMessage($message) //endregion $userMessage = null; - if ($m) { - $userMessage = trim(array_pop($m)); - if (is_numeric($userMessage)) { - $userMessage = ""; - } + if ($m && isset($m[6])) { + $userMessage = $m[6]; } return [$commitVerb, $issueKey, $userMessage];