added ibmqx_url as additional option to IBMQiskitDevice

[czachow]

fixes issue 38

[codecov]

Codecov Report

Merging #39 into master will decrease coverage by 3.83%.
The diff coverage is 14.28%.

@@            Coverage Diff             @@
##           master      #39      +/-   ##
==========================================
- Coverage   92.35%   88.52%   -3.84%     
==========================================
  Files           6        6              
  Lines         301      305       +4     
==========================================
- Hits          278      270       -8     
- Misses         23       35      +12

Impacted Files	Coverage Δ
pennylane_qiskit/devices.py	88% <14.28%> (-5.88%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 73cf9b5...687867a. Read the comment docs.

[josh146]

Thanks @czachow for the PR!

[quantshah]

Thank you @czachow for the quick fix. @josh146, some of the tests for the plugins do not make sense, should we exclude them somehow or still try to make dummy variables and tests e.g., device initializations?

[josh146]

some of the tests for the plugins do not make sense, should we exclude them somehow or still try to make dummy variables and tests e.g., device initializations?

What tests do you mean?

[quantshah]

Like a test to see if the plugin can connect to the IBM devices, e.g., passing of the url and token are working fine. In this PR, the test coverage decreases because we implement a kwarg for url but have no way of testing it. So my question is, can we write some test which makes sure that the plugin works well with the IBM API (with some dummy token and url). This way, if the IBM API changes (like the addition of a url option), these tests will fail and we will get to know immediately without having to wait for users to report it. But this will expose the token and API which seems like a security risk.

[quantshah]

Also, to get the codecov green light on this PR, we need to increase the test cov for the url option or disable testing of those lines. Any suggestions or preference on whether we should disable the lines or add some sort of dummy test?

[co9olguy]

@quantshah does it make any sense to mock out the IBM API? The test would just verify that we are passing on the required credentials and understand what to do with returned objects.

If not, then it likely makes more sense to disable testing of those lines

[josh146]

@co9olguy we had a quick chat with @ajavadia from Qiskit, it seems the best approach is to create a small integration test using an IBM API key (this is the approach they take in their CI as well). We can do this securely by encrypting it for use on Travis.

[co9olguy]

Sounds good, we can follow their lead 👍

[quantshah]

Could this be closed now that #44 is merged?

[josh146]

@czachow, thanks for your fix! This was included in PR #44, which necessitated a separate PR due to a large refactor of the code base. I will include your username in the changelog :)