{"action":"created","comment":{"url":"https://api.github.com/repos/QISKit/qiskit-sdk-py/pulls/comments/140452941","pull_request_review_id":64529801,"id":140452941,"diff_hunk":"@@ -234,22 +218,36 @@ def test_quantum_program_online(self):\n                             seed=78)\n \n     def test_run_job_processor_local_parallel(self):\n+        def job_done_callback(results):\n+            try:\n+                self.log.info(pprint.pformat(results))\n+                for result, _ in results:\n+                    self.assertTrue(result['result'][0]['status'] == 'DONE')\n+            except Exception as e:\n+                self.job_processor_exception = e\n+            finally:\n+                self.job_processor_finished = True\n+\n         njobs = 20\n         job_list = []\n         for i in range(njobs):\n             compiled_circuit = openquantumcompiler.compile(self.qc.qasm())\n             qjob = jobp.QuantumJob(compiled_circuit, backend='local_qasm_simulator')\n             job_list.append(qjob)\n \n-        def job_done_callback(results):\n-            self.log.info(pprint.pformat(results))\n-            for result, _ in results:\n-                self.assertTrue(result['result'][0]['status'] == 'DONE')\n-\n+        self.job_processor_finished = False\n+        self.job_processor_exception = None\n         jp = jobp.JobProcessor(job_list, max_workers=None,\n                                callback=job_done_callback)\n         jp.submit(silent=True)\n \n+        while not self.job_processor_finished:","path":"test/python/test_job_processor.py","position":139,"original_position":139,"commit_id":"49067d5cb993c4bdff842d4965a4cd6d58d90cfe","original_commit_id":"49067d5cb993c4bdff842d4965a4cd6d58d90cfe","user":{"login":"diego-plan9","id":418311,"avatar_url":"https://avatars3.githubusercontent.com/u/418311?v=4","gravatar_id":"","url":"https://api.github.com/users/diego-plan9","html_url":"https://github.com/diego-plan9","followers_url":"https://api.github.com/users/diego-plan9/followers","following_url":"https://api.github.com/users/diego-plan9/following{/other_user}","gists_url":"https://api.github.com/users/diego-plan9/gists{/gist_id}","starred_url":"https://api.github.com/users/diego-plan9/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/diego-plan9/subscriptions","organizations_url":"https://api.github.com/users/diego-plan9/orgs","repos_url":"https://api.github.com/users/diego-plan9/repos","events_url":"https://api.github.com/users/diego-plan9/events{/privacy}","received_events_url":"https://api.github.com/users/diego-plan9/received_events","type":"User","site_admin":false},"body":"I believe we _do_ need to wait for the callbacks to be finished - without that `while` (and the extra machinery, which admittedly is a bit clunky), `test_run_job_processor_local_parallel()` returns immediately after `jp.submit()`, and since no asserts are executed during the lifetime of that particular test (as they are _inside_ the callback) it would be always marked as `PASSED`.\r\n\r\nSo it seems we need to force those tests not to return in all cases that include assertions inside the callbacks - which I believe is the case in the four methods that have been modified (the two noted on the comments, plus two at `test_quantumprogram.py`)? For the record, the callbacks were being called, but they were called outside the lifetime of the test (they would end up showing on stdout, but not picked up correctly by the testing framework) - which was quite tricky to spot.\r\n\r\nI'm unsure about the note about the name changes, but would be happy to rename them if needed!","created_at":"2017-09-22T09:45:10Z","updated_at":"2017-09-22T09:45:10Z","html_url":"https://github.com/QISKit/qiskit-sdk-py/pull/77#discussion_r140452941","pull_request_url":"https://api.github.com/repos/QISKit/qiskit-sdk-py/pulls/77","author_association":"MEMBER","_links":{"self":{"href":"https://api.github.com/repos/QISKit/qiskit-sdk-py/pulls/comments/140452941"},"html":{"href":"https://github.com/QISKit/qiskit-sdk-py/pull/77#discussion_r140452941"},"pull_request":{"href":"https://api.github.com/repos/QISKit/qiskit-sdk-py/pulls/77"}},"in_reply_to_id":140334802},"pull_request":{"url":"https://api.github.com/repos/QISKit/qiskit-sdk-py/pulls/77","id":142281318,"html_url":"https://github.com/QISKit/qiskit-sdk-py/pull/77","diff_url":"https://github.com/QISKit/qiskit-sdk-py/pull/77.diff","patch_url":"https://github.com/QISKit/qiskit-sdk-py/pull/77.patch","issue_url":"https://api.github.com/repos/QISKit/qiskit-sdk-py/issues/77","number":77,"state":"open","locked":false,"title":"Tests: common functionality refactoring and cleanup","user":{"login":"diego-plan9","id":418311,"avatar_url":"https://avatars3.githubusercontent.com/u/418311?v=4","gravatar_id":"","url":"https://api.github.com/users/diego-plan9","html_url":"https://github.com/diego-plan9","followers_url":"https://api.github.com/users/diego-plan9/followers","following_url":"https://api.github.com/users/diego-plan9/following{/other_user}","gists_url":"https://api.github.com/users/diego-plan9/gists{/gist_id}","starred_url":"https://api.github.com/users/diego-plan9/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/diego-plan9/subscriptions","organizations_url":"https://api.github.com/users/diego-plan9/orgs","repos_url":"https://api.github.com/users/diego-plan9/repos","events_url":"https://api.github.com/users/diego-plan9/events{/privacy}","received_events_url":"https://api.github.com/users/diego-plan9/received_events","type":"User","site_admin":false},"body":"<!--- Provide a general summary of your changes in the Title above -->\r\n\r\n## Description\r\n<!--- Describe your changes in detail -->\r\nSome changes aimed to reduce the amount of duplicated code and overall following of standard practices on the tests:\r\n* a new `test.python.common` module that provides common functionality:\r\n    * a `QiskitTestCase` (subclassing `unittest.TestCase`)  class that setups the logging and has a helper method for retrieving the filename of the resources (asm examples, other files, etc). \r\n    * the `TRAVIS_FORK_PULL_REQUEST` variable as a module variable.\r\n* removed the `sys.path` handling from the tests, assuming (following a recent conversation) that the tests will always be run either via `unittest` or as a module, and not by executing the files directly (as this was the main cause of the `sys.path` modifications and conditional imports).\r\n* other minor tweaks such as revising the license headers, reorganizing imports, and PEP8 fixed (not fully yet).\r\n\r\nAdditionally two other functionality changes are included:\r\n* changed  `_random_circuit_generator.add_circuits()` to be Python 3.5 compatible - it was using `random.choices()`, that was introduced in Python 3.6, and my understanding is that the official stance (for now) is to be \"Python 3.5+ compatible\".\r\n* updated the tests for the async functionality in order to wait until the callbacks are called and completed before returning.\r\n\r\n## Motivation and Context\r\n<!--- Why is this change required? What problem does it solve? -->\r\n<!--- If it fixes an open issue, please link to the issue here. -->\r\n\r\nThe goal is to reduce the complexity of the tests themselves (by avoiding duplication and providing easy access to common functionality); and follow best Python practices for easier maintainability, etc.\r\n\r\n## How Has This Been Tested?\r\n<!--- Please describe in detail how you tested your changes. -->\r\n<!--- Include details of your testing environment, and the tests you ran to -->\r\n<!--- see how your change affects other areas of the code, etc. -->\r\n\r\nBy making sure `make test`, `pytest`,  and manual invocation of each module (via `python -m unittest test.python.test_x` and `python -m test.python.test_x`) produce the expected results (tests run, log files and other artifacts produced, resources can be read/written).\r\n\r\nPlease note that, with the changes on this PR, it is **not** possible to run the test files directly (via `python test_x.py`) anymore due to the relative imports (and the removal of the `sys.path` mangling). \r\n\r\n## Screenshots (if appropriate):\r\n\r\n## Types of changes\r\n<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->\r\n- [ ] Bug fix (non-breaking change which fixes an issue)\r\n- [ ] New feature (non-breaking change which adds functionality)\r\n- [ ] Breaking change (fix or feature that would cause existing functionality to change)\r\n\r\n## Checklist:\r\n<!--- Go over all the following points, and put an `x` in all the boxes that apply. -->\r\n<!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->\r\n- [x] My code follows the code style of this project.\r\n- [ ] My change requires a change to the documentation.\r\n- [ ] I have updated the documentation accordingly.\r\n- [x] I have read the **CONTRIBUTING** document.\r\n- [ ] I have added tests to cover my changes.\r\n- [x] All new and existing tests passed.","created_at":"2017-09-21T08:14:47Z","updated_at":"2017-09-22T09:45:10Z","closed_at":null,"merged_at":null,"merge_commit_sha":"63f821497c32978d6ecd75190655ad683c4fc482","assignee":null,"assignees":[],"requested_reviewers":[],"milestone":null,"commits_url":"https://api.github.com/repos/QISKit/qiskit-sdk-py/pulls/77/commits","review_comments_url":"https://api.github.com/repos/QISKit/qiskit-sdk-py/pulls/77/comments","review_comment_url":"https://api.github.com/repos/QISKit/qiskit-sdk-py/pulls/comments{/number}","comments_url":"https://api.github.com/repos/QISKit/qiskit-sdk-py/issues/77/comments","statuses_url":"https://api.github.com/repos/QISKit/qiskit-sdk-py/statuses/49067d5cb993c4bdff842d4965a4cd6d58d90cfe","head":{"label":"diego-plan9:feature/test-unifying","ref":"feature/test-unifying","sha":"49067d5cb993c4bdff842d4965a4cd6d58d90cfe","user":{"login":"diego-plan9","id":418311,"avatar_url":"https://avatars3.githubusercontent.com/u/418311?v=4","gravatar_id":"","url":"https://api.github.com/users/diego-plan9","html_url":"https://github.com/diego-plan9","followers_url":"https://api.github.com/users/diego-plan9/followers","following_url":"https://api.github.com/users/diego-plan9/following{/other_user}","gists_url":"https://api.github.com/users/diego-plan9/gists{/gist_id}","starred_url":"https://api.github.com/users/diego-plan9/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/diego-plan9/subscriptions","organizations_url":"https://api.github.com/users/diego-plan9/orgs","repos_url":"https://api.github.com/users/diego-plan9/repos","events_url":"https://api.github.com/users/diego-plan9/events{/privacy}","received_events_url":"https://api.github.com/users/diego-plan9/received_events","type":"User","site_admin":false},"repo":{"id":102835926,"name":"qiskit-sdk-py","full_name":"diego-plan9/qiskit-sdk-py","owner":{"login":"diego-plan9","id":418311,"avatar_url":"https://avatars3.githubusercontent.com/u/418311?v=4","gravatar_id":"","url":"https://api.github.com/users/diego-plan9","html_url":"https://github.com/diego-plan9","followers_url":"https://api.github.com/users/diego-plan9/followers","following_url":"https://api.github.com/users/diego-plan9/following{/other_user}","gists_url":"https://api.github.com/users/diego-plan9/gists{/gist_id}","starred_url":"https://api.github.com/users/diego-plan9/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/diego-plan9/subscriptions","organizations_url":"https://api.github.com/users/diego-plan9/orgs","repos_url":"https://api.github.com/users/diego-plan9/repos","events_url":"https://api.github.com/users/diego-plan9/events{/privacy}","received_events_url":"https://api.github.com/users/diego-plan9/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/diego-plan9/qiskit-sdk-py","description":"Python software development kit for writing quantum computing experiments,  programs, and applications","fork":true,"url":"https://api.github.com/repos/diego-plan9/qiskit-sdk-py","forks_url":"https://api.github.com/repos/diego-plan9/qiskit-sdk-py/forks","keys_url":"https://api.github.com/repos/diego-plan9/qiskit-sdk-py/keys{/key_id}","collaborators_url":"https://api.github.com/repos/diego-plan9/qiskit-sdk-py/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/diego-plan9/qiskit-sdk-py/teams","hooks_url":"https://api.github.com/repos/diego-plan9/qiskit-sdk-py/hooks","issue_events_url":"https://api.github.com/repos/diego-plan9/qiskit-sdk-py/issues/events{/number}","events_url":"https://api.github.com/repos/diego-plan9/qiskit-sdk-py/events","assignees_url":"https://api.github.com/repos/diego-plan9/qiskit-sdk-py/assignees{/user}","branches_url":"https://api.github.com/repos/diego-plan9/qiskit-sdk-py/branches{/branch}","tags_url":"https://api.github.com/repos/diego-plan9/qiskit-sdk-py/tags","blobs_url":"https://api.github.com/repos/diego-plan9/qiskit-sdk-py/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/diego-plan9/qiskit-sdk-py/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/diego-plan9/qiskit-sdk-py/git/refs{/sha}","trees_url":"https://api.github.com/repos/diego-plan9/qiskit-sdk-py/git/trees{/sha}","statuses_url":"https://api.github.com/repos/diego-plan9/qiskit-sdk-py/statuses/{sha}","languages_url":"https://api.github.com/repos/diego-plan9/qiskit-sdk-py/languages","stargazers_url":"https://api.github.com/repos/diego-plan9/qiskit-sdk-py/stargazers","contributors_url":"https://api.github.com/repos/diego-plan9/qiskit-sdk-py/contributors","subscribers_url":"https://api.github.com/repos/diego-plan9/qiskit-sdk-py/subscribers","subscription_url":"https://api.github.com/repos/diego-plan9/qiskit-sdk-py/subscription","commits_url":"https://api.github.com/repos/diego-plan9/qiskit-sdk-py/commits{/sha}","git_commits_url":"https://api.github.com/repos/diego-plan9/qiskit-sdk-py/git/commits{/sha}","comments_url":"https://api.github.com/repos/diego-plan9/qiskit-sdk-py/comments{/number}","issue_comment_url":"https://api.github.com/repos/diego-plan9/qiskit-sdk-py/issues/comments{/number}","contents_url":"https://api.github.com/repos/diego-plan9/qiskit-sdk-py/contents/{+path}","compare_url":"https://api.github.com/repos/diego-plan9/qiskit-sdk-py/compare/{base}...{head}","merges_url":"https://api.github.com/repos/diego-plan9/qiskit-sdk-py/merges","archive_url":"https://api.github.com/repos/diego-plan9/qiskit-sdk-py/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/diego-plan9/qiskit-sdk-py/downloads","issues_url":"https://api.github.com/repos/diego-plan9/qiskit-sdk-py/issues{/number}","pulls_url":"https://api.github.com/repos/diego-plan9/qiskit-sdk-py/pulls{/number}","milestones_url":"https://api.github.com/repos/diego-plan9/qiskit-sdk-py/milestones{/number}","notifications_url":"https://api.github.com/repos/diego-plan9/qiskit-sdk-py/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/diego-plan9/qiskit-sdk-py/labels{/name}","releases_url":"https://api.github.com/repos/diego-plan9/qiskit-sdk-py/releases{/id}","deployments_url":"https://api.github.com/repos/diego-plan9/qiskit-sdk-py/deployments","created_at":"2017-09-08T08:07:44Z","updated_at":"2017-09-08T08:07:46Z","pushed_at":"2017-09-21T10:46:59Z","git_url":"git://github.com/diego-plan9/qiskit-sdk-py.git","ssh_url":"git@github.com:diego-plan9/qiskit-sdk-py.git","clone_url":"https://github.com/diego-plan9/qiskit-sdk-py.git","svn_url":"https://github.com/diego-plan9/qiskit-sdk-py","homepage":"","size":20750,"stargazers_count":0,"watchers_count":0,"language":"Python","has_issues":false,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"QISKit:master","ref":"master","sha":"16d928d67b76843d8eeea4e145e43e3d073b1410","user":{"login":"QISKit","id":30696987,"avatar_url":"https://avatars1.githubusercontent.com/u/30696987?v=4","gravatar_id":"","url":"https://api.github.com/users/QISKit","html_url":"https://github.com/QISKit","followers_url":"https://api.github.com/users/QISKit/followers","following_url":"https://api.github.com/users/QISKit/following{/other_user}","gists_url":"https://api.github.com/users/QISKit/gists{/gist_id}","starred_url":"https://api.github.com/users/QISKit/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/QISKit/subscriptions","organizations_url":"https://api.github.com/users/QISKit/orgs","repos_url":"https://api.github.com/users/QISKit/repos","events_url":"https://api.github.com/users/QISKit/events{/privacy}","received_events_url":"https://api.github.com/users/QISKit/received_events","type":"Organization","site_admin":false},"repo":{"id":83821669,"name":"qiskit-sdk-py","full_name":"QISKit/qiskit-sdk-py","owner":{"login":"QISKit","id":30696987,"avatar_url":"https://avatars1.githubusercontent.com/u/30696987?v=4","gravatar_id":"","url":"https://api.github.com/users/QISKit","html_url":"https://github.com/QISKit","followers_url":"https://api.github.com/users/QISKit/followers","following_url":"https://api.github.com/users/QISKit/following{/other_user}","gists_url":"https://api.github.com/users/QISKit/gists{/gist_id}","starred_url":"https://api.github.com/users/QISKit/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/QISKit/subscriptions","organizations_url":"https://api.github.com/users/QISKit/orgs","repos_url":"https://api.github.com/users/QISKit/repos","events_url":"https://api.github.com/users/QISKit/events{/privacy}","received_events_url":"https://api.github.com/users/QISKit/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/QISKit/qiskit-sdk-py","description":"Python software development kit for writing quantum computing experiments,  programs, and applications","fork":false,"url":"https://api.github.com/repos/QISKit/qiskit-sdk-py","forks_url":"https://api.github.com/repos/QISKit/qiskit-sdk-py/forks","keys_url":"https://api.github.com/repos/QISKit/qiskit-sdk-py/keys{/key_id}","collaborators_url":"https://api.github.com/repos/QISKit/qiskit-sdk-py/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/QISKit/qiskit-sdk-py/teams","hooks_url":"https://api.github.com/repos/QISKit/qiskit-sdk-py/hooks","issue_events_url":"https://api.github.com/repos/QISKit/qiskit-sdk-py/issues/events{/number}","events_url":"https://api.github.com/repos/QISKit/qiskit-sdk-py/events","assignees_url":"https://api.github.com/repos/QISKit/qiskit-sdk-py/assignees{/user}","branches_url":"https://api.github.com/repos/QISKit/qiskit-sdk-py/branches{/branch}","tags_url":"https://api.github.com/repos/QISKit/qiskit-sdk-py/tags","blobs_url":"https://api.github.com/repos/QISKit/qiskit-sdk-py/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/QISKit/qiskit-sdk-py/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/QISKit/qiskit-sdk-py/git/refs{/sha}","trees_url":"https://api.github.com/repos/QISKit/qiskit-sdk-py/git/trees{/sha}","statuses_url":"https://api.github.com/repos/QISKit/qiskit-sdk-py/statuses/{sha}","languages_url":"https://api.github.com/repos/QISKit/qiskit-sdk-py/languages","stargazers_url":"https://api.github.com/repos/QISKit/qiskit-sdk-py/stargazers","contributors_url":"https://api.github.com/repos/QISKit/qiskit-sdk-py/contributors","subscribers_url":"https://api.github.com/repos/QISKit/qiskit-sdk-py/subscribers","subscription_url":"https://api.github.com/repos/QISKit/qiskit-sdk-py/subscription","commits_url":"https://api.github.com/repos/QISKit/qiskit-sdk-py/commits{/sha}","git_commits_url":"https://api.github.com/repos/QISKit/qiskit-sdk-py/git/commits{/sha}","comments_url":"https://api.github.com/repos/QISKit/qiskit-sdk-py/comments{/number}","issue_comment_url":"https://api.github.com/repos/QISKit/qiskit-sdk-py/issues/comments{/number}","contents_url":"https://api.github.com/repos/QISKit/qiskit-sdk-py/contents/{+path}","compare_url":"https://api.github.com/repos/QISKit/qiskit-sdk-py/compare/{base}...{head}","merges_url":"https://api.github.com/repos/QISKit/qiskit-sdk-py/merges","archive_url":"https://api.github.com/repos/QISKit/qiskit-sdk-py/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/QISKit/qiskit-sdk-py/downloads","issues_url":"https://api.github.com/repos/QISKit/qiskit-sdk-py/issues{/number}","pulls_url":"https://api.github.com/repos/QISKit/qiskit-sdk-py/pulls{/number}","milestones_url":"https://api.github.com/repos/QISKit/qiskit-sdk-py/milestones{/number}","notifications_url":"https://api.github.com/repos/QISKit/qiskit-sdk-py/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/QISKit/qiskit-sdk-py/labels{/name}","releases_url":"https://api.github.com/repos/QISKit/qiskit-sdk-py/releases{/id}","deployments_url":"https://api.github.com/repos/QISKit/qiskit-sdk-py/deployments","created_at":"2017-03-03T17:02:42Z","updated_at":"2017-09-22T08:31:24Z","pushed_at":"2017-09-21T22:08:31Z","git_url":"git://github.com/QISKit/qiskit-sdk-py.git","ssh_url":"git@github.com:QISKit/qiskit-sdk-py.git","clone_url":"https://github.com/QISKit/qiskit-sdk-py.git","svn_url":"https://github.com/QISKit/qiskit-sdk-py","homepage":"","size":20264,"stargazers_count":516,"watchers_count":516,"language":"Python","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":150,"mirror_url":null,"open_issues_count":12,"forks":150,"open_issues":12,"watchers":516,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/QISKit/qiskit-sdk-py/pulls/77"},"html":{"href":"https://github.com/QISKit/qiskit-sdk-py/pull/77"},"issue":{"href":"https://api.github.com/repos/QISKit/qiskit-sdk-py/issues/77"},"comments":{"href":"https://api.github.com/repos/QISKit/qiskit-sdk-py/issues/77/comments"},"review_comments":{"href":"https://api.github.com/repos/QISKit/qiskit-sdk-py/pulls/77/comments"},"review_comment":{"href":"https://api.github.com/repos/QISKit/qiskit-sdk-py/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/QISKit/qiskit-sdk-py/pulls/77/commits"},"statuses":{"href":"https://api.github.com/repos/QISKit/qiskit-sdk-py/statuses/49067d5cb993c4bdff842d4965a4cd6d58d90cfe"}},"author_association":"MEMBER"}}
{"actor":{"display_login":"diego-plan9"}}