Increase coverage of `IBMQClient` tests by max-reuter-2 · Pull Request #188 · Qiskit/qiskit-ibmq-provider

max-reuter-2 · 2019-06-24T20:42:25Z

Summary

[WIP] Addresses #93.

Details and comments

diego-plan9

Thanks @max-reuter - nice to see the new tests and a better organization! There will be another round of comments after merging 186, as the setupClass changes are likely to cause some conflicts - but we will tackle them when they arise.

diego-plan9 · 2019-06-25T17:52:11Z

+        # comparison against the original job because some fields might have changed.
+        self.assertIn('shots', job_excluded)
+
+    def test_job_submit(self):


Is there a difference (functionality-wise) between this new test and TestIBMQClient.test_run_job()? If not, we should probably remove one of them - conceptually I think it fits better here at TestIBMQClientJobs indeed (but see the notes below).

diego-plan9 · 2019-06-25T17:52:28Z

+
+    def test_job_submit(self):
+        """Test job submission."""
+        result = self.client.job_submit(self.backend_name, self.qobj.to_dict())


Due to the construction of the class, a job is submitted twice?

diego-plan9 · 2019-06-25T17:55:47Z

+    def test_job_submit(self):
+        """Test job submission."""
+        result = self.client.job_submit(self.backend_name, self.qobj.to_dict())
+        status_present = ['status' in [dict_ for dict_ in result.get('qasms')]]


It is probably enough to just check for status in the root of the dict (and a bit safer)?

diego-plan9 · 2019-06-25T17:56:43Z

+
+    @skip
+    # TODO: Q-Object-External-Storage property is not allowed in this backend
+    def test_job_submit_object_storage(self):


Can you check if this test would end up being the same as TestIBMQClient.test_run_job_object_storage, and if so, move it here instead of this test?

diego-plan9 · 2019-06-25T17:57:28Z

+
+    @skip
+    # TODO: RequestsApiError: 404 Client Error: Not Found for url: [...] None, Error code: NotFound.
+    def test_job_download_qobj_object_storage(self):


Can you remove this test? It is a bit harder to reach this function, and probably can be tackled in another issue or indirectly.

diego-plan9 · 2019-06-25T17:59:12Z

+        """Test getting job status."""
+        result = self.client.job_status(self.job_id)
+        status_present = 'status' in result
+        self.assertTrue(status_present)


Can you do instead:

self.assertIn('status', result)

for readability purposes? Note that this applies to other tests as well.

diego-plan9 · 2019-06-25T18:02:44Z

+
+    def test_job_properties(self):
+        """Test getting job properties."""
+        # TODO - is {} an acceptable response here?


Yes - properties might not be available until the job has finished. Actually, can you get the job to finish inside this test, and after that do the relevant assertions (you can use a field that is required for the BackendProperties - for example, backend_name, probably).

diego-plan9 · 2019-06-25T18:04:09Z

+        self.assertIsNotNone(result)
+
+    # TODO: RequestsApiError: 404 Client Error: Not Found for url: [...]
+    def test_job_cancel(self):


Can you remove this test? It is also one of those that require slightly trick logic (in this case, you need the job to be in a "cancellable" state) and better of included during #189

diego-plan9 · 2019-06-25T18:08:05Z

+        status_present = ['status' in [dict_ for dict_ in result.get('qasms')]]
+        self.assertTrue(status_present)
+
+    def test_job_get_filtered_fields(self):


Can you check if this tests is not already covered by the moved test_get_job_includes_xxx/test_get_job_excludes_xxx tests a few lines above? Note that client.get_job() is actually an alias for client.job_get() for compatibility purposes (actually, we should rename the mentioned tests to test_job_get_includes_.... for consistency).

Those tests test get_job_xxx, not job_get_xxx. I can update them to do the latter, or if we need both tests I'll keep the similar ones currently in the PR.

diego-plan9 · 2019-06-25T18:12:25Z

+        # TODO
+        pass
+
+    def test_job_get_responsive(self):


Can you rename this one to test_job_get, and also relax the assert?

diego-plan9

I went ahead and solved some conflicts after #186 - and in the process addressed a number of comments. Thanks for getting the tests more manageable and increasing the coverage of this critical piece, @max-reuter !

* update runtime metadata * return if no data * fix mypy * Update releasenotes/notes/update-runtime-metadata-d2ddbcfc0d034530.yaml Co-authored-by: Rathish Cholarajan <rathishc24@gmail.com> (cherry picked from commit b532b1476046eb7faa31f6027ae2f789675a804b)

* Allow updating runtime metadata in place (#188) * update runtime metadata * return if no data * fix mypy * Update releasenotes/notes/update-runtime-metadata-d2ddbcfc0d034530.yaml Co-authored-by: Rathish Cholarajan <rathishc24@gmail.com> (cherry picked from commit b532b1476046eb7faa31f6027ae2f789675a804b) * update release note

* Runtime release Q4 (#1069) * Remove version field from runtime program (#152) * Remove version field from runtime program * Add release note * Rename isPublic to is_public when creating or reading runtime programs (#155) * Update programId to program_id when running program (#139) This needs to change in the program upload body request in order to meet the IBM Cloud API guidance. Co-authored-by: Jessie Yu <jessieyu@us.ibm.com> * Add support to view program update date * Upload runtime program using 'data' field (#157) * Read programs from "programs" array in response (#161) * Pass program as base64 string to update (#168) * Accept JSON schema as program metadata (#158) * Accept JSON schema as program metadata * Update qiskit_ibm/runtime/ibm_runtime_service.py Co-authored-by: Jessie Yu <jessieyu@us.ibm.com> * Apply suggestions from code review * Apply suggestions from code review Co-authored-by: Jessie Yu <jessieyu@us.ibm.com> Co-authored-by: Jessie Yu <jessieyu@us.ibm.com> * Pass program params as object (#171) * Fix integration tests Co-authored-by: Renier Morales <renier@users.noreply.github.com> Co-authored-by: Jessie Yu <jessieyu@us.ibm.com> * Allow updating runtime metadata in place (qiskit-ibm #188) (#1071) * Allow updating runtime metadata in place (#188) * update runtime metadata * return if no data * fix mypy * Update releasenotes/notes/update-runtime-metadata-d2ddbcfc0d034530.yaml Co-authored-by: Rathish Cholarajan <rathishc24@gmail.com> (cherry picked from commit b532b1476046eb7faa31f6027ae2f789675a804b) * update release note * Use count to reduce one last extra call to API (#172) * Allow filtering runtime jobs by program ID (#1082) * Allow runtime program authors to retrieve program data (#174) (#1083) * retrieve program data * refetch once if no program data * remove unused import * refresh program on data property * fix lint * Update qiskit_ibm/runtime/runtime_program.py Co-authored-by: Rathish Cholarajan <rathishc24@gmail.com> * Update qiskit_ibm/runtime/runtime_program.py Co-authored-by: Rathish Cholarajan <rathishc24@gmail.com> * Update qiskit_ibm/runtime/runtime_program.py Co-authored-by: Rathish Cholarajan <rathishc24@gmail.com> * add test case * add test case * add default data constant * add _validate_program method * Update test/ibm/runtime/test_runtime.py Co-authored-by: Rathish Cholarajan <rathishc24@gmail.com> * Update cache after updating program (#196) (#1085) * Support pagination for retrieving runtime programs (#1087) Co-authored-by: Renier Morales <renier@users.noreply.github.com> Co-authored-by: Jessie Yu <jessieyu@us.ibm.com>

* Remove version field from runtime program (Qiskit#152) * Remove version field from runtime program * Add release note * Rename isPublic to is_public when creating or reading runtime programs (Qiskit#155) * Update programId to program_id when running program (Qiskit#139) This needs to change in the program upload body request in order to meet the IBM Cloud API guidance. Co-authored-by: Jessie Yu <jessieyu@us.ibm.com> * Add support to view program update date (Qiskit#160) * Upload runtime program using 'data' field (Qiskit#157) * Read programs from "programs" array in response (Qiskit#161) * Remove version field from runtime program (Qiskit#152) * Remove version field from runtime program * Add release note * Rename isPublic to is_public when creating or reading runtime programs (Qiskit#155) * Update programId to program_id when running program (Qiskit#139) This needs to change in the program upload body request in order to meet the IBM Cloud API guidance. Co-authored-by: Jessie Yu <jessieyu@us.ibm.com> * Add support to view program update date (Qiskit#160) * Upload runtime program using 'data' field (Qiskit#157) * Read programs from "programs" array in response (Qiskit#161) * Pass program as base64 string to update (Qiskit#168) * Accept JSON schema as program metadata (Qiskit#158) * Accept JSON schema as program metadata * Update qiskit_ibm/runtime/ibm_runtime_service.py Co-authored-by: Jessie Yu <jessieyu@us.ibm.com> * Apply suggestions from code review * Apply suggestions from code review Co-authored-by: Jessie Yu <jessieyu@us.ibm.com> Co-authored-by: Jessie Yu <jessieyu@us.ibm.com> * Pass program params as object (Qiskit#171) * Fix integration tests * Use count to reduce one last extra call to API (Qiskit#172) * Allow updating runtime metadata in place (Qiskit#188) * update runtime metadata * return if no data * fix mypy * Update releasenotes/notes/update-runtime-metadata-d2ddbcfc0d034530.yaml Co-authored-by: Rathish Cholarajan <rathishc24@gmail.com> * Remove version field from runtime program (Qiskit#152) * Remove version field from runtime program * Add release note * Rename isPublic to is_public when creating or reading runtime programs (Qiskit#155) * Update programId to program_id when running program (Qiskit#139) This needs to change in the program upload body request in order to meet the IBM Cloud API guidance. Co-authored-by: Jessie Yu <jessieyu@us.ibm.com> * Add support to view program update date (Qiskit#160) * Upload runtime program using 'data' field (Qiskit#157) * Read programs from "programs" array in response (Qiskit#161) * Pass program as base64 string to update (Qiskit#168) * Accept JSON schema as program metadata (Qiskit#158) * Accept JSON schema as program metadata * Update qiskit_ibm/runtime/ibm_runtime_service.py Co-authored-by: Jessie Yu <jessieyu@us.ibm.com> * Apply suggestions from code review * Apply suggestions from code review Co-authored-by: Jessie Yu <jessieyu@us.ibm.com> Co-authored-by: Jessie Yu <jessieyu@us.ibm.com> * Pass program params as object (Qiskit#171) * Fix integration tests * Use count to reduce one last extra call to API (Qiskit#172) * Allow updating runtime metadata in place (Qiskit#188) * update runtime metadata * return if no data * fix mypy * Update releasenotes/notes/update-runtime-metadata-d2ddbcfc0d034530.yaml Co-authored-by: Rathish Cholarajan <rathishc24@gmail.com> * Allow filtering runtime jobs by program ID (Qiskit#193) * Allow filtering runtime jobs by program ID * Fix lint * Allow runtime program authors to retrieve program data (Qiskit#174) * retrieve program data * refetch once if no program data * remove unused import * refresh program on data property * fix lint * Update qiskit_ibm/runtime/runtime_program.py Co-authored-by: Rathish Cholarajan <rathishc24@gmail.com> * Update qiskit_ibm/runtime/runtime_program.py Co-authored-by: Rathish Cholarajan <rathishc24@gmail.com> * Update qiskit_ibm/runtime/runtime_program.py Co-authored-by: Rathish Cholarajan <rathishc24@gmail.com> * add test case * add test case * add default data constant * add _validate_program method * Update test/ibm/runtime/test_runtime.py Co-authored-by: Rathish Cholarajan <rathishc24@gmail.com> * Update cache after updating program (Qiskit#196) * Allow filtering runtime jobs by provider (Qiskit#197) * add provider param * split provider into hub/group/project * add reno * wip add test case * fix lint/docs * Update qiskit_ibm/runtime/ibm_runtime_service.py Co-authored-by: Rathish Cholarajan <rathishc24@gmail.com> * Update releasenotes/notes/filter-jobs-by-provider-dead04faaf223840.yaml Co-authored-by: Rathish Cholarajan <rathishc24@gmail.com> * refactor test cases * remove print * add integration test Co-authored-by: Rathish Cholarajan <rathishc24@gmail.com> * Support pagination for retrieving runtime programs (Qiskit#170) * wip add limit/offset params * add reno * refactor & update test case * offset -> skip, implement refresh logic * refresh when skip/limit not default * Update releasenotes/notes/runtime-program-pagination-8d599ae984a5ce33.yaml Co-authored-by: Rathish Cholarajan <rathishc24@gmail.com> * Update releasenotes/notes/runtime-program-pagination-8d599ae984a5ce33.yaml Co-authored-by: Rathish Cholarajan <rathishc24@gmail.com> * add to test case * refactor refresh logic * refactor * fix lint * add integration test * update doc string * Update qiskit_ibm/api/clients/runtime.py Co-authored-by: Rathish Cholarajan <rathishc24@gmail.com> * cleanup merge * Apply suggestions from code review * fix lint refactor * Fetch all programs upfront 20 at a time and store in cache For subsequent requests paginate and return from cache * Fix integration test Co-authored-by: Rathish Cholarajan <rathishc24@gmail.com> Co-authored-by: Rathish Cholarajan <Rathish.C@ibm.com> Co-authored-by: Renier Morales <renier@users.noreply.github.com> Co-authored-by: Jessie Yu <jessieyu@us.ibm.com> Co-authored-by: Kevin Tian <kevin.tian@ibm.com>

max-reuter-2 added 3 commits June 24, 2019 16:41

Create jobs tests class, move some tests there

666d89b

Merge master into branch

d45d2da

Add to TestIBMQClientJobs class and tests

04ebfc8

diego-plan9 changed the title ~~[WIP] Increase coverage of IBMQClient tests~~ Increase coverage of IBMQClient tests Jun 25, 2019

diego-plan9 reviewed Jun 25, 2019

View reviewed changes

max-reuter-2 and others added 6 commits June 25, 2019 15:45

Remove some tests, reword some asserts

bd46b37

Merge branch 'master' into ibmqclient-tests

fb3129f

Merge remote-tracking branch 'upstream/master' into ibmqclient-tests

4f121f1

Update and revisit after merge

599c51b

Update names

a3234fc

Update tests

fd1ddad

diego-plan9 mentioned this pull request Jun 26, 2019

Revise test_get_job_includes and endpoint #133

Closed

4 tasks

diego-plan9 approved these changes Jun 26, 2019

View reviewed changes

diego-plan9 merged commit c36dc19 into Qiskit:master Jun 26, 2019

ryancocuzzo pushed a commit to ryancocuzzo/qiskit-ibmq-provider that referenced this pull request Jun 14, 2022

Bump version post release to 0.2.0 (Qiskit#188)

ad1f1dd

Conversation

max-reuter-2 commented Jun 24, 2019

Summary

Details and comments

Uh oh!

diego-plan9 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

diego-plan9 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants