Django command to post submissions to ora - #2001
Conversation
|
@cpennington @nedbat @wedaly please review. |
There was a problem hiding this comment.
The read_only=True should only be enabled with a command line argument. Will fix this.
|
I don't see any unit tests for this command or for the code you changed in |
There was a problem hiding this comment.
The current implementation of send_to_grader method always returns True, even if the XQueue interface reports an error. As far as I can tell, nothing in the system uses that return value. This might be a good time to update send_to_grader to return False on failure, then handle that condition explicitly here.
|
Thanks everyone for the detailed review and suggestions. I have made changes accordingly. Please have a look again. |
There was a problem hiding this comment.
@cpennington I have replaced get_current_task with get_task_at_index(task_index) and task_index can be specified as an argument to the commands. This will ensure that for multi-step problems the commands only operate on the intended step.
|
@wedaly I am already working on a separate branch for the fix to send_to_grader(). I am going to add some tests for the changes in /xmodule but I am not sure how to test the commands? |
|
@symbolist Create a test course with a I'd recommend patching / mocking the network call that posts the submission to verify it has the correct data. The easiest way to do this is to replace the xqueue interface object with a mock (in the runtime system passed to the open ended module). This will allow you to verify both success and failure conditions. There are examples of testing Django commands in the codebase: just look for "tests" folders inside "management/commands" packages. |
|
@wedaly I added some tests. Do they look okay? |
There was a problem hiding this comment.
I'm seeing that this line is uncovered by the tests. I'm not sure I understand when this line would be executed -- could you please clarify?
There was a problem hiding this comment.
When a student saves an answer (but not submits it) it is stored in task.stored_answer. But whether they have just looked at the problem or saved an answer the task_state == OpenEndedChild.INITIAL. Should I cover this too?
|
@symbolist The tests look good. 👍 when you make the style fix and cover the missing line with a test case. |
|
@cpennington I have refactored according to your suggestions. Can you give it a look? |
5 participants
No description provided.