-
Notifications
You must be signed in to change notification settings - Fork 31
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use scripts instead of notebooks for tests #1239
Conversation
We also now see all the warnings when the LocalProvider tries to use unimplemented file methods : https://github.com/Qiskit-Extensions/quantum-serverless/actions/runs/8177774823/job/22360183087?pr=1239 |
Oh wow, good refactorization @psschwei . Is it ready for review or are you testing something more? |
Ready for review 😄 |
Do you update this pull request with the last change fixing the decorator, @psschwei ? I can take a look into this tomorrow 🙏 |
I'll rebase now |
Signed-off-by: Paul S. Schweigert <[email protected]>
Signed-off-by: Paul S. Schweigert <[email protected]>
Signed-off-by: Paul S. Schweigert <[email protected]>
Signed-off-by: Paul S. Schweigert <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea of the pull request. My only comment @psschwei is if you could check if we can remove nbmake
dependency with this from the requirements:
https://github.com/Qiskit-Extensions/quantum-serverless/blob/main/client/requirements-dev.txt#L20
I think we don't need it with these last changes but I'm not sure 😅
Signed-off-by: Paul S. Schweigert <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, @psschwei ! LGTM 😄
Summary
Run tests against python scripts instead of jupyter notebooks
Details and comments
There's two reasons for doing this:
Note that the decorators test fails (xref #1238)