{"action":"created","issue":{"url":"https://api.github.com/repos/civicrm/civihr/issues/1087","repository_url":"https://api.github.com/repos/civicrm/civihr","labels_url":"https://api.github.com/repos/civicrm/civihr/issues/1087/labels{/name}","comments_url":"https://api.github.com/repos/civicrm/civihr/issues/1087/comments","events_url":"https://api.github.com/repos/civicrm/civihr/issues/1087/events","html_url":"https://github.com/civicrm/civihr/pull/1087","id":171208554,"number":1087,"title":"PCHR-1239: Cleaning and fix Hrjobcontract Extenstion tests","user":{"login":"omarabuhussein","id":6275540,"avatar_url":"https://avatars.githubusercontent.com/u/6275540?v=3","gravatar_id":"","url":"https://api.github.com/users/omarabuhussein","html_url":"https://github.com/omarabuhussein","followers_url":"https://api.github.com/users/omarabuhussein/followers","following_url":"https://api.github.com/users/omarabuhussein/following{/other_user}","gists_url":"https://api.github.com/users/omarabuhussein/gists{/gist_id}","starred_url":"https://api.github.com/users/omarabuhussein/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/omarabuhussein/subscriptions","organizations_url":"https://api.github.com/users/omarabuhussein/orgs","repos_url":"https://api.github.com/users/omarabuhussein/repos","events_url":"https://api.github.com/users/omarabuhussein/events{/privacy}","received_events_url":"https://api.github.com/users/omarabuhussein/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":2,"created_at":"2016-08-15T16:36:44Z","updated_at":"2016-08-15T18:33:15Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/civicrm/civihr/pulls/1087","html_url":"https://github.com/civicrm/civihr/pull/1087","diff_url":"https://github.com/civicrm/civihr/pull/1087.diff","patch_url":"https://github.com/civicrm/civihr/pull/1087.patch"},"body":"## Problem\r\n \r\nSome jobcontracts unit tests were not working.\r\n\r\n\r\n## Solution \r\n\r\nI walked through the extension tests and here is the changes I've made :\r\n\r\n1- removing the following test classes :\r\n\r\n- CRM/Hrjobcontract/AddRevisionTest.php\r\n- CRM/Hrjobcontract/EditRevisionTest.php\r\n- CRM/Hrjobcontract/CreateRevisionTest.php\r\n\r\nwhich I found useless.\r\n\r\n2- refactoring CRM/Hrjobcontract/BAO/HRJobContractRevisionTest.php to extend PHPUnit_Framework_TestCase and implement the headless interface instead of extending the slow CiviUnitTestCase class , also I still not convinced with the test cases inside it but I will keep it for now until Hrjobcontract data structure and implementation being revisited and everything is clear about it.\r\n\r\n3- removing HRJobContractTestBase.php which is not needed anymore.\r\n\r\n4- replacing CRM/Hrjobcontract/ValidateDatesTest.php with CRM/Hrjobcontract/BAO/HRJobDetailsTest.php and use BAO methods inside it instead of API calls.\r\n\r\n5- removing testGetActiveContractsReturnsActiveContracts() method from  api/v3/HRJobContractTest.php since CRM/Hrjobcontract/BAO/HRJobContractTest.php tests the same thing.\r\n"},"comment":{"url":"https://api.github.com/repos/civicrm/civihr/issues/comments/239887261","html_url":"https://github.com/civicrm/civihr/pull/1087#issuecomment-239887261","issue_url":"https://api.github.com/repos/civicrm/civihr/issues/1087","id":239887261,"user":{"login":"omarabuhussein","id":6275540,"avatar_url":"https://avatars.githubusercontent.com/u/6275540?v=3","gravatar_id":"","url":"https://api.github.com/users/omarabuhussein","html_url":"https://github.com/omarabuhussein","followers_url":"https://api.github.com/users/omarabuhussein/followers","following_url":"https://api.github.com/users/omarabuhussein/following{/other_user}","gists_url":"https://api.github.com/users/omarabuhussein/gists{/gist_id}","starred_url":"https://api.github.com/users/omarabuhussein/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/omarabuhussein/subscriptions","organizations_url":"https://api.github.com/users/omarabuhussein/orgs","repos_url":"https://api.github.com/users/omarabuhussein/repos","events_url":"https://api.github.com/users/omarabuhussein/events{/privacy}","received_events_url":"https://api.github.com/users/omarabuhussein/received_events","type":"User","site_admin":false},"created_at":"2016-08-15T18:33:15Z","updated_at":"2016-08-15T18:33:15Z","body":"> Would you care to explain why do you think AddRevisionTest.php, EditRevisionTest.php and CreateRevisionTest.php are useless? They're are far from what I would call good tests\r\n\r\nthese are really test nothing , look for example on the at the expected array :\r\n\r\n```\r\n$expected = array(\t\t\r\n \"id\" => \"8\",\t\t\r\n\"jobcontract_id\" => \"1\",\t\t\r\n\"details_revision_id\" => \"1\",\t\t\r\n \"health_revision_id\" => \"3\",\t\t\r\n\"role_revision_id\" => \"4\",\t\t\r\n \"hour_revision_id\" => \"5\",\t\t\r\n \"pay_revision_id\" => \"6\",\t\t\r\n  \"leave_revision_id\" => \"7\",\t\t\r\n \"pension_revision_id\" => \"8\",\t\t\r\n  );\r\n```\r\nwhich is testing auto-generated database IDs and expect them to be the same for each test which doesn't make sense and will usually fail .\r\n\r\n\r\nI thought about writing them in a proper way , but the problem that we are currently in the process of revisiting job contract data structure and implementation .. I expect many changes may happen so investing time on writing tests for this may be a waste of energy ... \r\n\r\nan example of that , the current way of creating job contracts should be inside a transaction and in order :\r\n- creating a record in hrjobcontract table.\r\n- creating a record in hrjobcontract_revision table\r\n- creating a recored in hrjobcontract_details table\r\n- creating a recored in hrjobcontract_pay table\r\n- creating a recored in hrjobcontract_hours table\r\n... etc ( hence that even me not still sure about the order above ) ."}}
{"actor":{"display_login":"omarabuhussein"}}