Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions src/Jira/Api.php
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,7 @@ public function getVersions($project_key)
* @param string $project_key Project Key.
* @param string $name The version name to match on.
*
* @return integer|null VersionId on match or null when there is no match.
* @return array|null Version data on match or null when there is no match.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch

* @since 2.0.0
*/
public function findVersionByName($project_key, $name)
Expand Down Expand Up @@ -592,7 +592,7 @@ public function createVersion($project_key, $version, array $options = array())
* @param integer $version_id Version ID.
* @param array $params Key->Value list to update the version with.
*
* @return Result|false
* @return false
* @since 2.0.0
* @link https://docs.atlassian.com/jira/REST/latest/#api/2/version-updateVersion
*/
Expand All @@ -608,7 +608,7 @@ public function updateVersion($version_id, array $params = array())
* @param string|null $release_date Date in Y-m-d format (defaults to today).
* @param array $params Optionally extra parameters.
*
* @return Result|false
* @return false
* @since 2.0.0
*/
public function releaseVersion($version_id, $release_date = null, array $params = array())
Expand Down Expand Up @@ -858,7 +858,7 @@ public function getProjectComponents($project_key)
{
return $this->api(self::REQUEST_GET, sprintf('/rest/api/2/project/%s/components', $project_key), array(), true);
}

/**
* Get all issue types with valid status values for a project.
*
Expand All @@ -867,11 +867,11 @@ public function getProjectComponents($project_key)
* @return array
* @since 2.0.0
*/
public function getProjectIssueTypes($project_key)
public function getProjectIssueTypes($project_key)
{
return $this->api(self::REQUEST_GET, sprintf('/rest/api/2/project/%s/statuses', $project_key), array(), true);
}

/**
* Returns a list of all resolutions.
*
Expand All @@ -880,7 +880,7 @@ public function getProjectIssueTypes($project_key)
* @return array|false
* @since 2.0.0
*/
public function getResolutions()
public function getResolutions()
{
return $this->api(self::REQUEST_GET, '/rest/api/2/resolution', array(), true);
}
Expand Down
177 changes: 113 additions & 64 deletions tests/Jira/ApiTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@


use chobie\Jira\Api;
use chobie\Jira\Api\Authentication\Anonymous;
use chobie\Jira\Api\Authentication\AuthenticationInterface;
use Prophecy\Prophecy\ObjectProphecy;

/**
* Class ApiTest
Expand All @@ -14,116 +15,164 @@
class ApiTest extends \PHPUnit_Framework_TestCase
{

const ENDPOINT = 'http://jira.company.com';

/**
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is replaced by testSetendpoint with data provider usage for cases, when trailing slash is present or not.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah saw it, tried to remove the comment but you were too fast ;)

Copy link
Member Author

@aik099 aik099 Jul 1, 2016 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

* Api.
*
* @var Api
*/
protected $api;

/**
* Tests that any trailing slash in the endpoint url is removed before being stored in the object state
* Credential.
*
* @var AuthenticationInterface
*/
public function testSetEndpointTrailingSlash()
protected $credential;

/**
* Client.
*
* @var ObjectProphecy
*/
protected $client;

protected function setUp()
{
$api = new Api('https://test.test/', new Anonymous(), null);
$this->assertEquals('https://test.test', $api->getEndpoint());
parent::setUp();

// Make sure nothing is removed if there is no trailing slash
$url = 'https://urlwithouttrailing.slash';
$api->setEndPoint($url);
$this->assertEquals($url, $api->getEndpoint());
$this->credential = $this->prophesize('chobie\Jira\Api\Authentication\AuthenticationInterface')->reveal();
$this->client = $this->prophesize('chobie\Jira\Api\Client\ClientInterface');

$this->api = new Api(self::ENDPOINT, $this->credential, $this->client->reveal());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed much nicer to mock the clientinterface instead of the Api::api method

}

/**
* Tests that the updateVersion call constructs the correct api call
* @dataProvider setEndpointDataProvider
*/
public function testSetEndpoint($given_endpoint, $used_endpoint)
{
$api = new Api($given_endpoint, $this->credential, $this->client->reveal());
$this->assertEquals($used_endpoint, $api->getEndpoint());
}

public function setEndpointDataProvider()
{
return array(
'trailing slash removed' => array('https://test.test/', 'https://test.test'),
'nothing removed' => array('https://test.test', 'https://test.test'),
);
}

public function testUpdateVersion()
{
$params = array(
'released' => true,
'releaseDate' => '2010-07-06',
'overdue' => true,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why change these inputs?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at previous test code it was obvious that sample data was copy-pasted from releaseVersion test, where exactly that data is submitted to JIRA.

In there we testing version update and that have nothing to do with indirect version release (it can be used to release if user wishes so, but that's not primary purpose of this method). To point that out even more I've used completely unrelated to releasing fields of the version.

'description' => 'new description',
);

// Stub the api method and keep the rest intact
/** @var Api|\PHPUnit_Framework_MockObject_MockObject $api */
$api = $this->getMockBuilder('\chobie\Jira\Api')->setMethods(array('api'))->disableOriginalConstructor()->getMock();
$api->expects($this->once())->method('api')->with(
$this->equalTo(Api::REQUEST_PUT),
$this->equalTo('/rest/api/2/version/111000'),
$this->equalTo($params)
$this->expectClientCall(
Api::REQUEST_PUT,
'/rest/api/2/version/111000',
$params,
''
);

$api->updateVersion(111000, $params);
$this->assertFalse($this->api->updateVersion(111000, $params));
}

/**
* Tests that the releaseVersion call constructs the correct api call
*/
public function testReleaseVersion()
public function testReleaseVersionAutomaticReleaseDate()
{
$params = array(
'released' => true,
'releaseDate' => date('Y-m-d'),
);

// Stub the api method and keep the rest intact
/** @var Api|\PHPUnit_Framework_MockObject_MockObject $api */
$api = $this->getMockBuilder('\chobie\Jira\Api')->setMethods(array('api'))->disableOriginalConstructor()->getMock();
$api->expects($this->once())->method('api')->with(
$this->equalTo(Api::REQUEST_PUT),
$this->equalTo('/rest/api/2/version/111000'),
$this->equalTo($params)
$this->expectClientCall(
Api::REQUEST_PUT,
'/rest/api/2/version/111000',
$params,
''
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Invalid return type, it would be either a Result or false.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this comment addressed to the right line of code?

Ther '' you've selected for comment is what ClientInterface implementing class returns. And it always returns strings, because that's what HTTP response is. It's Api::api that is json-decoding them and wrapping them into Result class.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah right

);

$api->releaseVersion(111000);
$this->assertFalse($this->api->releaseVersion(111000));
}

/**
* Tests that the releaseVersion call constructs the correct api call with overriden release data and params
*/
public function testReleaseVersionAdvanced()
public function testReleaseVersionParameterMerging()
{
$releaseDate = '2010-07-06';
$release_date = '2010-07-06';

$params = array(
$expected_params = array(
'released' => true,
'releaseDate' => $releaseDate,
'releaseDate' => $release_date,
'test' => 'extra',
);

// Stub the api method and keep the rest intact
/** @var Api|\PHPUnit_Framework_MockObject_MockObject $api */
$api = $this->getMockBuilder('\chobie\Jira\Api')->setMethods(array('api'))->disableOriginalConstructor()->getMock();
$api->expects($this->once())->method('api')->with(
$this->equalTo(Api::REQUEST_PUT),
$this->equalTo('/rest/api/2/version/111000'),
$this->equalTo($params)
$this->expectClientCall(
Api::REQUEST_PUT,
'/rest/api/2/version/111000',
$expected_params,
''
);

$api->releaseVersion(111000, $releaseDate, array('test' => 'extra'));
$this->assertFalse($this->api->releaseVersion(111000, $release_date, array('test' => 'extra')));
}

/**
* Tests FindVersionByName
*/
public function testFindVersionByName()
{
$name = '3.36.0';
$versionId = '14206';
$projectKey = 'POR';
$project_key = 'POR';
$version_id = '14206';
$version_name = '3.36.0';

$versions = array(
array('id' => '14205', 'name' => '3.62.0'),
array('id' => $versionId, 'name' => $name),
array('id' => $version_id, 'name' => $version_name),
array('id' => '14207', 'name' => '3.66.0'),
);

// Stub the getVersions method and keep the rest intact
/** @var Api|\PHPUnit_Framework_MockObject_MockObject $api */
$api = $this->getMockBuilder('\chobie\Jira\Api')->setMethods(array('getVersions'))->disableOriginalConstructor()->getMock();
$api->expects($this->exactly(2))->method('getVersions')->with(
$this->equalTo($projectKey)
)->willReturn($versions);
$this->expectClientCall(
Api::REQUEST_GET,
'/rest/api/2/project/' . $project_key . '/versions',
array(),
json_encode($versions)
);

// He should find this one
$this->assertEquals(array('id' => $versionId, 'name' => $name), $api->findVersionByName($projectKey, $name));
$this->assertEquals(
array('id' => $version_id, 'name' => $version_name),
$this->api->findVersionByName($project_key, $version_name),
'Version found'
);

// And there should be no result for this one
$this->assertNull($api->findVersionByName($projectKey, 'i_do_not_exist'));
$this->assertNull(
$this->api->findVersionByName($project_key, 'i_do_not_exist')
);
}

/**
* Expects a particular client call.
*
* @param string $method Request method.
* @param string $url URL.
* @param array|string $data Request data.
* @param string $return_value Return value.
* @param boolean $is_file This is a file upload request.
* @param boolean $debug Debug this request.
*
* @return void
*/
protected function expectClientCall(
$method,
$url,
$data = array(),
$return_value,
$is_file = false,
$debug = false
) {
$this->client
->sendRequest($method, $url, $data, self::ENDPOINT, $this->credential, $is_file, $debug)
->willReturn($return_value)
->shouldBeCalled();
}

}