Migrate from nose to pytest #24
Conversation
|
Thanks for the pull request, @ghassanmas! I've created OSPR-6010 to keep track of it in JIRA, where we prioritize reviews. Please note that it may take us up to several weeks or months to complete a review and merge your PR. Feel free to add as much of the following information to the ticket as you can:
All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here. Please let us know once your PR is ready for our review and all tests are green. |
There was a problem hiding this comment.
@ghassanmas Here are some quick thoughts.
Note that tests are failing, so we'll need to sort that out ;)
Also, I upgraded this repo to Github Actions (from Travis-CI) this morning, so that should get picked up when you rebase/push again.
|
@ghassanmas It looks like there are also references to |
|
@stvstnfrd I have resolved your comments, and sycned/merged with your latest push as well. |
|
The So to move forward I suppose we would either try to do a hacky thing, check this or run actual tests! |
|
@ghassanmas Thank you for your contribution. @stvstnfrd Do you plan to review this or should I reach out to Sarina about assigning this one? |
|
My pleasure @natabene :) |
| key_store = DictKeyValueStore() | ||
| field_data = KvsFieldData(key_store) | ||
| runtime = Runtime(services={'field-data': field_data}) | ||
| instace = InVideoQuizXBlock(runtime, scope_ids=Mock()) |
There was a problem hiding this comment.
nit: could this variable be named instance?
There was a problem hiding this comment.
I'd second this request, and even call it not a "nit"; misspelled words/variables should always be corrected ("nit" implies an optional request)
| from unittest.mock import Mock | ||
|
|
||
|
|
||
| def test_invideoquiz_view(): |
There was a problem hiding this comment.
nit: could you add a docstring that explains what is being tested?
| key_store = DictKeyValueStore() | ||
| field_data = KvsFieldData(key_store) | ||
| runtime = Runtime(services={'field-data': field_data}) | ||
| instace = InVideoQuizXBlock(runtime, scope_ids=Mock()) |
There was a problem hiding this comment.
I'd second this request, and even call it not a "nit"; misspelled words/variables should always be corrected ("nit" implies an optional request)
|
@ghassanmas this now looks good to me but I cannot merge until you resolve conflicts by rebasing, and you squash your commits and use conventional commits. https://open-edx-proposals.readthedocs.io/en/latest/oep-0051-bp-conventional-commits.html If you need help with these, please let us know. |
|
@ghassanmas Instructions to rebase and squash are here: https://github.com/edx/edx-platform/wiki/How-to-Rebase-a-Pull-Request |
this commit is part of part of supporting Django 3.2 upgrade, for more context: edx/upgrades#68, the changes are: 1. Removing nose as a tool for testing 2. Adding pytest as the alternative 3. Updating the requirments to reflect 1 and 2 4 Adding a test which tests the deafult value of the Xblock model
f9992b1 to
8204d8b
Compare
|
@sarina Thank you for the info, took little of time to get my head around rebase, gonna keep rewriting history from now on! 😁 |
|
@ghassanmas 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future. |
Fixes (Partially) : edx/upgrades#68