From 5c55595e8bae9f77731e82a5063f1612895d90d8 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Wed, 8 May 2013 12:10:49 -0400 Subject: [PATCH 01/21] Start to add in some more open ended module tests --- common/lib/xmodule/xmodule/tests/__init__.py | 2 +- .../xmodule/tests/test_combined_open_ended.py | 63 ++++++++++++++++++- common/test/data/open_ended/README.md | 1 + common/test/data/open_ended/course.xml | 1 + .../test/data/open_ended/course/2012_Fall.xml | 5 ++ .../data/open_ended/policies/2012_Fall.json | 14 +++++ .../test/data/open_ended/roots/2012_Fall.xml | 1 + .../selfassessment/SampleQuestion.xml | 26 ++++---- 8 files changed, 98 insertions(+), 15 deletions(-) create mode 100644 common/test/data/open_ended/README.md create mode 100644 common/test/data/open_ended/course.xml create mode 100644 common/test/data/open_ended/course/2012_Fall.xml create mode 100644 common/test/data/open_ended/policies/2012_Fall.json create mode 100644 common/test/data/open_ended/roots/2012_Fall.xml diff --git a/common/lib/xmodule/xmodule/tests/__init__.py b/common/lib/xmodule/xmodule/tests/__init__.py index 1a10654f6c..59495048a1 100644 --- a/common/lib/xmodule/xmodule/tests/__init__.py +++ b/common/lib/xmodule/xmodule/tests/__init__.py @@ -20,7 +20,7 @@ from xmodule.x_module import ModuleSystem from mock import Mock open_ended_grading_interface = { - 'url': 'http://sandbox-grader-001.m.edx.org/peer_grading', + 'url': 'blah', 'username': 'incorrect_user', 'password': 'incorrect_pass', 'staff_grading' : 'staff_grading', diff --git a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py index 59f0e222ee..8e9e63b530 100644 --- a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py +++ b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py @@ -2,12 +2,19 @@ import json from mock import Mock, MagicMock, ANY import unittest +from fs.memoryfs import MemoryFS +from mock import patch + from xmodule.open_ended_grading_classes.openendedchild import OpenEndedChild from xmodule.open_ended_grading_classes.open_ended_module import OpenEndedModule from xmodule.open_ended_grading_classes.combined_open_ended_modulev1 import CombinedOpenEndedV1Module from xmodule.combined_open_ended_module import CombinedOpenEndedModule - +from xmodule.modulestore.django import modulestore from xmodule.modulestore import Location +from xmodule.modulestore.xml import ImportSystem, XMLModuleStore + +from xmodule.tests.test_export import DATA_DIR + from lxml import etree import capa.xqueue_interface as xqueue_interface from datetime import datetime @@ -17,8 +24,36 @@ log = logging.getLogger(__name__) from . import test_system +ORG = 'test_org' +COURSE = 'open_ended' # name of directory with course data + import test_util_open_ended +class DummySystem(ImportSystem): + + @patch('xmodule.modulestore.xml.OSFS', lambda dir: MemoryFS()) + def __init__(self, load_error_modules): + + xmlstore = XMLModuleStore("data_dir", course_dirs=[], load_error_modules=load_error_modules) + course_id = "/".join([ORG, COURSE, 'test_run']) + course_dir = "test_dir" + policy = {} + error_tracker = Mock() + parent_tracker = Mock() + + super(DummySystem, self).__init__( + xmlstore, + course_id, + course_dir, + policy, + error_tracker, + parent_tracker, + load_error_modules=load_error_modules, + ) + + def render_template(self, template, context): + raise Exception("Shouldn't be called") + """ Tests for the various pieces of the CombinedOpenEndedGrading system @@ -436,6 +471,9 @@ class CombinedOpenEndedModuleTest(unittest.TestCase): changed = combinedoe.update_task_states() self.assertFalse(changed) + def test_ajax_actions(self): + self.combinedoe_container.handle_ajax('save_answer', {'student_answer' : "This is my answer"}) + def test_get_score_realistic(self): instance_state = r"""{"ready_to_reset": false, "skip_spelling_checks": true, "current_task_number": 1, "weight": 5.0, "graceperiod": "1 day 12 hours 59 minutes 59 seconds", "is_graded": "True", "task_states": ["{\"child_created\": false, \"child_attempts\": 4, \"version\": 1, \"child_history\": [{\"answer\": \"The students\\u2019 data are recorded in the table below.\\r\\n\\r\\nStarting Mass (g)\\tEnding Mass (g)\\tDifference in Mass (g)\\r\\nMarble\\t 9.8\\t 9.4\\t\\u20130.4\\r\\nLimestone\\t10.4\\t 9.1\\t\\u20131.3\\r\\nWood\\t11.2\\t11.2\\t 0.0\\r\\nPlastic\\t 7.2\\t 7.1\\t\\u20130.1\\r\\nAfter reading the group\\u2019s procedure, describe what additional information you would need in order to replicate the expe\", \"post_assessment\": \"{\\\"submission_id\\\": 3097, \\\"score\\\": 0, \\\"feedback\\\": \\\"{\\\\\\\"spelling\\\\\\\": \\\\\\\"Spelling: Ok.\\\\\\\", \\\\\\\"grammar\\\\\\\": \\\\\\\"Grammar: More grammar errors than average.\\\\\\\", \\\\\\\"markup-text\\\\\\\": \\\\\\\"the students data are recorded in the table below . starting mass g ending mass g difference in mass g marble . . . limestone . . . wood . . . plastic . . . after reading the groups procedure , describe what additional information you would need in order to replicate the expe\\\\\\\"}\\\", \\\"success\\\": true, \\\"grader_id\\\": 3233, \\\"grader_type\\\": \\\"ML\\\", \\\"rubric_scores_complete\\\": true, \\\"rubric_xml\\\": \\\"Response Quality0\\\"}\", \"score\": 0}, {\"answer\": \"After 24 hours, remove the samples from the containers and rinse each sample with distilled water.\\r\\nAllow the samples to sit and dry for 30 minutes.\\r\\nDetermine the mass of each sample.\\r\\nThe students\\u2019 data are recorded in the table below.\\r\\n\\r\\nStarting Mass (g)\\tEnding Mass (g)\\tDifference in Mass (g)\\r\\nMarble\\t 9.8\\t 9.4\\t\\u20130.4\\r\\nLimestone\\t10.4\\t 9.1\\t\\u20131.3\\r\\nWood\\t11.2\\t11.2\\t 0.0\\r\\nPlastic\\t 7.2\\t 7.1\\t\\u20130.1\\r\\nAfter reading the\", \"post_assessment\": \"[3]\", \"score\": 3}, {\"answer\": \"To replicate the experiment, the procedure would require more detail. One piece of information that is omitted is the amount of vinegar used in the experiment. It is also important to know what temperature the experiment was kept at during the 24 hours. Finally, the procedure needs to include details about the experiment, for example if the whole sample must be submerged.\", \"post_assessment\": \"[3]\", \"score\": 3}, {\"answer\": \"e the mass of four different samples.\\r\\nPour vinegar in each of four separate, but identical, containers.\\r\\nPlace a sample of one material into one container and label. Repeat with remaining samples, placing a single sample into a single container.\\r\\nAfter 24 hours, remove the samples from the containers and rinse each sample with distilled water.\\r\\nAllow the samples to sit and dry for 30 minutes.\\r\\nDetermine the mass of each sample.\\r\\nThe students\\u2019 data are recorded in the table below.\\r\\n\", \"post_assessment\": \"[3]\", \"score\": 3}, {\"answer\": \"\", \"post_assessment\": \"[3]\", \"score\": 3}], \"max_score\": 3, \"child_state\": \"done\"}", "{\"child_created\": false, \"child_attempts\": 0, \"version\": 1, \"child_history\": [{\"answer\": \"The students\\u2019 data are recorded in the table below.\\r\\n\\r\\nStarting Mass (g)\\tEnding Mass (g)\\tDifference in Mass (g)\\r\\nMarble\\t 9.8\\t 9.4\\t\\u20130.4\\r\\nLimestone\\t10.4\\t 9.1\\t\\u20131.3\\r\\nWood\\t11.2\\t11.2\\t 0.0\\r\\nPlastic\\t 7.2\\t 7.1\\t\\u20130.1\\r\\nAfter reading the group\\u2019s procedure, describe what additional information you would need in order to replicate the expe\", \"post_assessment\": \"{\\\"submission_id\\\": 3097, \\\"score\\\": 0, \\\"feedback\\\": \\\"{\\\\\\\"spelling\\\\\\\": \\\\\\\"Spelling: Ok.\\\\\\\", \\\\\\\"grammar\\\\\\\": \\\\\\\"Grammar: More grammar errors than average.\\\\\\\", \\\\\\\"markup-text\\\\\\\": \\\\\\\"the students data are recorded in the table below . starting mass g ending mass g difference in mass g marble . . . limestone . . . wood . . . plastic . . . after reading the groups procedure , describe what additional information you would need in order to replicate the expe\\\\\\\"}\\\", \\\"success\\\": true, \\\"grader_id\\\": 3233, \\\"grader_type\\\": \\\"ML\\\", \\\"rubric_scores_complete\\\": true, \\\"rubric_xml\\\": \\\"Response Quality0\\\"}\", \"score\": 0}, {\"answer\": \"After 24 hours, remove the samples from the containers and rinse each sample with distilled water.\\r\\nAllow the samples to sit and dry for 30 minutes.\\r\\nDetermine the mass of each sample.\\r\\nThe students\\u2019 data are recorded in the table below.\\r\\n\\r\\nStarting Mass (g)\\tEnding Mass (g)\\tDifference in Mass (g)\\r\\nMarble\\t 9.8\\t 9.4\\t\\u20130.4\\r\\nLimestone\\t10.4\\t 9.1\\t\\u20131.3\\r\\nWood\\t11.2\\t11.2\\t 0.0\\r\\nPlastic\\t 7.2\\t 7.1\\t\\u20130.1\\r\\nAfter reading the\", \"post_assessment\": \"{\\\"submission_id\\\": 3098, \\\"score\\\": 0, \\\"feedback\\\": \\\"{\\\\\\\"spelling\\\\\\\": \\\\\\\"Spelling: Ok.\\\\\\\", \\\\\\\"grammar\\\\\\\": \\\\\\\"Grammar: Ok.\\\\\\\", \\\\\\\"markup-text\\\\\\\": \\\\\\\"after hours , remove the samples from the containers and rinse each sample with distilled water . allow the samples to sit and dry for minutes . determine the mass of each sample . the students data are recorded in the table below . starting mass g ending mass g difference in mass g marble . . . limestone . . . wood . . . plastic . . . after reading the\\\\\\\"}\\\", \\\"success\\\": true, \\\"grader_id\\\": 3235, \\\"grader_type\\\": \\\"ML\\\", \\\"rubric_scores_complete\\\": true, \\\"rubric_xml\\\": \\\"Response Quality0\\\"}\", \"score\": 0}, {\"answer\": \"To replicate the experiment, the procedure would require more detail. One piece of information that is omitted is the amount of vinegar used in the experiment. It is also important to know what temperature the experiment was kept at during the 24 hours. Finally, the procedure needs to include details about the experiment, for example if the whole sample must be submerged.\", \"post_assessment\": \"{\\\"submission_id\\\": 3099, \\\"score\\\": 3, \\\"feedback\\\": \\\"{\\\\\\\"spelling\\\\\\\": \\\\\\\"Spelling: Ok.\\\\\\\", \\\\\\\"grammar\\\\\\\": \\\\\\\"Grammar: Ok.\\\\\\\", \\\\\\\"markup-text\\\\\\\": \\\\\\\"to replicate the experiment , the procedure would require more detail . one piece of information that is omitted is the amount of vinegar used in the experiment . it is also important to know what temperature the experiment was kept at during the hours . finally , the procedure needs to include details about the experiment , for example if the whole sample must be submerged .\\\\\\\"}\\\", \\\"success\\\": true, \\\"grader_id\\\": 3237, \\\"grader_type\\\": \\\"ML\\\", \\\"rubric_scores_complete\\\": true, \\\"rubric_xml\\\": \\\"Response Quality3\\\"}\", \"score\": 3}, {\"answer\": \"e the mass of four different samples.\\r\\nPour vinegar in each of four separate, but identical, containers.\\r\\nPlace a sample of one material into one container and label. Repeat with remaining samples, placing a single sample into a single container.\\r\\nAfter 24 hours, remove the samples from the containers and rinse each sample with distilled water.\\r\\nAllow the samples to sit and dry for 30 minutes.\\r\\nDetermine the mass of each sample.\\r\\nThe students\\u2019 data are recorded in the table below.\\r\\n\", \"post_assessment\": \"{\\\"submission_id\\\": 3100, \\\"score\\\": 0, \\\"feedback\\\": \\\"{\\\\\\\"spelling\\\\\\\": \\\\\\\"Spelling: Ok.\\\\\\\", \\\\\\\"grammar\\\\\\\": \\\\\\\"Grammar: Ok.\\\\\\\", \\\\\\\"markup-text\\\\\\\": \\\\\\\"e the mass of four different samples . pour vinegar in each of four separate , but identical , containers . place a sample of one material into one container and label . repeat with remaining samples , placing a single sample into a single container . after hours , remove the samples from the containers and rinse each sample with distilled water . allow the samples to sit and dry for minutes . determine the mass of each sample . the students data are recorded in the table below . \\\\\\\"}\\\", \\\"success\\\": true, \\\"grader_id\\\": 3239, \\\"grader_type\\\": \\\"ML\\\", \\\"rubric_scores_complete\\\": true, \\\"rubric_xml\\\": \\\"Response Quality0\\\"}\", \"score\": 0}, {\"answer\": \"\", \"post_assessment\": \"{\\\"submission_id\\\": 3101, \\\"score\\\": 0, \\\"feedback\\\": \\\"{\\\\\\\"spelling\\\\\\\": \\\\\\\"Spelling: Ok.\\\\\\\", \\\\\\\"grammar\\\\\\\": \\\\\\\"Grammar: Ok.\\\\\\\", \\\\\\\"markup-text\\\\\\\": \\\\\\\"invalid essay .\\\\\\\"}\\\", \\\"success\\\": true, \\\"grader_id\\\": 3241, \\\"grader_type\\\": \\\"ML\\\", \\\"rubric_scores_complete\\\": true, \\\"rubric_xml\\\": \\\"Response Quality0\\\"}\", \"score\": 0}], \"max_score\": 3, \"child_state\": \"done\"}"], "attempts": "10000", "student_attempts": 0, "due": null, "state": "done", "accept_file_upload": false, "display_name": "Science Question -- Machine Assessed"}""" instance_state = json.loads(instance_state) @@ -466,6 +504,29 @@ class CombinedOpenEndedModuleTest(unittest.TestCase): self.assertEqual(score_dict['score'], 15.0) self.assertEqual(score_dict['total'], 15.0) +class OpenEndedModuleXmlTest(unittest.TestCase): + def setUp(self): + self.test_system = test_system() + + @staticmethod + def get_import_system(load_error_modules=True): + '''Get a dummy system''' + return DummySystem(load_error_modules) + + def get_course(self, name): + """Get a test course by directory name. If there's more than one, error.""" + print "Importing {0}".format(name) + + modulestore = XMLModuleStore(DATA_DIR, course_dirs=[name]) + courses = modulestore.get_courses() + self.modulestore = modulestore + self.assertEquals(len(courses), 1) + return courses[0] + + def test_open_ended_load(self): + course = self.get_course('open_ended') + log.info(course.id) + diff --git a/common/test/data/open_ended/README.md b/common/test/data/open_ended/README.md new file mode 100644 index 0000000000..7fe58ac17f --- /dev/null +++ b/common/test/data/open_ended/README.md @@ -0,0 +1 @@ +This is a very very simple course, useful for debugging self assessment code. diff --git a/common/test/data/open_ended/course.xml b/common/test/data/open_ended/course.xml new file mode 100644 index 0000000000..ea7d5c420d --- /dev/null +++ b/common/test/data/open_ended/course.xml @@ -0,0 +1 @@ + diff --git a/common/test/data/open_ended/course/2012_Fall.xml b/common/test/data/open_ended/course/2012_Fall.xml new file mode 100644 index 0000000000..f2d16488a7 --- /dev/null +++ b/common/test/data/open_ended/course/2012_Fall.xml @@ -0,0 +1,5 @@ + + + + + diff --git a/common/test/data/open_ended/policies/2012_Fall.json b/common/test/data/open_ended/policies/2012_Fall.json new file mode 100644 index 0000000000..09b68ab400 --- /dev/null +++ b/common/test/data/open_ended/policies/2012_Fall.json @@ -0,0 +1,14 @@ +{ + "course/2012_Fall": { + "graceperiod": "2 days 5 hours 59 minutes 59 seconds", + "start": "2015-07-17T12:00", + "display_name": "Self Assessment Test", + "graded": "true" + }, + "chapter/Overview": { + "display_name": "Overview" + }, + "combinedopenended/SampleQuestion": { + "display_name": "Sample Question", + }, +} diff --git a/common/test/data/open_ended/roots/2012_Fall.xml b/common/test/data/open_ended/roots/2012_Fall.xml new file mode 100644 index 0000000000..ea7d5c420d --- /dev/null +++ b/common/test/data/open_ended/roots/2012_Fall.xml @@ -0,0 +1 @@ + diff --git a/common/test/data/self_assessment/selfassessment/SampleQuestion.xml b/common/test/data/self_assessment/selfassessment/SampleQuestion.xml index 6c383763b1..f8affa903d 100644 --- a/common/test/data/self_assessment/selfassessment/SampleQuestion.xml +++ b/common/test/data/self_assessment/selfassessment/SampleQuestion.xml @@ -1,14 +1,14 @@ - - What is the meaning of life? - - - This is a rubric. - - - Thanks for your submission! - - - Enter a hint below: - - + + What is the meaning of life? + + + This is a rubric. + + + Thanks for your submission! + + + Enter a hint below: + + \ No newline at end of file From 1d53625673a33dec59fc9bd2558936517bfec350 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Wed, 8 May 2013 15:37:25 -0400 Subject: [PATCH 02/21] Add in ability to mock a server, a lot more testing code for open ended --- common/lib/xmodule/xmodule/tests/__init__.py | 2 +- .../lib/xmodule/xmodule/tests/mock_server.py | 34 +++++++++ .../xmodule/tests/test_combined_open_ended.py | 75 +++++++++++++++++-- common/test/data/open_ended/README.md | 2 +- .../combinedopenended/SampleQuestion.xml | 33 ++++++++ common/test/data/open_ended/course.xml | 2 +- 6 files changed, 140 insertions(+), 8 deletions(-) create mode 100644 common/lib/xmodule/xmodule/tests/mock_server.py create mode 100644 common/test/data/open_ended/combinedopenended/SampleQuestion.xml diff --git a/common/lib/xmodule/xmodule/tests/__init__.py b/common/lib/xmodule/xmodule/tests/__init__.py index 59495048a1..b58308252e 100644 --- a/common/lib/xmodule/xmodule/tests/__init__.py +++ b/common/lib/xmodule/xmodule/tests/__init__.py @@ -20,7 +20,7 @@ from xmodule.x_module import ModuleSystem from mock import Mock open_ended_grading_interface = { - 'url': 'blah', + 'url': 'blah/', 'username': 'incorrect_user', 'password': 'incorrect_pass', 'staff_grading' : 'staff_grading', diff --git a/common/lib/xmodule/xmodule/tests/mock_server.py b/common/lib/xmodule/xmodule/tests/mock_server.py new file mode 100644 index 0000000000..d8ae2f9124 --- /dev/null +++ b/common/lib/xmodule/xmodule/tests/mock_server.py @@ -0,0 +1,34 @@ +from threading import Thread +import socket +import threading + +import SimpleHTTPServer +import SocketServer + +class ThreadedRequestHandler(SimpleHTTPServer.SimpleHTTPRequestHandler): + def handle(self): + data = self.request.recv(1024) + cur_thread = threading.current_thread() + response = "{}: {}".format(cur_thread.name, data) + self.request.sendall(response) + return + +class ThreadedTCPServer(SocketServer.ThreadingMixIn, SocketServer.TCPServer): + pass + +def create_server(host,port): + """ + Mock a server to be used for the open ended grading tests + @param host: the hostname ie "localhost" or "127.0.0.1" + @param port: the integer of the port to open a connection on + @return: The created server object + """ + server = ThreadedTCPServer((host,port), ThreadedRequestHandler) + + # Start a thread with the server -- that thread will then start one + # more thread for each request + server_thread = threading.Thread(target=server.serve_forever) + # Exit the server thread when the main thread terminates + server_thread.daemon = True + server_thread.start() + return server \ No newline at end of file diff --git a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py index 8e9e63b530..0f7db73db4 100644 --- a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py +++ b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py @@ -20,6 +20,8 @@ import capa.xqueue_interface as xqueue_interface from datetime import datetime import logging +from mock_server import create_server + log = logging.getLogger(__name__) from . import test_system @@ -29,6 +31,18 @@ COURSE = 'open_ended' # name of directory with course data import test_util_open_ended +class MockQueryDict(dict): + """ + Mock a query set so that it can be used with default authorization + """ + def getlist(self, key, default=None): + try: + return super(MockQueryDict, self).__getitem__(key) + except KeyError: + if default is None: + return [] + return default + class DummySystem(ImportSystem): @patch('xmodule.modulestore.xml.OSFS', lambda dir: MemoryFS()) @@ -471,9 +485,6 @@ class CombinedOpenEndedModuleTest(unittest.TestCase): changed = combinedoe.update_task_states() self.assertFalse(changed) - def test_ajax_actions(self): - self.combinedoe_container.handle_ajax('save_answer', {'student_answer' : "This is my answer"}) - def test_get_score_realistic(self): instance_state = r"""{"ready_to_reset": false, "skip_spelling_checks": true, "current_task_number": 1, "weight": 5.0, "graceperiod": "1 day 12 hours 59 minutes 59 seconds", "is_graded": "True", "task_states": ["{\"child_created\": false, \"child_attempts\": 4, \"version\": 1, \"child_history\": [{\"answer\": \"The students\\u2019 data are recorded in the table below.\\r\\n\\r\\nStarting Mass (g)\\tEnding Mass (g)\\tDifference in Mass (g)\\r\\nMarble\\t 9.8\\t 9.4\\t\\u20130.4\\r\\nLimestone\\t10.4\\t 9.1\\t\\u20131.3\\r\\nWood\\t11.2\\t11.2\\t 0.0\\r\\nPlastic\\t 7.2\\t 7.1\\t\\u20130.1\\r\\nAfter reading the group\\u2019s procedure, describe what additional information you would need in order to replicate the expe\", \"post_assessment\": \"{\\\"submission_id\\\": 3097, \\\"score\\\": 0, \\\"feedback\\\": \\\"{\\\\\\\"spelling\\\\\\\": \\\\\\\"Spelling: Ok.\\\\\\\", \\\\\\\"grammar\\\\\\\": \\\\\\\"Grammar: More grammar errors than average.\\\\\\\", \\\\\\\"markup-text\\\\\\\": \\\\\\\"the students data are recorded in the table below . starting mass g ending mass g difference in mass g marble . . . limestone . . . wood . . . plastic . . . after reading the groups procedure , describe what additional information you would need in order to replicate the expe\\\\\\\"}\\\", \\\"success\\\": true, \\\"grader_id\\\": 3233, \\\"grader_type\\\": \\\"ML\\\", \\\"rubric_scores_complete\\\": true, \\\"rubric_xml\\\": \\\"Response Quality0\\\"}\", \"score\": 0}, {\"answer\": \"After 24 hours, remove the samples from the containers and rinse each sample with distilled water.\\r\\nAllow the samples to sit and dry for 30 minutes.\\r\\nDetermine the mass of each sample.\\r\\nThe students\\u2019 data are recorded in the table below.\\r\\n\\r\\nStarting Mass (g)\\tEnding Mass (g)\\tDifference in Mass (g)\\r\\nMarble\\t 9.8\\t 9.4\\t\\u20130.4\\r\\nLimestone\\t10.4\\t 9.1\\t\\u20131.3\\r\\nWood\\t11.2\\t11.2\\t 0.0\\r\\nPlastic\\t 7.2\\t 7.1\\t\\u20130.1\\r\\nAfter reading the\", \"post_assessment\": \"[3]\", \"score\": 3}, {\"answer\": \"To replicate the experiment, the procedure would require more detail. One piece of information that is omitted is the amount of vinegar used in the experiment. It is also important to know what temperature the experiment was kept at during the 24 hours. Finally, the procedure needs to include details about the experiment, for example if the whole sample must be submerged.\", \"post_assessment\": \"[3]\", \"score\": 3}, {\"answer\": \"e the mass of four different samples.\\r\\nPour vinegar in each of four separate, but identical, containers.\\r\\nPlace a sample of one material into one container and label. Repeat with remaining samples, placing a single sample into a single container.\\r\\nAfter 24 hours, remove the samples from the containers and rinse each sample with distilled water.\\r\\nAllow the samples to sit and dry for 30 minutes.\\r\\nDetermine the mass of each sample.\\r\\nThe students\\u2019 data are recorded in the table below.\\r\\n\", \"post_assessment\": \"[3]\", \"score\": 3}, {\"answer\": \"\", \"post_assessment\": \"[3]\", \"score\": 3}], \"max_score\": 3, \"child_state\": \"done\"}", "{\"child_created\": false, \"child_attempts\": 0, \"version\": 1, \"child_history\": [{\"answer\": \"The students\\u2019 data are recorded in the table below.\\r\\n\\r\\nStarting Mass (g)\\tEnding Mass (g)\\tDifference in Mass (g)\\r\\nMarble\\t 9.8\\t 9.4\\t\\u20130.4\\r\\nLimestone\\t10.4\\t 9.1\\t\\u20131.3\\r\\nWood\\t11.2\\t11.2\\t 0.0\\r\\nPlastic\\t 7.2\\t 7.1\\t\\u20130.1\\r\\nAfter reading the group\\u2019s procedure, describe what additional information you would need in order to replicate the expe\", \"post_assessment\": \"{\\\"submission_id\\\": 3097, \\\"score\\\": 0, \\\"feedback\\\": \\\"{\\\\\\\"spelling\\\\\\\": \\\\\\\"Spelling: Ok.\\\\\\\", \\\\\\\"grammar\\\\\\\": \\\\\\\"Grammar: More grammar errors than average.\\\\\\\", \\\\\\\"markup-text\\\\\\\": \\\\\\\"the students data are recorded in the table below . starting mass g ending mass g difference in mass g marble . . . limestone . . . wood . . . plastic . . . after reading the groups procedure , describe what additional information you would need in order to replicate the expe\\\\\\\"}\\\", \\\"success\\\": true, \\\"grader_id\\\": 3233, \\\"grader_type\\\": \\\"ML\\\", \\\"rubric_scores_complete\\\": true, \\\"rubric_xml\\\": \\\"Response Quality0\\\"}\", \"score\": 0}, {\"answer\": \"After 24 hours, remove the samples from the containers and rinse each sample with distilled water.\\r\\nAllow the samples to sit and dry for 30 minutes.\\r\\nDetermine the mass of each sample.\\r\\nThe students\\u2019 data are recorded in the table below.\\r\\n\\r\\nStarting Mass (g)\\tEnding Mass (g)\\tDifference in Mass (g)\\r\\nMarble\\t 9.8\\t 9.4\\t\\u20130.4\\r\\nLimestone\\t10.4\\t 9.1\\t\\u20131.3\\r\\nWood\\t11.2\\t11.2\\t 0.0\\r\\nPlastic\\t 7.2\\t 7.1\\t\\u20130.1\\r\\nAfter reading the\", \"post_assessment\": \"{\\\"submission_id\\\": 3098, \\\"score\\\": 0, \\\"feedback\\\": \\\"{\\\\\\\"spelling\\\\\\\": \\\\\\\"Spelling: Ok.\\\\\\\", \\\\\\\"grammar\\\\\\\": \\\\\\\"Grammar: Ok.\\\\\\\", \\\\\\\"markup-text\\\\\\\": \\\\\\\"after hours , remove the samples from the containers and rinse each sample with distilled water . allow the samples to sit and dry for minutes . determine the mass of each sample . the students data are recorded in the table below . starting mass g ending mass g difference in mass g marble . . . limestone . . . wood . . . plastic . . . after reading the\\\\\\\"}\\\", \\\"success\\\": true, \\\"grader_id\\\": 3235, \\\"grader_type\\\": \\\"ML\\\", \\\"rubric_scores_complete\\\": true, \\\"rubric_xml\\\": \\\"Response Quality0\\\"}\", \"score\": 0}, {\"answer\": \"To replicate the experiment, the procedure would require more detail. One piece of information that is omitted is the amount of vinegar used in the experiment. It is also important to know what temperature the experiment was kept at during the 24 hours. Finally, the procedure needs to include details about the experiment, for example if the whole sample must be submerged.\", \"post_assessment\": \"{\\\"submission_id\\\": 3099, \\\"score\\\": 3, \\\"feedback\\\": \\\"{\\\\\\\"spelling\\\\\\\": \\\\\\\"Spelling: Ok.\\\\\\\", \\\\\\\"grammar\\\\\\\": \\\\\\\"Grammar: Ok.\\\\\\\", \\\\\\\"markup-text\\\\\\\": \\\\\\\"to replicate the experiment , the procedure would require more detail . one piece of information that is omitted is the amount of vinegar used in the experiment . it is also important to know what temperature the experiment was kept at during the hours . finally , the procedure needs to include details about the experiment , for example if the whole sample must be submerged .\\\\\\\"}\\\", \\\"success\\\": true, \\\"grader_id\\\": 3237, \\\"grader_type\\\": \\\"ML\\\", \\\"rubric_scores_complete\\\": true, \\\"rubric_xml\\\": \\\"Response Quality3\\\"}\", \"score\": 3}, {\"answer\": \"e the mass of four different samples.\\r\\nPour vinegar in each of four separate, but identical, containers.\\r\\nPlace a sample of one material into one container and label. Repeat with remaining samples, placing a single sample into a single container.\\r\\nAfter 24 hours, remove the samples from the containers and rinse each sample with distilled water.\\r\\nAllow the samples to sit and dry for 30 minutes.\\r\\nDetermine the mass of each sample.\\r\\nThe students\\u2019 data are recorded in the table below.\\r\\n\", \"post_assessment\": \"{\\\"submission_id\\\": 3100, \\\"score\\\": 0, \\\"feedback\\\": \\\"{\\\\\\\"spelling\\\\\\\": \\\\\\\"Spelling: Ok.\\\\\\\", \\\\\\\"grammar\\\\\\\": \\\\\\\"Grammar: Ok.\\\\\\\", \\\\\\\"markup-text\\\\\\\": \\\\\\\"e the mass of four different samples . pour vinegar in each of four separate , but identical , containers . place a sample of one material into one container and label . repeat with remaining samples , placing a single sample into a single container . after hours , remove the samples from the containers and rinse each sample with distilled water . allow the samples to sit and dry for minutes . determine the mass of each sample . the students data are recorded in the table below . \\\\\\\"}\\\", \\\"success\\\": true, \\\"grader_id\\\": 3239, \\\"grader_type\\\": \\\"ML\\\", \\\"rubric_scores_complete\\\": true, \\\"rubric_xml\\\": \\\"Response Quality0\\\"}\", \"score\": 0}, {\"answer\": \"\", \"post_assessment\": \"{\\\"submission_id\\\": 3101, \\\"score\\\": 0, \\\"feedback\\\": \\\"{\\\\\\\"spelling\\\\\\\": \\\\\\\"Spelling: Ok.\\\\\\\", \\\\\\\"grammar\\\\\\\": \\\\\\\"Grammar: Ok.\\\\\\\", \\\\\\\"markup-text\\\\\\\": \\\\\\\"invalid essay .\\\\\\\"}\\\", \\\"success\\\": true, \\\"grader_id\\\": 3241, \\\"grader_type\\\": \\\"ML\\\", \\\"rubric_scores_complete\\\": true, \\\"rubric_xml\\\": \\\"Response Quality0\\\"}\", \"score\": 0}], \"max_score\": 3, \"child_state\": \"done\"}"], "attempts": "10000", "student_attempts": 0, "due": null, "state": "done", "accept_file_upload": false, "display_name": "Science Question -- Machine Assessed"}""" instance_state = json.loads(instance_state) @@ -505,6 +516,11 @@ class CombinedOpenEndedModuleTest(unittest.TestCase): self.assertEqual(score_dict['total'], 15.0) class OpenEndedModuleXmlTest(unittest.TestCase): + problem_location = Location(["i4x", "edX", "oe_test", "combinedopenended", "SampleQuestion"]) + answer = "blah blah" + assessment = [0,1] + hint = "blah" + test_server = create_server("127.0.0.1", 3034) def setUp(self): self.test_system = test_system() @@ -523,10 +539,59 @@ class OpenEndedModuleXmlTest(unittest.TestCase): self.assertEquals(len(courses), 1) return courses[0] - def test_open_ended_load(self): + def get_module_from_location(self, location): course = self.get_course('open_ended') - log.info(course.id) + if not isinstance(location, Location): + location = Location(location) + descriptor = self.modulestore.get_instance(course.id, location, depth=None) + return descriptor.xmodule(self.test_system) + def test_open_ended_load_and_save(self): + module = self.get_module_from_location(self.problem_location) + module.handle_ajax("save_answer", {"student_answer" : self.answer}) + task_one_json = json.loads(module.task_states[0]) + self.assertEqual(task_one_json['child_history'][0]['answer'], self.answer) + def test_open_ended_flow_reset(self): + assessment = [0,1] + module = self.get_module_from_location(self.problem_location) + #Simulate a student saving an answer + module.handle_ajax("save_answer", {"student_answer" : self.answer}) + status = module.handle_ajax("get_status", {}) + #Mock a student submitting an assessment + assessment_dict = MockQueryDict() + assessment_dict.update({'assessment' : sum(assessment), 'score_list[]' : assessment}) + module.handle_ajax("save_assessment", assessment_dict) + task_one_json = json.loads(module.task_states[0]) + self.assertEqual(json.loads(task_one_json['child_history'][0]['post_assessment']), assessment) + module.handle_ajax("get_status", {}) + + #Move to the next step in the problem + module.handle_ajax("next_problem", {}) + module.get_html() + module.handle_ajax("get_combined_rubric", {}) + + module.handle_ajax("reset", {}) + + def test_open_ended_flow_correct(self): + assessment = [1,1] + module = self.get_module_from_location(self.problem_location) + + #Simulate a student saving an answer + module.handle_ajax("save_answer", {"student_answer" : self.answer}) + status = module.handle_ajax("get_status", {}) + + #Mock a student submitting an assessment + assessment_dict = MockQueryDict() + assessment_dict.update({'assessment' : sum(assessment), 'score_list[]' : assessment}) + module.handle_ajax("save_assessment", assessment_dict) + task_one_json = json.loads(module.task_states[0]) + self.assertEqual(json.loads(task_one_json['child_history'][0]['post_assessment']), assessment) + module.handle_ajax("get_status", {}) + + #Move to the next step in the problem + module.handle_ajax("next_problem", {}) + module.get_html() + module.handle_ajax("get_combined_rubric", {}) diff --git a/common/test/data/open_ended/README.md b/common/test/data/open_ended/README.md index 7fe58ac17f..ed1d5c771d 100644 --- a/common/test/data/open_ended/README.md +++ b/common/test/data/open_ended/README.md @@ -1 +1 @@ -This is a very very simple course, useful for debugging self assessment code. +This is a very very simple course, useful for debugging open ended grading code. diff --git a/common/test/data/open_ended/combinedopenended/SampleQuestion.xml b/common/test/data/open_ended/combinedopenended/SampleQuestion.xml new file mode 100644 index 0000000000..5dbe285526 --- /dev/null +++ b/common/test/data/open_ended/combinedopenended/SampleQuestion.xml @@ -0,0 +1,33 @@ + + + + + Writing Applications + + + + + Language Conventions + + + + + + +

Censorship in the Libraries

+

"All of us can think of a book that we hope none of our children or any other children have taken off the shelf. But if I have the right to remove that book from the shelf -- that work I abhor -- then you also have exactly the same right and so does everyone else. And then we have no books left on the shelf for any of us." --Katherine Paterson, Author

+

Write a persuasive essay to a newspaper reflecting your vies on censorship in libraries. Do you believe that certain materials, such as books, music, movies, magazines, etc., should be removed from the shelves if they are found offensive? Support your position with convincing arguments from your own experience, observations, and/or reading.

+
+ + + + + + + Enter essay here. + This is the answer. + {"grader_settings" : "ml_grading.conf", "problem_id" : "6.002x/Welcome/OETest"} + + + +
\ No newline at end of file diff --git a/common/test/data/open_ended/course.xml b/common/test/data/open_ended/course.xml index ea7d5c420d..bf3ed687fb 100644 --- a/common/test/data/open_ended/course.xml +++ b/common/test/data/open_ended/course.xml @@ -1 +1 @@ - + From 3801f574a4f230fbd6ccb3e4a6d8aef2ffb23b79 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Wed, 8 May 2013 16:00:07 -0400 Subject: [PATCH 03/21] Add in xqueue submission tests --- .../open_ended_module.py | 2 +- common/lib/xmodule/xmodule/tests/__init__.py | 2 +- .../lib/xmodule/xmodule/tests/mock_server.py | 34 ------------------- .../xmodule/tests/test_combined_open_ended.py | 21 +++++++++--- 4 files changed, 19 insertions(+), 40 deletions(-) delete mode 100644 common/lib/xmodule/xmodule/tests/mock_server.py diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py index 8373700837..4a8604ac30 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py @@ -243,7 +243,7 @@ class OpenEndedModule(openendedchild.OpenEndedChild): }) # Submit request. When successful, 'msg' is the prior length of the queue - (error, msg) = qinterface.send_to_queue(header=xheader, + qinterface.send_to_queue(header=xheader, body=json.dumps(contents)) # State associated with the queueing request diff --git a/common/lib/xmodule/xmodule/tests/__init__.py b/common/lib/xmodule/xmodule/tests/__init__.py index b58308252e..0a2f22aa68 100644 --- a/common/lib/xmodule/xmodule/tests/__init__.py +++ b/common/lib/xmodule/xmodule/tests/__init__.py @@ -52,7 +52,7 @@ def test_system(): user=Mock(is_staff=False), filestore=Mock(), debug=True, - xqueue={'interface': None, 'callback_url': '/', 'default_queuename': 'testqueue', 'waittime': 10}, + xqueue={'interface': None, 'callback_url': '/', 'default_queuename': 'testqueue', 'waittime': 10, 'construct_callback' : Mock(side_effect="/")}, node_path=os.environ.get("NODE_PATH", "/usr/local/lib/node_modules"), xblock_model_data=lambda descriptor: descriptor._model_data, anonymous_student_id='student', diff --git a/common/lib/xmodule/xmodule/tests/mock_server.py b/common/lib/xmodule/xmodule/tests/mock_server.py deleted file mode 100644 index d8ae2f9124..0000000000 --- a/common/lib/xmodule/xmodule/tests/mock_server.py +++ /dev/null @@ -1,34 +0,0 @@ -from threading import Thread -import socket -import threading - -import SimpleHTTPServer -import SocketServer - -class ThreadedRequestHandler(SimpleHTTPServer.SimpleHTTPRequestHandler): - def handle(self): - data = self.request.recv(1024) - cur_thread = threading.current_thread() - response = "{}: {}".format(cur_thread.name, data) - self.request.sendall(response) - return - -class ThreadedTCPServer(SocketServer.ThreadingMixIn, SocketServer.TCPServer): - pass - -def create_server(host,port): - """ - Mock a server to be used for the open ended grading tests - @param host: the hostname ie "localhost" or "127.0.0.1" - @param port: the integer of the port to open a connection on - @return: The created server object - """ - server = ThreadedTCPServer((host,port), ThreadedRequestHandler) - - # Start a thread with the server -- that thread will then start one - # more thread for each request - server_thread = threading.Thread(target=server.serve_forever) - # Exit the server thread when the main thread terminates - server_thread.daemon = True - server_thread.start() - return server \ No newline at end of file diff --git a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py index 0f7db73db4..a8127eec30 100644 --- a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py +++ b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py @@ -8,6 +8,7 @@ from mock import patch from xmodule.open_ended_grading_classes.openendedchild import OpenEndedChild from xmodule.open_ended_grading_classes.open_ended_module import OpenEndedModule from xmodule.open_ended_grading_classes.combined_open_ended_modulev1 import CombinedOpenEndedV1Module +from xmodule.open_ended_grading_classes.grading_service_module import GradingServiceError from xmodule.combined_open_ended_module import CombinedOpenEndedModule from xmodule.modulestore.django import modulestore from xmodule.modulestore import Location @@ -520,9 +521,11 @@ class OpenEndedModuleXmlTest(unittest.TestCase): answer = "blah blah" assessment = [0,1] hint = "blah" - test_server = create_server("127.0.0.1", 3034) def setUp(self): self.test_system = test_system() + self.test_system.xqueue['interface'] = Mock( + send_to_queue = Mock(side_effect=[1,"queued"]) + ) @staticmethod def get_import_system(load_error_modules=True): @@ -592,6 +595,16 @@ class OpenEndedModuleXmlTest(unittest.TestCase): module.handle_ajax("get_status", {}) #Move to the next step in the problem - module.handle_ajax("next_problem", {}) - module.get_html() - module.handle_ajax("get_combined_rubric", {}) + try: + module.handle_ajax("next_problem", {}) + except GradingServiceError: + #This error is okay. We don't have a grading service to connect to! + pass + #Move to the next step in the problem + try: + module.get_html() + except GradingServiceError: + #This error is okay. We don't have a grading service to connect to! + pass + + module.handle_ajax("get_combined_rubric", {}) From c39a21d833c51737413afee586444fa4a4ebde64 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Wed, 8 May 2013 16:03:11 -0400 Subject: [PATCH 04/21] Remove old import --- common/lib/xmodule/xmodule/tests/test_combined_open_ended.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py index a8127eec30..d845a9a711 100644 --- a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py +++ b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py @@ -21,8 +21,6 @@ import capa.xqueue_interface as xqueue_interface from datetime import datetime import logging -from mock_server import create_server - log = logging.getLogger(__name__) from . import test_system From f26a2585982c796e6aa1be0835882c070e1f155c Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Wed, 8 May 2013 18:07:17 -0400 Subject: [PATCH 05/21] Add in test for grader reply --- .../open_ended_module.py | 2 +- .../xmodule/tests/test_combined_open_ended.py | 20 +++++++++++++++++-- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py index 4a8604ac30..afdfeef6de 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py @@ -516,7 +516,7 @@ class OpenEndedModule(openendedchild.OpenEndedChild): try: feedback_dict = json.loads(score_result['feedback']) except: - pass + feedback_dict = score_result.get('feedback', '') feedback_dicts = [feedback_dict] grader_ids = [score_result['grader_id']] submission_ids = [score_result['submission_id']] diff --git a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py index d845a9a711..7cfec5fb62 100644 --- a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py +++ b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py @@ -598,11 +598,27 @@ class OpenEndedModuleXmlTest(unittest.TestCase): except GradingServiceError: #This error is okay. We don't have a grading service to connect to! pass - #Move to the next step in the problem try: module.get_html() except GradingServiceError: #This error is okay. We don't have a grading service to connect to! pass - module.handle_ajax("get_combined_rubric", {}) + module.handle_ajax("get_combined_rubric", {}) + + queue_reply = { + 'queuekey' : "", + 'xqueue_body' : json.dumps({ + 'score' : 0, + 'feedback' : json.dumps({"spelling": "Spelling: Ok.", "grammar": "Grammar: Ok.", "markup-text": " all of us can think of a book that we hope none of our children or any other children have taken off the shelf . but if i have the right to remove that book from the shelf that work i abhor then you also have exactly the same right and so does everyone else . and then we have no books left on the shelf for any of us . katherine paterson , author write a persuasive essay to a newspaper reflecting your vies on censorship in libraries . do you believe that certain materials , such as books , music , movies , magazines , etc . , should be removed from the shelves if they are found offensive ? support your position with convincing arguments from your own experience , observations , and or reading . "}), + 'grader_type' : "ML", + 'success' : True, + 'grader_id' : 1, + 'submission_id' : 1, + 'rubric_xml' : "Writing Applications0 Language Conventions 0", + 'rubric_scores_complete' : True, + }) + } + + module.handle_ajax("score_update", queue_reply) + From 4d759e9772b690c3ded7458bb7b9ed6eaecad6a1 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Wed, 8 May 2013 18:15:57 -0400 Subject: [PATCH 06/21] Test full flow, including reset --- .../combined_open_ended_modulev1.py | 12 ++++++++++++ .../xmodule/tests/test_combined_open_ended.py | 17 +++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py index 12f90ed1b3..6767851d3a 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py @@ -803,6 +803,18 @@ class CombinedOpenEndedV1Module(): return progress_object + def out_of_sync_error(self, get, msg=''): + """ + return dict out-of-sync error message, and also log. + """ + #This is a dev_facing_error + log.warning("Combined module state out sync. state: %r, get: %r. %s", + self.state, get, msg) + #This is a student_facing_error + return {'success': False, + 'error': 'The problem state got out-of-sync. Please try reloading the page.'} + + class CombinedOpenEndedV1Descriptor(): """ diff --git a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py index 7cfec5fb62..3b8019290f 100644 --- a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py +++ b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py @@ -604,8 +604,10 @@ class OpenEndedModuleXmlTest(unittest.TestCase): #This error is okay. We don't have a grading service to connect to! pass + #Try to get the rubric from the module module.handle_ajax("get_combined_rubric", {}) + #Make a fake reply from the queue queue_reply = { 'queuekey' : "", 'xqueue_body' : json.dumps({ @@ -620,5 +622,20 @@ class OpenEndedModuleXmlTest(unittest.TestCase): }) } + module.handle_ajax("check_for_score", {}) + + #Update the module with the fake queue reply module.handle_ajax("score_update", queue_reply) + #Get html and other data client will request + html = module.get_html() + legend = module.handle_ajax("get_legend", {}) + status = module.handle_ajax("get_status", {}) + legend = module.handle_ajax("skip_post_assessment", {}) + + #Get all results + legend = module.handle_ajax("get_results", {}) + + log.info(module.task_states) + #reset the problem + module.handle_ajax("reset", {}) From ff84545f3155c00e3824b79845e6ab3fac160b2e Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Wed, 8 May 2013 18:49:28 -0400 Subject: [PATCH 07/21] Start to add peer grading tests, make dummy system a separate thing --- .../peer_grading_service.py | 27 +++++---- .../lib/xmodule/xmodule/tests/dummy_system.py | 55 ++++++++++++++++++ .../xmodule/tests/test_combined_open_ended.py | 57 ++----------------- .../xmodule/tests/test_peer_grading.py | 49 ++++++++++++++++ .../test/data/open_ended/course/2012_Fall.xml | 1 + .../peergrading/PeerGradingSample.xml | 1 + .../data/open_ended/policies/2012_Fall.json | 5 +- 7 files changed, 131 insertions(+), 64 deletions(-) create mode 100644 common/lib/xmodule/xmodule/tests/dummy_system.py create mode 100644 common/lib/xmodule/xmodule/tests/test_peer_grading.py create mode 100644 common/test/data/open_ended/peergrading/PeerGradingSample.xml diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/peer_grading_service.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/peer_grading_service.py index 85c7a98132..19cc013cb7 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/peer_grading_service.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/peer_grading_service.py @@ -100,29 +100,29 @@ without making actual service calls to the grading controller class MockPeerGradingService(object): def get_next_submission(self, problem_location, grader_id): - return json.dumps({'success': True, + return {'success': True, 'submission_id': 1, 'submission_key': "", 'student_response': 'fake student response', 'prompt': 'fake submission prompt', 'rubric': 'fake rubric', - 'max_score': 4}) + 'max_score': 4} def save_grade(self, location, grader_id, submission_id, score, feedback, submission_key, rubric_scores, submission_flagged): - return json.dumps({'success': True}) + return {'success': True} def is_student_calibrated(self, problem_location, grader_id): - return json.dumps({'success': True, 'calibrated': True}) + return {'success': True, 'calibrated': True} def show_calibration_essay(self, problem_location, grader_id): - return json.dumps({'success': True, + return {'success': True, 'submission_id': 1, 'submission_key': '', 'student_response': 'fake student response', 'prompt': 'fake submission prompt', 'rubric': 'fake rubric', - 'max_score': 4}) + 'max_score': 4} def save_calibration_essay(self, problem_location, grader_id, calibration_essay_id, submission_key, score, @@ -130,10 +130,13 @@ class MockPeerGradingService(object): return {'success': True, 'actual_score': 2} def get_problem_list(self, course_id, grader_id): - return json.dumps({'success': True, + return {'success': True, 'problem_list': [ - json.dumps({'location': 'i4x://MITx/3.091x/problem/open_ended_demo1', - 'problem_name': "Problem 1", 'num_graded': 3, 'num_pending': 5}), - json.dumps({'location': 'i4x://MITx/3.091x/problem/open_ended_demo2', - 'problem_name': "Problem 2", 'num_graded': 1, 'num_pending': 5}) - ]}) + {'location': 'i4x://MITx/3.091x/problem/open_ended_demo1', + 'problem_name': "Problem 1", 'num_graded': 3, 'num_pending': 5}, + {'location': 'i4x://MITx/3.091x/problem/open_ended_demo2', + 'problem_name': "Problem 2", 'num_graded': 1, 'num_pending': 5} + ]} + + def get_data_for_location(self, problem_location, student_id): + return {"version": 1, "count_graded": 3, "count_required": 3, "success": True, "student_sub_count": 1} diff --git a/common/lib/xmodule/xmodule/tests/dummy_system.py b/common/lib/xmodule/xmodule/tests/dummy_system.py new file mode 100644 index 0000000000..b4ca6136eb --- /dev/null +++ b/common/lib/xmodule/xmodule/tests/dummy_system.py @@ -0,0 +1,55 @@ +from . import test_system +import unittest +from xmodule.modulestore import Location +from xmodule.modulestore.xml import ImportSystem, XMLModuleStore +from xmodule.tests.test_export import DATA_DIR +from fs.memoryfs import MemoryFS +from mock import patch, Mock + +class DummySystem(ImportSystem): + + @patch('xmodule.modulestore.xml.OSFS', lambda dir: MemoryFS()) + def __init__(self, load_error_modules, org, course): + + xmlstore = XMLModuleStore("data_dir", course_dirs=[], load_error_modules=load_error_modules) + course_id = "/".join([org, course, 'test_run']) + course_dir = "test_dir" + policy = {} + error_tracker = Mock() + parent_tracker = Mock() + + super(DummySystem, self).__init__( + xmlstore, + course_id, + course_dir, + policy, + error_tracker, + parent_tracker, + load_error_modules=load_error_modules, + ) + + def render_template(self, template, context): + raise Exception("Shouldn't be called") + +class DummySystemUser(object): + test_system = test_system() + @staticmethod + def get_import_system(org, course, load_error_modules=True): + '''Get a dummy system''' + return DummySystem(load_error_modules, org, course) + + def get_course(self, name): + """Get a test course by directory name. If there's more than one, error.""" + + modulestore = XMLModuleStore(DATA_DIR, course_dirs=[name]) + courses = modulestore.get_courses() + self.modulestore = modulestore + self.assertEquals(len(courses), 1) + return courses[0] + + def get_module_from_location(self, location, course): + course = self.get_course(course) + if not isinstance(location, Location): + location = Location(location) + descriptor = self.modulestore.get_instance(course.id, location, depth=None) + return descriptor.xmodule(self.test_system) \ No newline at end of file diff --git a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py index 3b8019290f..fdbd37c6ab 100644 --- a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py +++ b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py @@ -5,6 +5,8 @@ import unittest from fs.memoryfs import MemoryFS from mock import patch +from dummy_system import DummySystemUser + from xmodule.open_ended_grading_classes.openendedchild import OpenEndedChild from xmodule.open_ended_grading_classes.open_ended_module import OpenEndedModule from xmodule.open_ended_grading_classes.combined_open_ended_modulev1 import CombinedOpenEndedV1Module @@ -42,31 +44,6 @@ class MockQueryDict(dict): return [] return default -class DummySystem(ImportSystem): - - @patch('xmodule.modulestore.xml.OSFS', lambda dir: MemoryFS()) - def __init__(self, load_error_modules): - - xmlstore = XMLModuleStore("data_dir", course_dirs=[], load_error_modules=load_error_modules) - course_id = "/".join([ORG, COURSE, 'test_run']) - course_dir = "test_dir" - policy = {} - error_tracker = Mock() - parent_tracker = Mock() - - super(DummySystem, self).__init__( - xmlstore, - course_id, - course_dir, - policy, - error_tracker, - parent_tracker, - load_error_modules=load_error_modules, - ) - - def render_template(self, template, context): - raise Exception("Shouldn't be called") - """ Tests for the various pieces of the CombinedOpenEndedGrading system @@ -514,7 +491,7 @@ class CombinedOpenEndedModuleTest(unittest.TestCase): self.assertEqual(score_dict['score'], 15.0) self.assertEqual(score_dict['total'], 15.0) -class OpenEndedModuleXmlTest(unittest.TestCase): +class OpenEndedModuleXmlTest(unittest.TestCase, DummySystemUser): problem_location = Location(["i4x", "edX", "oe_test", "combinedopenended", "SampleQuestion"]) answer = "blah blah" assessment = [0,1] @@ -525,37 +502,15 @@ class OpenEndedModuleXmlTest(unittest.TestCase): send_to_queue = Mock(side_effect=[1,"queued"]) ) - @staticmethod - def get_import_system(load_error_modules=True): - '''Get a dummy system''' - return DummySystem(load_error_modules) - - def get_course(self, name): - """Get a test course by directory name. If there's more than one, error.""" - print "Importing {0}".format(name) - - modulestore = XMLModuleStore(DATA_DIR, course_dirs=[name]) - courses = modulestore.get_courses() - self.modulestore = modulestore - self.assertEquals(len(courses), 1) - return courses[0] - - def get_module_from_location(self, location): - course = self.get_course('open_ended') - if not isinstance(location, Location): - location = Location(location) - descriptor = self.modulestore.get_instance(course.id, location, depth=None) - return descriptor.xmodule(self.test_system) - def test_open_ended_load_and_save(self): - module = self.get_module_from_location(self.problem_location) + module = self.get_module_from_location(self.problem_location, COURSE) module.handle_ajax("save_answer", {"student_answer" : self.answer}) task_one_json = json.loads(module.task_states[0]) self.assertEqual(task_one_json['child_history'][0]['answer'], self.answer) def test_open_ended_flow_reset(self): assessment = [0,1] - module = self.get_module_from_location(self.problem_location) + module = self.get_module_from_location(self.problem_location, COURSE) #Simulate a student saving an answer module.handle_ajax("save_answer", {"student_answer" : self.answer}) @@ -578,7 +533,7 @@ class OpenEndedModuleXmlTest(unittest.TestCase): def test_open_ended_flow_correct(self): assessment = [1,1] - module = self.get_module_from_location(self.problem_location) + module = self.get_module_from_location(self.problem_location, COURSE) #Simulate a student saving an answer module.handle_ajax("save_answer", {"student_answer" : self.answer}) diff --git a/common/lib/xmodule/xmodule/tests/test_peer_grading.py b/common/lib/xmodule/xmodule/tests/test_peer_grading.py new file mode 100644 index 0000000000..01b3da0778 --- /dev/null +++ b/common/lib/xmodule/xmodule/tests/test_peer_grading.py @@ -0,0 +1,49 @@ +import unittest +from xmodule.modulestore import Location +import json +from lxml import etree +from mock import Mock +from . import test_system +from dummy_system import DummySystem, DummySystemUser + +from xmodule.peer_grading_module import PeerGradingModule, PeerGradingDescriptor +from xmodule.open_ended_grading_classes.grading_service_module import GradingServiceError + +ORG = "edX" +COURSE="open_ended" + + +class PeerGradingModuleTest(unittest.TestCase, DummySystemUser): + location = Location(["i4x", "edX", "open_ended", "peergrading", + "SampleQuestion"]) + max_score = 1 + + definition = "" + descriptor = Mock(data=definition) + + def setUp(self): + self.test_system = test_system() + self.test_system.open_ended_grading_interface = None + self.peer_grading = PeerGradingModule(self.test_system, self.location,self.descriptor, model_data={'data': self.definition}) + + def test_module_closed(self): + closed = self.peer_grading.closed() + self.assertEqual(closed, False) + + def test_get_html(self): + html = self.peer_grading.get_html() + + def test_get_data(self): + try: + success, data = self.peer_grading.query_data_for_location() + except GradingServiceError: + pass + + def test_get_score(self): + score = self.peer_grading.get_score() + + def test_get_max_score(self): + max_score = self.peer_grading.max_score() + + def get_next_submission(self): + success, next_submission = self.peer_grading.get_next_submission({'location' : 'blah'}) \ No newline at end of file diff --git a/common/test/data/open_ended/course/2012_Fall.xml b/common/test/data/open_ended/course/2012_Fall.xml index f2d16488a7..34369979ca 100644 --- a/common/test/data/open_ended/course/2012_Fall.xml +++ b/common/test/data/open_ended/course/2012_Fall.xml @@ -1,5 +1,6 @@ + diff --git a/common/test/data/open_ended/peergrading/PeerGradingSample.xml b/common/test/data/open_ended/peergrading/PeerGradingSample.xml new file mode 100644 index 0000000000..7e3afddf3a --- /dev/null +++ b/common/test/data/open_ended/peergrading/PeerGradingSample.xml @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/common/test/data/open_ended/policies/2012_Fall.json b/common/test/data/open_ended/policies/2012_Fall.json index 09b68ab400..8f8ba13437 100644 --- a/common/test/data/open_ended/policies/2012_Fall.json +++ b/common/test/data/open_ended/policies/2012_Fall.json @@ -9,6 +9,9 @@ "display_name": "Overview" }, "combinedopenended/SampleQuestion": { - "display_name": "Sample Question", + "display_name": "Sample Question" }, + "peergrading/PeerGradingSample": { + "display_name": "Sample Question" + } } From fd46ebd1fa950c86b8147093a644bd8b0d1561be Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Wed, 8 May 2013 19:04:07 -0400 Subject: [PATCH 08/21] Move some functions, make notification tests more robust --- .../lib/xmodule/xmodule/tests/dummy_system.py | 15 +++++- .../xmodule/tests/test_combined_open_ended.py | 25 ++-------- .../xmodule/tests/test_peer_grading.py | 10 ++-- common/test/data/open_ended/course.xml | 2 +- lms/djangoapps/open_ended_grading/tests.py | 46 +++++++++++++++---- 5 files changed, 57 insertions(+), 41 deletions(-) diff --git a/common/lib/xmodule/xmodule/tests/dummy_system.py b/common/lib/xmodule/xmodule/tests/dummy_system.py index b4ca6136eb..d0b7513321 100644 --- a/common/lib/xmodule/xmodule/tests/dummy_system.py +++ b/common/lib/xmodule/xmodule/tests/dummy_system.py @@ -44,7 +44,6 @@ class DummySystemUser(object): modulestore = XMLModuleStore(DATA_DIR, course_dirs=[name]) courses = modulestore.get_courses() self.modulestore = modulestore - self.assertEquals(len(courses), 1) return courses[0] def get_module_from_location(self, location, course): @@ -52,4 +51,16 @@ class DummySystemUser(object): if not isinstance(location, Location): location = Location(location) descriptor = self.modulestore.get_instance(course.id, location, depth=None) - return descriptor.xmodule(self.test_system) \ No newline at end of file + return descriptor.xmodule(self.test_system) + +class MockQueryDict(dict): + """ + Mock a query set so that it can be used with default authorization + """ + def getlist(self, key, default=None): + try: + return super(MockQueryDict, self).__getitem__(key) + except KeyError: + if default is None: + return [] + return default \ No newline at end of file diff --git a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py index fdbd37c6ab..665addefa2 100644 --- a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py +++ b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py @@ -2,21 +2,14 @@ import json from mock import Mock, MagicMock, ANY import unittest -from fs.memoryfs import MemoryFS -from mock import patch - -from dummy_system import DummySystemUser +from dummy_system import DummySystemUser, MockQueryDict from xmodule.open_ended_grading_classes.openendedchild import OpenEndedChild from xmodule.open_ended_grading_classes.open_ended_module import OpenEndedModule from xmodule.open_ended_grading_classes.combined_open_ended_modulev1 import CombinedOpenEndedV1Module from xmodule.open_ended_grading_classes.grading_service_module import GradingServiceError from xmodule.combined_open_ended_module import CombinedOpenEndedModule -from xmodule.modulestore.django import modulestore from xmodule.modulestore import Location -from xmodule.modulestore.xml import ImportSystem, XMLModuleStore - -from xmodule.tests.test_export import DATA_DIR from lxml import etree import capa.xqueue_interface as xqueue_interface @@ -27,23 +20,11 @@ log = logging.getLogger(__name__) from . import test_system -ORG = 'test_org' +ORG = 'edX' COURSE = 'open_ended' # name of directory with course data import test_util_open_ended -class MockQueryDict(dict): - """ - Mock a query set so that it can be used with default authorization - """ - def getlist(self, key, default=None): - try: - return super(MockQueryDict, self).__getitem__(key) - except KeyError: - if default is None: - return [] - return default - """ Tests for the various pieces of the CombinedOpenEndedGrading system @@ -492,7 +473,7 @@ class CombinedOpenEndedModuleTest(unittest.TestCase): self.assertEqual(score_dict['total'], 15.0) class OpenEndedModuleXmlTest(unittest.TestCase, DummySystemUser): - problem_location = Location(["i4x", "edX", "oe_test", "combinedopenended", "SampleQuestion"]) + problem_location = Location(["i4x", "edX", "open_ended", "combinedopenended", "SampleQuestion"]) answer = "blah blah" assessment = [0,1] hint = "blah" diff --git a/common/lib/xmodule/xmodule/tests/test_peer_grading.py b/common/lib/xmodule/xmodule/tests/test_peer_grading.py index 01b3da0778..3ab2e9301f 100644 --- a/common/lib/xmodule/xmodule/tests/test_peer_grading.py +++ b/common/lib/xmodule/xmodule/tests/test_peer_grading.py @@ -14,17 +14,13 @@ COURSE="open_ended" class PeerGradingModuleTest(unittest.TestCase, DummySystemUser): - location = Location(["i4x", "edX", "open_ended", "peergrading", - "SampleQuestion"]) - max_score = 1 - - definition = "" - descriptor = Mock(data=definition) + problem_location = Location(["i4x", "edX", "open_ended", "peergrading", + "PeerGradingSample"]) def setUp(self): self.test_system = test_system() self.test_system.open_ended_grading_interface = None - self.peer_grading = PeerGradingModule(self.test_system, self.location,self.descriptor, model_data={'data': self.definition}) + self.peer_grading = self.get_module_from_location(self.problem_location, COURSE) def test_module_closed(self): closed = self.peer_grading.closed() diff --git a/common/test/data/open_ended/course.xml b/common/test/data/open_ended/course.xml index bf3ed687fb..9848343f58 100644 --- a/common/test/data/open_ended/course.xml +++ b/common/test/data/open_ended/course.xml @@ -1 +1 @@ - + diff --git a/lms/djangoapps/open_ended_grading/tests.py b/lms/djangoapps/open_ended_grading/tests.py index 93d27d8e24..542c366cab 100644 --- a/lms/djangoapps/open_ended_grading/tests.py +++ b/lms/djangoapps/open_ended_grading/tests.py @@ -84,7 +84,13 @@ class TestStaffGradingService(LoginEnrollmentTestCase): data = {'location': self.location} r = self.check_for_post_code(200, url, data) - d = json.loads(r.content) + + d = r.content + try: + d = json.loads(d) + except Exception: + pass + self.assertTrue(d['success']) self.assertEquals(d['submission_id'], self.mock_service.cnt) self.assertIsNotNone(d['submission']) @@ -112,7 +118,11 @@ class TestStaffGradingService(LoginEnrollmentTestCase): data.update({'skipped' : True}) r = self.check_for_post_code(200, url, data) - d = json.loads(r.content) + d = r.content + try: + d = json.loads(d) + except Exception: + pass self.assertTrue(d['success'], str(d)) self.assertEquals(d['submission_id'], self.mock_service.cnt) @@ -129,7 +139,11 @@ class TestStaffGradingService(LoginEnrollmentTestCase): data = {} r = self.check_for_post_code(200, url, data) - d = json.loads(r.content) + d = r.content + try: + d = json.loads(d) + except Exception: + pass self.assertTrue(d['success'], str(d)) self.assertIsNotNone(d['problem_list']) @@ -179,7 +193,11 @@ class TestPeerGradingService(LoginEnrollmentTestCase): data = {'location': self.location} r = self.peer_module.get_next_submission(data) - d = json.loads(r) + d = r + try: + d = json.loads(d) + except Exception: + pass self.assertTrue(d['success']) self.assertIsNotNone(d['submission_id']) self.assertIsNotNone(d['prompt']) @@ -213,7 +231,11 @@ class TestPeerGradingService(LoginEnrollmentTestCase): qdict.keys = data.keys r = self.peer_module.save_grade(qdict) - d = json.loads(r) + d = r + try: + d = json.loads(d) + except Exception: + pass self.assertTrue(d['success']) def test_save_grade_missing_keys(self): @@ -225,7 +247,11 @@ class TestPeerGradingService(LoginEnrollmentTestCase): def test_is_calibrated_success(self): data = {'location': self.location} r = self.peer_module.is_student_calibrated(data) - d = json.loads(r) + d = r + try: + d = json.loads(d) + except Exception: + pass self.assertTrue(d['success']) self.assertTrue('calibrated' in d) @@ -239,9 +265,11 @@ class TestPeerGradingService(LoginEnrollmentTestCase): data = {'location': self.location} r = self.peer_module.show_calibration_essay(data) - d = json.loads(r) - log.debug(d) - log.debug(type(d)) + d = r + try: + d = json.loads(r) + except Exception: + pass self.assertTrue(d['success']) self.assertIsNotNone(d['submission_id']) self.assertIsNotNone(d['prompt']) From a2e5bd071b57f19fc36d0a9f8f41c0a46a3a27bf Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Wed, 8 May 2013 19:17:14 -0400 Subject: [PATCH 09/21] Add in tests for peer grading module --- .../peer_grading_service.py | 4 -- .../xmodule/tests/test_peer_grading.py | 38 ++++++++++++++++++- 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/peer_grading_service.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/peer_grading_service.py index 19cc013cb7..418784f618 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/peer_grading_service.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/peer_grading_service.py @@ -132,10 +132,6 @@ class MockPeerGradingService(object): def get_problem_list(self, course_id, grader_id): return {'success': True, 'problem_list': [ - {'location': 'i4x://MITx/3.091x/problem/open_ended_demo1', - 'problem_name': "Problem 1", 'num_graded': 3, 'num_pending': 5}, - {'location': 'i4x://MITx/3.091x/problem/open_ended_demo2', - 'problem_name': "Problem 2", 'num_graded': 1, 'num_pending': 5} ]} def get_data_for_location(self, problem_location, student_id): diff --git a/common/lib/xmodule/xmodule/tests/test_peer_grading.py b/common/lib/xmodule/xmodule/tests/test_peer_grading.py index 3ab2e9301f..3ecfc759e5 100644 --- a/common/lib/xmodule/xmodule/tests/test_peer_grading.py +++ b/common/lib/xmodule/xmodule/tests/test_peer_grading.py @@ -4,7 +4,7 @@ import json from lxml import etree from mock import Mock from . import test_system -from dummy_system import DummySystem, DummySystemUser +from dummy_system import DummySystem, DummySystemUser, MockQueryDict from xmodule.peer_grading_module import PeerGradingModule, PeerGradingDescriptor from xmodule.open_ended_grading_classes.grading_service_module import GradingServiceError @@ -16,6 +16,17 @@ COURSE="open_ended" class PeerGradingModuleTest(unittest.TestCase, DummySystemUser): problem_location = Location(["i4x", "edX", "open_ended", "peergrading", "PeerGradingSample"]) + calibrated_dict = {'location' : "blah"} + save_dict = MockQueryDict() + save_dict.update({ + 'location' : "blah", + 'submission_id' : 1, + 'submission_key' : "", + 'score': 1, + 'feedback' : "", + 'rubric_scores[]' : [0,1], + 'submission_flagged': False, + }) def setUp(self): self.test_system = test_system() @@ -42,4 +53,27 @@ class PeerGradingModuleTest(unittest.TestCase, DummySystemUser): max_score = self.peer_grading.max_score() def get_next_submission(self): - success, next_submission = self.peer_grading.get_next_submission({'location' : 'blah'}) \ No newline at end of file + success, next_submission = self.peer_grading.get_next_submission({'location' : 'blah'}) + + def test_save_grade(self): + self.peer_grading.save_grade(self.save_dict) + + def test_is_student_calibrated(self): + calibrated_dict = {'location' : "blah"} + self.peer_grading.is_student_calibrated(self.calibrated_dict) + + def test_show_calibration_essay(self): + + self.peer_grading.show_calibration_essay(self.calibrated_dict) + + def test_save_calibration_essay(self): + self.peer_grading.save_calibration_essay(self.save_dict) + + def test_peer_grading_closed(self): + self.peer_grading.peer_grading_closed() + + def test_peer_grading_problem(self): + self.peer_grading.peer_grading_problem(self.calibrated_dict) + + def test_get_instance_state(self): + self.peer_grading.get_instance_state() \ No newline at end of file From 7b8b168f2e8f005f7e89e4e68012e5e7c0e85aa1 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Wed, 8 May 2013 19:21:12 -0400 Subject: [PATCH 10/21] Move the mockquerydict --- common/lib/xmodule/xmodule/tests/dummy_system.py | 14 +------------- .../xmodule/tests/test_combined_open_ended.py | 3 ++- .../xmodule/xmodule/tests/test_peer_grading.py | 3 ++- .../xmodule/tests/test_util_open_ended.py | 16 ++++++++++++++-- 4 files changed, 19 insertions(+), 17 deletions(-) diff --git a/common/lib/xmodule/xmodule/tests/dummy_system.py b/common/lib/xmodule/xmodule/tests/dummy_system.py index d0b7513321..02fa0450f6 100644 --- a/common/lib/xmodule/xmodule/tests/dummy_system.py +++ b/common/lib/xmodule/xmodule/tests/dummy_system.py @@ -51,16 +51,4 @@ class DummySystemUser(object): if not isinstance(location, Location): location = Location(location) descriptor = self.modulestore.get_instance(course.id, location, depth=None) - return descriptor.xmodule(self.test_system) - -class MockQueryDict(dict): - """ - Mock a query set so that it can be used with default authorization - """ - def getlist(self, key, default=None): - try: - return super(MockQueryDict, self).__getitem__(key) - except KeyError: - if default is None: - return [] - return default \ No newline at end of file + return descriptor.xmodule(self.test_system) \ No newline at end of file diff --git a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py index 665addefa2..157d403ffe 100644 --- a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py +++ b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py @@ -2,7 +2,8 @@ import json from mock import Mock, MagicMock, ANY import unittest -from dummy_system import DummySystemUser, MockQueryDict +from dummy_system import DummySystemUser +from test_util_open_ended import MockQueryDict from xmodule.open_ended_grading_classes.openendedchild import OpenEndedChild from xmodule.open_ended_grading_classes.open_ended_module import OpenEndedModule diff --git a/common/lib/xmodule/xmodule/tests/test_peer_grading.py b/common/lib/xmodule/xmodule/tests/test_peer_grading.py index 3ecfc759e5..630f693333 100644 --- a/common/lib/xmodule/xmodule/tests/test_peer_grading.py +++ b/common/lib/xmodule/xmodule/tests/test_peer_grading.py @@ -4,7 +4,8 @@ import json from lxml import etree from mock import Mock from . import test_system -from dummy_system import DummySystem, DummySystemUser, MockQueryDict +from dummy_system import DummySystem, DummySystemUser +from test_util_open_ended import MockQueryDict from xmodule.peer_grading_module import PeerGradingModule, PeerGradingDescriptor from xmodule.open_ended_grading_classes.grading_service_module import GradingServiceError diff --git a/common/lib/xmodule/xmodule/tests/test_util_open_ended.py b/common/lib/xmodule/xmodule/tests/test_util_open_ended.py index db580f1e0e..088a5af87d 100644 --- a/common/lib/xmodule/xmodule/tests/test_util_open_ended.py +++ b/common/lib/xmodule/xmodule/tests/test_util_open_ended.py @@ -1,5 +1,5 @@ OPEN_ENDED_GRADING_INTERFACE = { - 'url': 'http://127.0.0.1:3033/', + 'url': 'blah/', 'username': 'incorrect', 'password': 'incorrect', 'staff_grading': 'staff_grading', @@ -11,4 +11,16 @@ S3_INTERFACE = { 'aws_access_key': "", 'aws_secret_key': "", "aws_bucket_name": "", -} \ No newline at end of file +} + +class MockQueryDict(dict): + """ + Mock a query set so that it can be used with default authorization + """ + def getlist(self, key, default=None): + try: + return super(MockQueryDict, self).__getitem__(key) + except KeyError: + if default is None: + return [] + return default \ No newline at end of file From 8323cc7c60174105361262b912ac1c25fa1fc93d Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Thu, 9 May 2013 09:44:16 -0400 Subject: [PATCH 11/21] Refactor tests --- .../lib/xmodule/xmodule/tests/dummy_system.py | 54 ---------- .../xmodule/tests/test_combined_open_ended.py | 102 +++++++++--------- .../xmodule/tests/test_peer_grading.py | 37 ++++--- .../xmodule/tests/test_util_open_ended.py | 27 ++++- .../test/data/open_ended/course/2012_Fall.xml | 1 + .../peergrading/PeerGradingScored.xml | 1 + 6 files changed, 98 insertions(+), 124 deletions(-) delete mode 100644 common/lib/xmodule/xmodule/tests/dummy_system.py create mode 100644 common/test/data/open_ended/peergrading/PeerGradingScored.xml diff --git a/common/lib/xmodule/xmodule/tests/dummy_system.py b/common/lib/xmodule/xmodule/tests/dummy_system.py deleted file mode 100644 index 02fa0450f6..0000000000 --- a/common/lib/xmodule/xmodule/tests/dummy_system.py +++ /dev/null @@ -1,54 +0,0 @@ -from . import test_system -import unittest -from xmodule.modulestore import Location -from xmodule.modulestore.xml import ImportSystem, XMLModuleStore -from xmodule.tests.test_export import DATA_DIR -from fs.memoryfs import MemoryFS -from mock import patch, Mock - -class DummySystem(ImportSystem): - - @patch('xmodule.modulestore.xml.OSFS', lambda dir: MemoryFS()) - def __init__(self, load_error_modules, org, course): - - xmlstore = XMLModuleStore("data_dir", course_dirs=[], load_error_modules=load_error_modules) - course_id = "/".join([org, course, 'test_run']) - course_dir = "test_dir" - policy = {} - error_tracker = Mock() - parent_tracker = Mock() - - super(DummySystem, self).__init__( - xmlstore, - course_id, - course_dir, - policy, - error_tracker, - parent_tracker, - load_error_modules=load_error_modules, - ) - - def render_template(self, template, context): - raise Exception("Shouldn't be called") - -class DummySystemUser(object): - test_system = test_system() - @staticmethod - def get_import_system(org, course, load_error_modules=True): - '''Get a dummy system''' - return DummySystem(load_error_modules, org, course) - - def get_course(self, name): - """Get a test course by directory name. If there's more than one, error.""" - - modulestore = XMLModuleStore(DATA_DIR, course_dirs=[name]) - courses = modulestore.get_courses() - self.modulestore = modulestore - return courses[0] - - def get_module_from_location(self, location, course): - course = self.get_course(course) - if not isinstance(location, Location): - location = Location(location) - descriptor = self.modulestore.get_instance(course.id, location, depth=None) - return descriptor.xmodule(self.test_system) \ No newline at end of file diff --git a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py index 157d403ffe..2faecce08d 100644 --- a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py +++ b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py @@ -2,8 +2,7 @@ import json from mock import Mock, MagicMock, ANY import unittest -from dummy_system import DummySystemUser -from test_util_open_ended import MockQueryDict +from test_util_open_ended import MockQueryDict, DummyModulestore from xmodule.open_ended_grading_classes.openendedchild import OpenEndedChild from xmodule.open_ended_grading_classes.open_ended_module import OpenEndedModule @@ -19,7 +18,7 @@ import logging log = logging.getLogger(__name__) -from . import test_system +from .import test_system ORG = 'edX' COURSE = 'open_ended' # name of directory with course data @@ -68,7 +67,7 @@ class OpenEndedChildTest(unittest.TestCase): def setUp(self): self.test_system = test_system() self.openendedchild = OpenEndedChild(self.test_system, self.location, - self.definition, self.descriptor, self.static_data, self.metadata) + self.definition, self.descriptor, self.static_data, self.metadata) def test_latest_answer_empty(self): @@ -115,7 +114,7 @@ class OpenEndedChildTest(unittest.TestCase): post_assessment = "Post assessment" self.openendedchild.record_latest_post_assessment(post_assessment) self.assertEqual(post_assessment, - self.openendedchild.latest_post_assessment(self.test_system)) + self.openendedchild.latest_post_assessment(self.test_system)) def test_get_score(self): new_answer = "New Answer" @@ -142,12 +141,12 @@ class OpenEndedChildTest(unittest.TestCase): self.openendedchild.new_history_entry(new_answer) self.openendedchild.record_latest_score(self.static_data['max_score']) self.assertEqual(self.openendedchild.is_last_response_correct(), - 'correct') + 'correct') self.openendedchild.new_history_entry(new_answer) self.openendedchild.record_latest_score(0) self.assertEqual(self.openendedchild.is_last_response_correct(), - 'incorrect') + 'incorrect') class OpenEndedModuleTest(unittest.TestCase): @@ -202,7 +201,7 @@ class OpenEndedModuleTest(unittest.TestCase): 'default_queuename': 'testqueue', 'waittime': 1} self.openendedmodule = OpenEndedModule(self.test_system, self.location, - self.definition, self.descriptor, self.static_data, self.metadata) + self.definition, self.descriptor, self.static_data, self.metadata) def test_message_post(self): get = {'feedback': 'feedback text', @@ -364,21 +363,21 @@ class CombinedOpenEndedModuleTest(unittest.TestCase): descriptor = Mock(data=full_definition) test_system = test_system() combinedoe_container = CombinedOpenEndedModule(test_system, - location, - descriptor, - model_data={'data': full_definition, 'weight': '1'}) + location, + descriptor, + model_data={'data': full_definition, 'weight': '1'}) def setUp(self): # TODO: this constructor call is definitely wrong, but neither branch # of the merge matches the module constructor. Someone (Vik?) should fix this. self.combinedoe = CombinedOpenEndedV1Module(self.test_system, - self.location, - self.definition, - self.descriptor, - static_data=self.static_data, - metadata=self.metadata, - instance_state=self.static_data) + self.location, + self.definition, + self.descriptor, + static_data=self.static_data, + metadata=self.metadata, + instance_state=self.static_data) def test_get_tag_name(self): name = self.combinedoe.get_tag_name("Tag") @@ -433,12 +432,12 @@ class CombinedOpenEndedModuleTest(unittest.TestCase): definition = {'prompt': etree.XML(self.prompt), 'rubric': etree.XML(self.rubric), 'task_xml': xml} descriptor = Mock(data=definition) combinedoe = CombinedOpenEndedV1Module(self.test_system, - self.location, - definition, - descriptor, - static_data=self.static_data, - metadata=self.metadata, - instance_state=self.static_data) + self.location, + definition, + descriptor, + static_data=self.static_data, + metadata=self.metadata, + instance_state=self.static_data) changed = combinedoe.update_task_states() self.assertFalse(changed) @@ -463,44 +462,46 @@ class CombinedOpenEndedModuleTest(unittest.TestCase): 'task_xml': [self.task_xml1, self.task_xml2]} descriptor = Mock(data=definition) combinedoe = CombinedOpenEndedV1Module(self.test_system, - self.location, - definition, - descriptor, - static_data=self.static_data, - metadata=self.metadata, - instance_state=instance_state) + self.location, + definition, + descriptor, + static_data=self.static_data, + metadata=self.metadata, + instance_state=instance_state) score_dict = combinedoe.get_score() self.assertEqual(score_dict['score'], 15.0) self.assertEqual(score_dict['total'], 15.0) -class OpenEndedModuleXmlTest(unittest.TestCase, DummySystemUser): + +class OpenEndedModuleXmlTest(unittest.TestCase, DummyModulestore): problem_location = Location(["i4x", "edX", "open_ended", "combinedopenended", "SampleQuestion"]) answer = "blah blah" - assessment = [0,1] + assessment = [0, 1] hint = "blah" + def setUp(self): self.test_system = test_system() self.test_system.xqueue['interface'] = Mock( - send_to_queue = Mock(side_effect=[1,"queued"]) - ) + send_to_queue=Mock(side_effect=[1, "queued"]) + ) def test_open_ended_load_and_save(self): module = self.get_module_from_location(self.problem_location, COURSE) - module.handle_ajax("save_answer", {"student_answer" : self.answer}) + module.handle_ajax("save_answer", {"student_answer": self.answer}) task_one_json = json.loads(module.task_states[0]) self.assertEqual(task_one_json['child_history'][0]['answer'], self.answer) def test_open_ended_flow_reset(self): - assessment = [0,1] + assessment = [0, 1] module = self.get_module_from_location(self.problem_location, COURSE) #Simulate a student saving an answer - module.handle_ajax("save_answer", {"student_answer" : self.answer}) + module.handle_ajax("save_answer", {"student_answer": self.answer}) status = module.handle_ajax("get_status", {}) #Mock a student submitting an assessment assessment_dict = MockQueryDict() - assessment_dict.update({'assessment' : sum(assessment), 'score_list[]' : assessment}) + assessment_dict.update({'assessment': sum(assessment), 'score_list[]': assessment}) module.handle_ajax("save_assessment", assessment_dict) task_one_json = json.loads(module.task_states[0]) self.assertEqual(json.loads(task_one_json['child_history'][0]['post_assessment']), assessment) @@ -514,16 +515,16 @@ class OpenEndedModuleXmlTest(unittest.TestCase, DummySystemUser): module.handle_ajax("reset", {}) def test_open_ended_flow_correct(self): - assessment = [1,1] + assessment = [1, 1] module = self.get_module_from_location(self.problem_location, COURSE) #Simulate a student saving an answer - module.handle_ajax("save_answer", {"student_answer" : self.answer}) + module.handle_ajax("save_answer", {"student_answer": self.answer}) status = module.handle_ajax("get_status", {}) #Mock a student submitting an assessment assessment_dict = MockQueryDict() - assessment_dict.update({'assessment' : sum(assessment), 'score_list[]' : assessment}) + assessment_dict.update({'assessment': sum(assessment), 'score_list[]': assessment}) module.handle_ajax("save_assessment", assessment_dict) task_one_json = json.loads(module.task_states[0]) self.assertEqual(json.loads(task_one_json['child_history'][0]['post_assessment']), assessment) @@ -546,16 +547,17 @@ class OpenEndedModuleXmlTest(unittest.TestCase, DummySystemUser): #Make a fake reply from the queue queue_reply = { - 'queuekey' : "", - 'xqueue_body' : json.dumps({ - 'score' : 0, - 'feedback' : json.dumps({"spelling": "Spelling: Ok.", "grammar": "Grammar: Ok.", "markup-text": " all of us can think of a book that we hope none of our children or any other children have taken off the shelf . but if i have the right to remove that book from the shelf that work i abhor then you also have exactly the same right and so does everyone else . and then we have no books left on the shelf for any of us . katherine paterson , author write a persuasive essay to a newspaper reflecting your vies on censorship in libraries . do you believe that certain materials , such as books , music , movies , magazines , etc . , should be removed from the shelves if they are found offensive ? support your position with convincing arguments from your own experience , observations , and or reading . "}), - 'grader_type' : "ML", - 'success' : True, - 'grader_id' : 1, - 'submission_id' : 1, - 'rubric_xml' : "Writing Applications0 Language Conventions 0", - 'rubric_scores_complete' : True, + 'queuekey': "", + 'xqueue_body': json.dumps({ + 'score': 0, + 'feedback': json.dumps({"spelling": "Spelling: Ok.", "grammar": "Grammar: Ok.", + "markup-text": " all of us can think of a book that we hope none of our children or any other children have taken off the shelf . but if i have the right to remove that book from the shelf that work i abhor then you also have exactly the same right and so does everyone else . and then we have no books left on the shelf for any of us . katherine paterson , author write a persuasive essay to a newspaper reflecting your vies on censorship in libraries . do you believe that certain materials , such as books , music , movies , magazines , etc . , should be removed from the shelves if they are found offensive ? support your position with convincing arguments from your own experience , observations , and or reading . "}), + 'grader_type': "ML", + 'success': True, + 'grader_id': 1, + 'submission_id': 1, + 'rubric_xml': "Writing Applications0 Language Conventions 0", + 'rubric_scores_complete': True, }) } diff --git a/common/lib/xmodule/xmodule/tests/test_peer_grading.py b/common/lib/xmodule/xmodule/tests/test_peer_grading.py index 630f693333..d7cd7a4afd 100644 --- a/common/lib/xmodule/xmodule/tests/test_peer_grading.py +++ b/common/lib/xmodule/xmodule/tests/test_peer_grading.py @@ -1,33 +1,33 @@ import unittest from xmodule.modulestore import Location -import json -from lxml import etree -from mock import Mock -from . import test_system -from dummy_system import DummySystem, DummySystemUser -from test_util_open_ended import MockQueryDict +from .import test_system +from test_util_open_ended import MockQueryDict, DummyModulestore from xmodule.peer_grading_module import PeerGradingModule, PeerGradingDescriptor from xmodule.open_ended_grading_classes.grading_service_module import GradingServiceError +import logging + +log = logging.getLogger(__name__) + ORG = "edX" -COURSE="open_ended" +COURSE = "open_ended" -class PeerGradingModuleTest(unittest.TestCase, DummySystemUser): +class PeerGradingModuleTest(unittest.TestCase, DummyModulestore): problem_location = Location(["i4x", "edX", "open_ended", "peergrading", - "PeerGradingSample"]) - calibrated_dict = {'location' : "blah"} + "PeerGradingSample"]) + calibrated_dict = {'location': "blah"} save_dict = MockQueryDict() save_dict.update({ - 'location' : "blah", - 'submission_id' : 1, - 'submission_key' : "", + 'location': "blah", + 'submission_id': 1, + 'submission_key': "", 'score': 1, - 'feedback' : "", - 'rubric_scores[]' : [0,1], + 'feedback': "", + 'rubric_scores[]': [0, 1], 'submission_flagged': False, - }) + }) def setUp(self): self.test_system = test_system() @@ -54,17 +54,16 @@ class PeerGradingModuleTest(unittest.TestCase, DummySystemUser): max_score = self.peer_grading.max_score() def get_next_submission(self): - success, next_submission = self.peer_grading.get_next_submission({'location' : 'blah'}) + success, next_submission = self.peer_grading.get_next_submission({'location': 'blah'}) def test_save_grade(self): self.peer_grading.save_grade(self.save_dict) def test_is_student_calibrated(self): - calibrated_dict = {'location' : "blah"} + calibrated_dict = {'location': "blah"} self.peer_grading.is_student_calibrated(self.calibrated_dict) def test_show_calibration_essay(self): - self.peer_grading.show_calibration_essay(self.calibrated_dict) def test_save_calibration_essay(self): diff --git a/common/lib/xmodule/xmodule/tests/test_util_open_ended.py b/common/lib/xmodule/xmodule/tests/test_util_open_ended.py index 088a5af87d..38f083de38 100644 --- a/common/lib/xmodule/xmodule/tests/test_util_open_ended.py +++ b/common/lib/xmodule/xmodule/tests/test_util_open_ended.py @@ -1,3 +1,8 @@ +from .import test_system +from xmodule.modulestore import Location +from xmodule.modulestore.xml import ImportSystem, XMLModuleStore +from xmodule.tests.test_export import DATA_DIR + OPEN_ENDED_GRADING_INTERFACE = { 'url': 'blah/', 'username': 'incorrect', @@ -17,10 +22,30 @@ class MockQueryDict(dict): """ Mock a query set so that it can be used with default authorization """ + def getlist(self, key, default=None): try: return super(MockQueryDict, self).__getitem__(key) except KeyError: if default is None: return [] - return default \ No newline at end of file + return default + + +class DummyModulestore(object): + test_system = test_system() + + def get_course(self, name): + """Get a test course by directory name. If there's more than one, error.""" + + modulestore = XMLModuleStore(DATA_DIR, course_dirs=[name]) + courses = modulestore.get_courses() + self.modulestore = modulestore + return courses[0] + + def get_module_from_location(self, location, course): + course = self.get_course(course) + if not isinstance(location, Location): + location = Location(location) + descriptor = self.modulestore.get_instance(course.id, location, depth=None) + return descriptor.xmodule(self.test_system) \ No newline at end of file diff --git a/common/test/data/open_ended/course/2012_Fall.xml b/common/test/data/open_ended/course/2012_Fall.xml index 34369979ca..32c810174b 100644 --- a/common/test/data/open_ended/course/2012_Fall.xml +++ b/common/test/data/open_ended/course/2012_Fall.xml @@ -2,5 +2,6 @@ + diff --git a/common/test/data/open_ended/peergrading/PeerGradingScored.xml b/common/test/data/open_ended/peergrading/PeerGradingScored.xml new file mode 100644 index 0000000000..b2380b1e1b --- /dev/null +++ b/common/test/data/open_ended/peergrading/PeerGradingScored.xml @@ -0,0 +1 @@ + \ No newline at end of file From 09d34a02ccd3d36c2b1b2d4b014d100a698c07e4 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Thu, 9 May 2013 09:50:32 -0400 Subject: [PATCH 12/21] Add in some comments --- .../xmodule/tests/test_combined_open_ended.py | 29 +++++++++ .../xmodule/tests/test_peer_grading.py | 59 ++++++++++++++++++- .../xmodule/tests/test_util_open_ended.py | 5 +- 3 files changed, 89 insertions(+), 4 deletions(-) diff --git a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py index 2faecce08d..1d67e94376 100644 --- a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py +++ b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py @@ -35,6 +35,9 @@ OpenEndedModule class OpenEndedChildTest(unittest.TestCase): + """ + Test the open ended child class + """ location = Location(["i4x", "edX", "sa_test", "selfassessment", "SampleQuestion"]) @@ -150,6 +153,9 @@ class OpenEndedChildTest(unittest.TestCase): class OpenEndedModuleTest(unittest.TestCase): + """ + Test the open ended module class + """ location = Location(["i4x", "edX", "sa_test", "selfassessment", "SampleQuestion"]) @@ -291,6 +297,9 @@ class OpenEndedModuleTest(unittest.TestCase): class CombinedOpenEndedModuleTest(unittest.TestCase): + """ + Unit tests for the combined open ended xmodule + """ location = Location(["i4x", "edX", "open_ended", "combinedopenended", "SampleQuestion"]) definition_template = """ @@ -474,6 +483,9 @@ class CombinedOpenEndedModuleTest(unittest.TestCase): class OpenEndedModuleXmlTest(unittest.TestCase, DummyModulestore): + """ + Test the student flow in the combined open ended xmodule + """ problem_location = Location(["i4x", "edX", "open_ended", "combinedopenended", "SampleQuestion"]) answer = "blah blah" assessment = [0, 1] @@ -486,12 +498,23 @@ class OpenEndedModuleXmlTest(unittest.TestCase, DummyModulestore): ) def test_open_ended_load_and_save(self): + """ + See if we can load the module and save an answer + @return: + """ + #Load the module module = self.get_module_from_location(self.problem_location, COURSE) + + #Try saving an answer module.handle_ajax("save_answer", {"student_answer": self.answer}) task_one_json = json.loads(module.task_states[0]) self.assertEqual(task_one_json['child_history'][0]['answer'], self.answer) def test_open_ended_flow_reset(self): + """ + Test the flow of the module if we complete the self assessment step and then reset + @return: + """ assessment = [0, 1] module = self.get_module_from_location(self.problem_location, COURSE) @@ -515,7 +538,13 @@ class OpenEndedModuleXmlTest(unittest.TestCase, DummyModulestore): module.handle_ajax("reset", {}) def test_open_ended_flow_correct(self): + """ + Test a two step problem where the student first goes through the self assessment step, and then the + open ended step. + @return: + """ assessment = [1, 1] + #Load the module module = self.get_module_from_location(self.problem_location, COURSE) #Simulate a student saving an answer diff --git a/common/lib/xmodule/xmodule/tests/test_peer_grading.py b/common/lib/xmodule/xmodule/tests/test_peer_grading.py index d7cd7a4afd..49c696f741 100644 --- a/common/lib/xmodule/xmodule/tests/test_peer_grading.py +++ b/common/lib/xmodule/xmodule/tests/test_peer_grading.py @@ -15,6 +15,10 @@ COURSE = "open_ended" class PeerGradingModuleTest(unittest.TestCase, DummyModulestore): + """ + Test peer grading xmodule at the unit level. More detailed tests are difficult, as the module relies on an + external grading service. + """ problem_location = Location(["i4x", "edX", "open_ended", "peergrading", "PeerGradingSample"]) calibrated_dict = {'location': "blah"} @@ -30,50 +34,99 @@ class PeerGradingModuleTest(unittest.TestCase, DummyModulestore): }) def setUp(self): + """ + Create a peer grading module from a test system + @return: + """ self.test_system = test_system() self.test_system.open_ended_grading_interface = None self.peer_grading = self.get_module_from_location(self.problem_location, COURSE) def test_module_closed(self): + """ + Test if peer grading is closed + @return: + """ closed = self.peer_grading.closed() self.assertEqual(closed, False) def test_get_html(self): + """ + Test to see if the module can be rendered + @return: + """ html = self.peer_grading.get_html() def test_get_data(self): + """ + Try getting data from the external grading service + @return: + """ try: success, data = self.peer_grading.query_data_for_location() except GradingServiceError: pass def test_get_score(self): + """ + Test getting the score + @return: + """ score = self.peer_grading.get_score() def test_get_max_score(self): + """ + Test getting the max score + @return: + """ max_score = self.peer_grading.max_score() def get_next_submission(self): + """ + Test to see if we can get the next mock submission + @return: + """ success, next_submission = self.peer_grading.get_next_submission({'location': 'blah'}) def test_save_grade(self): + """ + Test if we can save the grade + @return: + """ self.peer_grading.save_grade(self.save_dict) def test_is_student_calibrated(self): + """ + Check to see if the student has calibrated yet + @return: + """ calibrated_dict = {'location': "blah"} self.peer_grading.is_student_calibrated(self.calibrated_dict) def test_show_calibration_essay(self): + """ + Test showing the calibration essay + @return: + """ self.peer_grading.show_calibration_essay(self.calibrated_dict) def test_save_calibration_essay(self): + """ + Test saving the calibration essay + @return: + """ self.peer_grading.save_calibration_essay(self.save_dict) - def test_peer_grading_closed(self): - self.peer_grading.peer_grading_closed() - def test_peer_grading_problem(self): + """ + See if we can render a single problem + @return: + """ self.peer_grading.peer_grading_problem(self.calibrated_dict) def test_get_instance_state(self): + """ + Get the instance state dict + @return: + """ self.peer_grading.get_instance_state() \ No newline at end of file diff --git a/common/lib/xmodule/xmodule/tests/test_util_open_ended.py b/common/lib/xmodule/xmodule/tests/test_util_open_ended.py index 38f083de38..f269a8a002 100644 --- a/common/lib/xmodule/xmodule/tests/test_util_open_ended.py +++ b/common/lib/xmodule/xmodule/tests/test_util_open_ended.py @@ -20,7 +20,7 @@ S3_INTERFACE = { class MockQueryDict(dict): """ - Mock a query set so that it can be used with default authorization + Mock a query dict so that it can be used in test classes """ def getlist(self, key, default=None): @@ -33,6 +33,9 @@ class MockQueryDict(dict): class DummyModulestore(object): + """ + A mixin that allows test classes to have convenience functions to get a module given a location + """ test_system = test_system() def get_course(self, name): From 8c12eb78c92c76b26b6d42ba16362e60a84f078a Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Thu, 9 May 2013 09:54:57 -0400 Subject: [PATCH 13/21] Fix some exceptions --- .../xmodule/open_ended_grading_classes/open_ended_module.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py index afdfeef6de..a4b4afe499 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py @@ -496,8 +496,8 @@ class OpenEndedModule(openendedchild.OpenEndedChild): grader_types.append(score_result['grader_type']) try: feedback_dict = json.loads(score_result['feedback'][i]) - except: - pass + except Exception: + feedback_dict = score_result['feedback'][i] feedback_dicts.append(feedback_dict) grader_ids.append(score_result['grader_id'][i]) submission_ids.append(score_result['submission_id']) @@ -515,7 +515,7 @@ class OpenEndedModule(openendedchild.OpenEndedChild): feedback_items = [feedback] try: feedback_dict = json.loads(score_result['feedback']) - except: + except Exception: feedback_dict = score_result.get('feedback', '') feedback_dicts = [feedback_dict] grader_ids = [score_result['grader_id']] From 7a1ef62ee31fe7d52fc619514ac4dbc31f1bddce Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Thu, 9 May 2013 09:56:02 -0400 Subject: [PATCH 14/21] Do some code reformatting --- .../open_ended_module.py | 14 ++++----- .../openendedchild.py | 12 ++++---- .../peer_grading_service.py | 30 +++++++++---------- 3 files changed, 28 insertions(+), 28 deletions(-) diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py index a4b4afe499..266d332a7f 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py @@ -191,7 +191,7 @@ class OpenEndedModule(openendedchild.OpenEndedChild): } (error, msg) = qinterface.send_to_queue(header=xheader, - body=json.dumps(contents)) + body=json.dumps(contents)) #Convert error to a success value success = True @@ -225,8 +225,8 @@ class OpenEndedModule(openendedchild.OpenEndedChild): str(len(self.child_history))) xheader = xqueue_interface.make_xheader(lms_callback_url=system.xqueue['construct_callback'](), - lms_key=queuekey, - queue_name=self.queue_name) + lms_key=queuekey, + queue_name=self.queue_name) contents = self.payload.copy() @@ -244,7 +244,7 @@ class OpenEndedModule(openendedchild.OpenEndedChild): # Submit request. When successful, 'msg' is the prior length of the queue qinterface.send_to_queue(header=xheader, - body=json.dumps(contents)) + body=json.dumps(contents)) # State associated with the queueing request queuestate = {'key': queuekey, @@ -402,7 +402,7 @@ class OpenEndedModule(openendedchild.OpenEndedChild): if not response_items['success']: return system.render_template("{0}/open_ended_error.html".format(self.TEMPLATE_DIR), - {'errors': feedback}) + {'errors': feedback}) feedback_template = system.render_template("{0}/open_ended_feedback.html".format(self.TEMPLATE_DIR), { 'grader_type': response_items['grader_type'], @@ -546,7 +546,7 @@ class OpenEndedModule(openendedchild.OpenEndedChild): return "" feedback_dict = self._parse_score_msg(self.child_history[-1].get('post_assessment', ""), system, - join_feedback=join_feedback) + join_feedback=join_feedback) if not short_feedback: return feedback_dict['feedback'] if feedback_dict['valid'] else '' if feedback_dict['valid']: @@ -711,7 +711,7 @@ class OpenEndedDescriptor(): template_dir_name = "openended" def __init__(self, system): - self.system =system + self.system = system @classmethod def definition_from_xml(cls, xml_object, system): diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/openendedchild.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/openendedchild.py index d5889636ed..2d8d3805f1 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/openendedchild.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/openendedchild.py @@ -102,7 +102,7 @@ class OpenEndedChild(object): if system.open_ended_grading_interface: self.peer_gs = PeerGradingService(system.open_ended_grading_interface, system) self.controller_qs = controller_query_service.ControllerQueryService(system.open_ended_grading_interface, - system) + system) else: self.peer_gs = MockPeerGradingService() self.controller_qs = None @@ -180,8 +180,8 @@ class OpenEndedChild(object): try: answer = autolink_html(answer) cleaner = Cleaner(style=True, links=True, add_nofollow=False, page_structure=True, safe_attrs_only=True, - host_whitelist=open_ended_image_submission.TRUSTED_IMAGE_DOMAINS, - whitelist_tags=set(['embed', 'iframe', 'a', 'img'])) + host_whitelist=open_ended_image_submission.TRUSTED_IMAGE_DOMAINS, + whitelist_tags=set(['embed', 'iframe', 'a', 'img'])) clean_html = cleaner.clean_html(answer) clean_html = re.sub(r'

$', '', re.sub(r'^

', '', clean_html)) except: @@ -282,7 +282,7 @@ class OpenEndedChild(object): """ #This is a dev_facing_error log.warning("Open ended child state out sync. state: %r, get: %r. %s", - self.child_state, get, msg) + self.child_state, get, msg) #This is a student_facing_error return {'success': False, 'error': 'The problem state got out-of-sync. Please try reloading the page.'} @@ -343,7 +343,7 @@ class OpenEndedChild(object): try: image_data.seek(0) success, s3_public_url = open_ended_image_submission.upload_to_s3(image_data, image_key, - self.s3_interface) + self.s3_interface) except: log.exception("Could not upload image to S3.") @@ -462,7 +462,7 @@ class OpenEndedChild(object): allowed_to_submit = False #This is a student_facing_error error_message = error_string.format(count_required - count_graded, count_graded, count_required, - student_sub_count) + student_sub_count) return success, allowed_to_submit, error_message def get_eta(self): diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/peer_grading_service.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/peer_grading_service.py index 418784f618..56bd1ec0a8 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/peer_grading_service.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/peer_grading_service.py @@ -37,7 +37,7 @@ class PeerGradingService(GradingService): def get_next_submission(self, problem_location, grader_id): response = self.get(self.get_next_submission_url, - {'location': problem_location, 'grader_id': grader_id}) + {'location': problem_location, 'grader_id': grader_id}) return self.try_to_decode(self._render_rubric(response)) def save_grade(self, location, grader_id, submission_id, score, feedback, submission_key, rubric_scores, @@ -101,12 +101,12 @@ without making actual service calls to the grading controller class MockPeerGradingService(object): def get_next_submission(self, problem_location, grader_id): return {'success': True, - 'submission_id': 1, - 'submission_key': "", - 'student_response': 'fake student response', - 'prompt': 'fake submission prompt', - 'rubric': 'fake rubric', - 'max_score': 4} + 'submission_id': 1, + 'submission_key': "", + 'student_response': 'fake student response', + 'prompt': 'fake submission prompt', + 'rubric': 'fake rubric', + 'max_score': 4} def save_grade(self, location, grader_id, submission_id, score, feedback, submission_key, rubric_scores, submission_flagged): @@ -117,12 +117,12 @@ class MockPeerGradingService(object): def show_calibration_essay(self, problem_location, grader_id): return {'success': True, - 'submission_id': 1, - 'submission_key': '', - 'student_response': 'fake student response', - 'prompt': 'fake submission prompt', - 'rubric': 'fake rubric', - 'max_score': 4} + 'submission_id': 1, + 'submission_key': '', + 'student_response': 'fake student response', + 'prompt': 'fake submission prompt', + 'rubric': 'fake rubric', + 'max_score': 4} def save_calibration_essay(self, problem_location, grader_id, calibration_essay_id, submission_key, score, @@ -131,8 +131,8 @@ class MockPeerGradingService(object): def get_problem_list(self, course_id, grader_id): return {'success': True, - 'problem_list': [ - ]} + 'problem_list': [ + ]} def get_data_for_location(self, problem_location, student_id): return {"version": 1, "count_graded": 3, "count_required": 3, "success": True, "student_sub_count": 1} From f9e97cb935e943baed19f1192e6dc371952245eb Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Thu, 9 May 2013 10:05:48 -0400 Subject: [PATCH 15/21] Add test for proper saving --- .../xmodule/tests/test_combined_open_ended.py | 5 ++++ .../xmodule/tests/test_peer_grading.py | 23 ++++++++++++++++++- .../xmodule/tests/test_util_open_ended.py | 8 +++---- .../peergrading/PeerGradingScored.xml | 2 +- 4 files changed, 32 insertions(+), 6 deletions(-) diff --git a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py index 1d67e94376..48ea6e7911 100644 --- a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py +++ b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py @@ -496,6 +496,7 @@ class OpenEndedModuleXmlTest(unittest.TestCase, DummyModulestore): self.test_system.xqueue['interface'] = Mock( send_to_queue=Mock(side_effect=[1, "queued"]) ) + self.setup_modulestore(COURSE) def test_open_ended_load_and_save(self): """ @@ -510,6 +511,10 @@ class OpenEndedModuleXmlTest(unittest.TestCase, DummyModulestore): task_one_json = json.loads(module.task_states[0]) self.assertEqual(task_one_json['child_history'][0]['answer'], self.answer) + module = self.get_module_from_location(self.problem_location, COURSE) + task_one_json = json.loads(module.task_states[0]) + self.assertEqual(task_one_json['child_history'][0]['answer'], self.answer) + def test_open_ended_flow_reset(self): """ Test the flow of the module if we complete the self assessment step and then reset diff --git a/common/lib/xmodule/xmodule/tests/test_peer_grading.py b/common/lib/xmodule/xmodule/tests/test_peer_grading.py index 49c696f741..036ab4a85b 100644 --- a/common/lib/xmodule/xmodule/tests/test_peer_grading.py +++ b/common/lib/xmodule/xmodule/tests/test_peer_grading.py @@ -40,6 +40,7 @@ class PeerGradingModuleTest(unittest.TestCase, DummyModulestore): """ self.test_system = test_system() self.test_system.open_ended_grading_interface = None + self.setup_modulestore(COURSE) self.peer_grading = self.get_module_from_location(self.problem_location, COURSE) def test_module_closed(self): @@ -129,4 +130,24 @@ class PeerGradingModuleTest(unittest.TestCase, DummyModulestore): Get the instance state dict @return: """ - self.peer_grading.get_instance_state() \ No newline at end of file + self.peer_grading.get_instance_state() + +class PeerGradingModuleScoredTest(unittest.TestCase, DummyModulestore): + """ + Test peer grading xmodule at the unit level. More detailed tests are difficult, as the module relies on an + external grading service. + """ + problem_location = Location(["i4x", "edX", "open_ended", "peergrading", + "PeerGradingScored"]) + def setUp(self): + """ + Create a peer grading module from a test system + @return: + """ + self.test_system = test_system() + self.test_system.open_ended_grading_interface = None + self.setup_modulestore(COURSE) + + def test_metadata_load(self): + peer_grading = self.get_module_from_location(self.problem_location, COURSE) + self.assertEqual(peer_grading.closed(), False) \ No newline at end of file diff --git a/common/lib/xmodule/xmodule/tests/test_util_open_ended.py b/common/lib/xmodule/xmodule/tests/test_util_open_ended.py index f269a8a002..3737586232 100644 --- a/common/lib/xmodule/xmodule/tests/test_util_open_ended.py +++ b/common/lib/xmodule/xmodule/tests/test_util_open_ended.py @@ -38,12 +38,12 @@ class DummyModulestore(object): """ test_system = test_system() + def setup_modulestore(self, name): + self.modulestore = XMLModuleStore(DATA_DIR, course_dirs=[name]) + def get_course(self, name): """Get a test course by directory name. If there's more than one, error.""" - - modulestore = XMLModuleStore(DATA_DIR, course_dirs=[name]) - courses = modulestore.get_courses() - self.modulestore = modulestore + courses = self.modulestore.get_courses() return courses[0] def get_module_from_location(self, location, course): diff --git a/common/test/data/open_ended/peergrading/PeerGradingScored.xml b/common/test/data/open_ended/peergrading/PeerGradingScored.xml index b2380b1e1b..6398a9b4c5 100644 --- a/common/test/data/open_ended/peergrading/PeerGradingScored.xml +++ b/common/test/data/open_ended/peergrading/PeerGradingScored.xml @@ -1 +1 @@ - \ No newline at end of file + \ No newline at end of file From f72659fa2e34fabda38260f75b250585e6e71800 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Thu, 9 May 2013 10:15:07 -0400 Subject: [PATCH 16/21] Add in asserts --- .../xmodule/tests/test_peer_grading.py | 25 ++++++++++++------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/common/lib/xmodule/xmodule/tests/test_peer_grading.py b/common/lib/xmodule/xmodule/tests/test_peer_grading.py index 036ab4a85b..a0877eab81 100644 --- a/common/lib/xmodule/xmodule/tests/test_peer_grading.py +++ b/common/lib/xmodule/xmodule/tests/test_peer_grading.py @@ -2,6 +2,7 @@ import unittest from xmodule.modulestore import Location from .import test_system from test_util_open_ended import MockQueryDict, DummyModulestore +import json from xmodule.peer_grading_module import PeerGradingModule, PeerGradingDescriptor from xmodule.open_ended_grading_classes.grading_service_module import GradingServiceError @@ -63,10 +64,8 @@ class PeerGradingModuleTest(unittest.TestCase, DummyModulestore): Try getting data from the external grading service @return: """ - try: - success, data = self.peer_grading.query_data_for_location() - except GradingServiceError: - pass + success, data = self.peer_grading.query_data_for_location() + self.assertEqual(success, True) def test_get_score(self): """ @@ -74,6 +73,7 @@ class PeerGradingModuleTest(unittest.TestCase, DummyModulestore): @return: """ score = self.peer_grading.get_score() + self.assertEquals(score['score'], None) def test_get_max_score(self): """ @@ -81,6 +81,7 @@ class PeerGradingModuleTest(unittest.TestCase, DummyModulestore): @return: """ max_score = self.peer_grading.max_score() + self.assertEquals(max_score, None) def get_next_submission(self): """ @@ -88,13 +89,15 @@ class PeerGradingModuleTest(unittest.TestCase, DummyModulestore): @return: """ success, next_submission = self.peer_grading.get_next_submission({'location': 'blah'}) + self.assertEqual(success, True) def test_save_grade(self): """ Test if we can save the grade @return: """ - self.peer_grading.save_grade(self.save_dict) + response = self.peer_grading.save_grade(self.save_dict) + self.assertEqual(response['success'], True) def test_is_student_calibrated(self): """ @@ -102,28 +105,32 @@ class PeerGradingModuleTest(unittest.TestCase, DummyModulestore): @return: """ calibrated_dict = {'location': "blah"} - self.peer_grading.is_student_calibrated(self.calibrated_dict) + response = self.peer_grading.is_student_calibrated(self.calibrated_dict) + self.assertEqual(response['success'], True) def test_show_calibration_essay(self): """ Test showing the calibration essay @return: """ - self.peer_grading.show_calibration_essay(self.calibrated_dict) + response = self.peer_grading.show_calibration_essay(self.calibrated_dict) + self.assertEqual(response['success'], True) def test_save_calibration_essay(self): """ Test saving the calibration essay @return: """ - self.peer_grading.save_calibration_essay(self.save_dict) + response = self.peer_grading.save_calibration_essay(self.save_dict) + self.assertEqual(response['success'], True) def test_peer_grading_problem(self): """ See if we can render a single problem @return: """ - self.peer_grading.peer_grading_problem(self.calibrated_dict) + response = self.peer_grading.peer_grading_problem(self.calibrated_dict) + self.assertEqual(response['success'], True) def test_get_instance_state(self): """ From 682d06bcff1fd07b8ba158ee18562e0b729073b3 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Thu, 9 May 2013 11:27:44 -0400 Subject: [PATCH 17/21] Fix a lot of pep8 violations --- .../xmodule/combined_open_ended_module.py | 13 ++-- .../combined_open_ended_modulev1.py | 12 ++- .../grading_service_module.py | 3 +- .../open_ended_module.py | 76 ++++++++++++------- .../openendedchild.py | 15 ++-- .../xmodule/xmodule/peer_grading_module.py | 1 - .../xmodule/tests/test_combined_open_ended.py | 56 +++++++------- .../xmodule/tests/test_util_open_ended.py | 4 +- 8 files changed, 99 insertions(+), 81 deletions(-) diff --git a/common/lib/xmodule/xmodule/combined_open_ended_module.py b/common/lib/xmodule/xmodule/combined_open_ended_module.py index 67ff206e89..f4074283fe 100644 --- a/common/lib/xmodule/xmodule/combined_open_ended_module.py +++ b/common/lib/xmodule/xmodule/combined_open_ended_module.py @@ -104,11 +104,14 @@ class CombinedOpenEndedModule(CombinedOpenEndedFields, XModule): icon_class = 'problem' - js = {'coffee': - [resource_string(__name__, 'js/src/combinedopenended/display.coffee'), - resource_string(__name__, 'js/src/collapsible.coffee'), - resource_string(__name__, 'js/src/javascript_loader.coffee'), - ]} + js = { + 'coffee': + [ + resource_string(__name__, 'js/src/combinedopenended/display.coffee'), + resource_string(__name__, 'js/src/collapsible.coffee'), + resource_string(__name__, 'js/src/javascript_loader.coffee'), + ] + } js_module_name = "CombinedOpenEnded" css = {'scss': [resource_string(__name__, 'css/combinedopenended/display.scss')]} diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py index 6767851d3a..1404f52300 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py @@ -294,9 +294,8 @@ class CombinedOpenEndedV1Module(): if self.current_task_number > 0: last_response_data = self.get_last_response(self.current_task_number - 1) current_response_data = self.get_current_attributes(self.current_task_number) - if (current_response_data['min_score_to_attempt'] > last_response_data['score'] - or current_response_data['max_score_to_attempt'] < last_response_data['score']): + or current_response_data['max_score_to_attempt'] < last_response_data['score']): self.state = self.DONE self.ready_to_reset = True @@ -662,9 +661,10 @@ class CombinedOpenEndedV1Module(): return { 'success': False, #This is a student_facing_error - 'error': ('You have attempted this question {0} times. ' - 'You are only allowed to attempt it {1} times.').format( - self.student_attempts, self.attempts) + 'error': ( + 'You have attempted this question {0} times. ' + 'You are only allowed to attempt it {1} times.' + ).format(self.student_attempts, self.attempts) } self.state = self.INITIAL self.ready_to_reset = False @@ -815,7 +815,6 @@ class CombinedOpenEndedV1Module(): 'error': 'The problem state got out-of-sync. Please try reloading the page.'} - class CombinedOpenEndedV1Descriptor(): """ Module for adding combined open ended questions @@ -861,7 +860,6 @@ class CombinedOpenEndedV1Descriptor(): return {'task_xml': parse_task('task'), 'prompt': parse('prompt'), 'rubric': parse('rubric')} - def definition_to_xml(self, resource_fs): '''Return an xml element representing this definition.''' elt = etree.Element('combinedopenended') diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/grading_service_module.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/grading_service_module.py index f3f6568b1e..b16f0618bb 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/grading_service_module.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/grading_service_module.py @@ -76,7 +76,6 @@ class GradingService(object): return r.text - def _try_with_login(self, operation): """ Call operation(), which should return a requests response object. If @@ -87,7 +86,7 @@ class GradingService(object): """ response = operation() if (response.json - and response.json.get('success') == False + and response.json.get('success') is False and response.json.get('error') == 'login_required'): # apparrently we aren't logged in. Try to fix that. r = self._login() diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py index 266d332a7f..7ba046b2ad 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py @@ -72,7 +72,7 @@ class OpenEndedModule(openendedchild.OpenEndedChild): self._parse(oeparam, self.child_prompt, self.child_rubric, system) - if self.child_created == True and self.child_state == self.ASSESSING: + if self.child_created is True and self.child_state == self.ASSESSING: self.child_created = False self.send_to_grader(self.latest_answer(), system) self.child_created = False @@ -159,9 +159,11 @@ class OpenEndedModule(openendedchild.OpenEndedChild): score = int(survey_responses['score']) except: #This is a dev_facing_error - error_message = ("Could not parse submission id, grader id, " - "or feedback from message_post ajax call. Here is the message data: {0}".format( - survey_responses)) + error_message = ( + "Could not parse submission id, grader id, " + "or feedback from message_post ajax call. " + "Here is the message data: {0}".format(survey_responses) + ) log.exception(error_message) #This is a student_facing_error return {'success': False, 'msg': "There was an error saving your feedback. Please contact course staff."} @@ -179,8 +181,9 @@ class OpenEndedModule(openendedchild.OpenEndedChild): queue_name=self.message_queue_name ) - student_info = {'anonymous_student_id': anonymous_student_id, - 'submission_time': qtime, + student_info = { + 'anonymous_student_id': anonymous_student_id, + 'submission_time': qtime, } contents = { 'feedback': feedback, @@ -190,8 +193,10 @@ class OpenEndedModule(openendedchild.OpenEndedChild): 'student_info': json.dumps(student_info), } - (error, msg) = qinterface.send_to_queue(header=xheader, - body=json.dumps(contents)) + (error, msg) = qinterface.send_to_queue( + header=xheader, + body=json.dumps(contents) + ) #Convert error to a success value success = True @@ -224,15 +229,18 @@ class OpenEndedModule(openendedchild.OpenEndedChild): anonymous_student_id + str(len(self.child_history))) - xheader = xqueue_interface.make_xheader(lms_callback_url=system.xqueue['construct_callback'](), + xheader = xqueue_interface.make_xheader( + lms_callback_url=system.xqueue['construct_callback'](), lms_key=queuekey, - queue_name=self.queue_name) + queue_name=self.queue_name + ) contents = self.payload.copy() # Metadata related to the student submission revealed to the external grader - student_info = {'anonymous_student_id': anonymous_student_id, - 'submission_time': qtime, + student_info = { + 'anonymous_student_id': anonymous_student_id, + 'submission_time': qtime, } #Update contents with student response and student info @@ -243,12 +251,16 @@ class OpenEndedModule(openendedchild.OpenEndedChild): }) # Submit request. When successful, 'msg' is the prior length of the queue - qinterface.send_to_queue(header=xheader, - body=json.dumps(contents)) + qinterface.send_to_queue( + header=xheader, + body=json.dumps(contents) + ) # State associated with the queueing request - queuestate = {'key': queuekey, - 'time': qtime, } + queuestate = { + 'key': queuekey, + 'time': qtime, + } return True def _update_score(self, score_msg, queuekey, system): @@ -302,11 +314,13 @@ class OpenEndedModule(openendedchild.OpenEndedChild): # We want to display available feedback in a particular order. # This dictionary specifies which goes first--lower first. - priorities = {# These go at the start of the feedback - 'spelling': 0, - 'grammar': 1, - # needs to be after all the other feedback - 'markup_text': 3} + priorities = { + # These go at the start of the feedback + 'spelling': 0, + 'grammar': 1, + # needs to be after all the other feedback + 'markup_text': 3 + } do_not_render = ['topicality', 'prompt-overlap'] default_priority = 2 @@ -393,7 +407,7 @@ class OpenEndedModule(openendedchild.OpenEndedChild): rubric_feedback = "" feedback = self._convert_longform_feedback_to_html(response_items) rubric_scores = [] - if response_items['rubric_scores_complete'] == True: + if response_items['rubric_scores_complete'] is True: rubric_renderer = CombinedOpenEndedRubric(system, True) rubric_dict = rubric_renderer.render_rubric(response_items['rubric_xml']) success = rubric_dict['success'] @@ -401,8 +415,10 @@ class OpenEndedModule(openendedchild.OpenEndedChild): rubric_scores = rubric_dict['rubric_scores'] if not response_items['success']: - return system.render_template("{0}/open_ended_error.html".format(self.TEMPLATE_DIR), - {'errors': feedback}) + return system.render_template( + "{0}/open_ended_error.html".format(self.TEMPLATE_DIR), + {'errors': feedback} + ) feedback_template = system.render_template("{0}/open_ended_feedback.html".format(self.TEMPLATE_DIR), { 'grader_type': response_items['grader_type'], @@ -545,8 +561,11 @@ class OpenEndedModule(openendedchild.OpenEndedChild): if not self.child_history: return "" - feedback_dict = self._parse_score_msg(self.child_history[-1].get('post_assessment', ""), system, - join_feedback=join_feedback) + feedback_dict = self._parse_score_msg( + self.child_history[-1].get('post_assessment', ""), + system, + join_feedback=join_feedback + ) if not short_feedback: return feedback_dict['feedback'] if feedback_dict['valid'] else '' if feedback_dict['valid']: @@ -734,8 +753,9 @@ class OpenEndedDescriptor(): """Assumes that xml_object has child k""" return xml_object.xpath(k)[0] - return {'oeparam': parse('openendedparam')} - + return { + 'oeparam': parse('openendedparam') + } def definition_to_xml(self, resource_fs): '''Return an xml element representing this definition.''' diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/openendedchild.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/openendedchild.py index 2d8d3805f1..7dc8d99451 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/openendedchild.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/openendedchild.py @@ -101,8 +101,9 @@ class OpenEndedChild(object): # completion (doesn't matter if you self-assessed correct/incorrect). if system.open_ended_grading_interface: self.peer_gs = PeerGradingService(system.open_ended_grading_interface, system) - self.controller_qs = controller_query_service.ControllerQueryService(system.open_ended_grading_interface, - system) + self.controller_qs = controller_query_service.ControllerQueryService( + system.open_ended_grading_interface,system + ) else: self.peer_gs = MockPeerGradingService() self.controller_qs = None @@ -180,8 +181,8 @@ class OpenEndedChild(object): try: answer = autolink_html(answer) cleaner = Cleaner(style=True, links=True, add_nofollow=False, page_structure=True, safe_attrs_only=True, - host_whitelist=open_ended_image_submission.TRUSTED_IMAGE_DOMAINS, - whitelist_tags=set(['embed', 'iframe', 'a', 'img'])) + host_whitelist=open_ended_image_submission.TRUSTED_IMAGE_DOMAINS, + whitelist_tags=set(['embed', 'iframe', 'a', 'img'])) clean_html = cleaner.clean_html(answer) clean_html = re.sub(r'

$', '', re.sub(r'^

', '', clean_html)) except: @@ -282,7 +283,7 @@ class OpenEndedChild(object): """ #This is a dev_facing_error log.warning("Open ended child state out sync. state: %r, get: %r. %s", - self.child_state, get, msg) + self.child_state, get, msg) #This is a student_facing_error return {'success': False, 'error': 'The problem state got out-of-sync. Please try reloading the page.'} @@ -343,7 +344,7 @@ class OpenEndedChild(object): try: image_data.seek(0) success, s3_public_url = open_ended_image_submission.upload_to_s3(image_data, image_key, - self.s3_interface) + self.s3_interface) except: log.exception("Could not upload image to S3.") @@ -462,7 +463,7 @@ class OpenEndedChild(object): allowed_to_submit = False #This is a student_facing_error error_message = error_string.format(count_required - count_graded, count_graded, count_required, - student_sub_count) + student_sub_count) return success, allowed_to_submit, error_message def get_eta(self): diff --git a/common/lib/xmodule/xmodule/peer_grading_module.py b/common/lib/xmodule/xmodule/peer_grading_module.py index 1ad31922f5..eebfbe22e5 100644 --- a/common/lib/xmodule/xmodule/peer_grading_module.py +++ b/common/lib/xmodule/xmodule/peer_grading_module.py @@ -498,7 +498,6 @@ class PeerGradingModule(PeerGradingFields, XModule): log.error("Problem {0} does not exist in this course".format(location)) raise - for problem in problem_list: problem_location = problem['location'] descriptor = _find_corresponding_module_for_location(problem_location) diff --git a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py index 48ea6e7911..d8f4fbbca1 100644 --- a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py +++ b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py @@ -18,7 +18,7 @@ import logging log = logging.getLogger(__name__) -from .import test_system +from . import test_system ORG = 'edX' COURSE = 'open_ended' # name of directory with course data @@ -70,8 +70,7 @@ class OpenEndedChildTest(unittest.TestCase): def setUp(self): self.test_system = test_system() self.openendedchild = OpenEndedChild(self.test_system, self.location, - self.definition, self.descriptor, self.static_data, self.metadata) - + self.definition, self.descriptor, self.static_data, self.metadata) def test_latest_answer_empty(self): answer = self.openendedchild.latest_answer() @@ -117,7 +116,7 @@ class OpenEndedChildTest(unittest.TestCase): post_assessment = "Post assessment" self.openendedchild.record_latest_post_assessment(post_assessment) self.assertEqual(post_assessment, - self.openendedchild.latest_post_assessment(self.test_system)) + self.openendedchild.latest_post_assessment(self.test_system)) def test_get_score(self): new_answer = "New Answer" @@ -144,12 +143,12 @@ class OpenEndedChildTest(unittest.TestCase): self.openendedchild.new_history_entry(new_answer) self.openendedchild.record_latest_score(self.static_data['max_score']) self.assertEqual(self.openendedchild.is_last_response_correct(), - 'correct') + 'correct') self.openendedchild.new_history_entry(new_answer) self.openendedchild.record_latest_score(0) self.assertEqual(self.openendedchild.is_last_response_correct(), - 'incorrect') + 'incorrect') class OpenEndedModuleTest(unittest.TestCase): @@ -207,7 +206,7 @@ class OpenEndedModuleTest(unittest.TestCase): 'default_queuename': 'testqueue', 'waittime': 1} self.openendedmodule = OpenEndedModule(self.test_system, self.location, - self.definition, self.descriptor, self.static_data, self.metadata) + self.definition, self.descriptor, self.static_data, self.metadata) def test_message_post(self): get = {'feedback': 'feedback text', @@ -372,21 +371,20 @@ class CombinedOpenEndedModuleTest(unittest.TestCase): descriptor = Mock(data=full_definition) test_system = test_system() combinedoe_container = CombinedOpenEndedModule(test_system, - location, - descriptor, - model_data={'data': full_definition, 'weight': '1'}) - + location, + descriptor, + model_data={'data': full_definition, 'weight': '1'}) def setUp(self): # TODO: this constructor call is definitely wrong, but neither branch # of the merge matches the module constructor. Someone (Vik?) should fix this. self.combinedoe = CombinedOpenEndedV1Module(self.test_system, - self.location, - self.definition, - self.descriptor, - static_data=self.static_data, - metadata=self.metadata, - instance_state=self.static_data) + self.location, + self.definition, + self.descriptor, + static_data=self.static_data, + metadata=self.metadata, + instance_state=self.static_data) def test_get_tag_name(self): name = self.combinedoe.get_tag_name("Tag") @@ -441,12 +439,12 @@ class CombinedOpenEndedModuleTest(unittest.TestCase): definition = {'prompt': etree.XML(self.prompt), 'rubric': etree.XML(self.rubric), 'task_xml': xml} descriptor = Mock(data=definition) combinedoe = CombinedOpenEndedV1Module(self.test_system, - self.location, - definition, - descriptor, - static_data=self.static_data, - metadata=self.metadata, - instance_state=self.static_data) + self.location, + definition, + descriptor, + static_data=self.static_data, + metadata=self.metadata, + instance_state=self.static_data) changed = combinedoe.update_task_states() self.assertFalse(changed) @@ -471,12 +469,12 @@ class CombinedOpenEndedModuleTest(unittest.TestCase): 'task_xml': [self.task_xml1, self.task_xml2]} descriptor = Mock(data=definition) combinedoe = CombinedOpenEndedV1Module(self.test_system, - self.location, - definition, - descriptor, - static_data=self.static_data, - metadata=self.metadata, - instance_state=instance_state) + self.location, + definition, + descriptor, + static_data=self.static_data, + metadata=self.metadata, + instance_state=instance_state) score_dict = combinedoe.get_score() self.assertEqual(score_dict['score'], 15.0) self.assertEqual(score_dict['total'], 15.0) diff --git a/common/lib/xmodule/xmodule/tests/test_util_open_ended.py b/common/lib/xmodule/xmodule/tests/test_util_open_ended.py index 3737586232..42d6410ebd 100644 --- a/common/lib/xmodule/xmodule/tests/test_util_open_ended.py +++ b/common/lib/xmodule/xmodule/tests/test_util_open_ended.py @@ -18,11 +18,11 @@ S3_INTERFACE = { "aws_bucket_name": "", } + class MockQueryDict(dict): """ Mock a query dict so that it can be used in test classes """ - def getlist(self, key, default=None): try: return super(MockQueryDict, self).__getitem__(key) @@ -51,4 +51,4 @@ class DummyModulestore(object): if not isinstance(location, Location): location = Location(location) descriptor = self.modulestore.get_instance(course.id, location, depth=None) - return descriptor.xmodule(self.test_system) \ No newline at end of file + return descriptor.xmodule(self.test_system) From a5549fd7fbc74b56d6ef22b3697e2c2ab5dc8bcb Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Mon, 13 May 2013 12:25:23 -0400 Subject: [PATCH 18/21] Add in some assert statements --- .../xmodule/tests/test_combined_open_ended.py | 27 ++++++++++++++----- 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py index d8f4fbbca1..16406e24fc 100644 --- a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py +++ b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py @@ -524,6 +524,7 @@ class OpenEndedModuleXmlTest(unittest.TestCase, DummyModulestore): #Simulate a student saving an answer module.handle_ajax("save_answer", {"student_answer": self.answer}) status = module.handle_ajax("get_status", {}) + self.assertTrue(isinstance(status, basestring)) #Mock a student submitting an assessment assessment_dict = MockQueryDict() @@ -531,14 +532,21 @@ class OpenEndedModuleXmlTest(unittest.TestCase, DummyModulestore): module.handle_ajax("save_assessment", assessment_dict) task_one_json = json.loads(module.task_states[0]) self.assertEqual(json.loads(task_one_json['child_history'][0]['post_assessment']), assessment) - module.handle_ajax("get_status", {}) + status = module.handle_ajax("get_status", {}) + self.assertTrue(isinstance(status, basestring)) #Move to the next step in the problem module.handle_ajax("next_problem", {}) - module.get_html() - module.handle_ajax("get_combined_rubric", {}) + self.assertEqual(module.current_task_number,0) + html = module.get_html() + self.assertTrue(isinstance(html, basestring)) + + rubric = module.handle_ajax("get_combined_rubric", {}) + self.assertTrue(isinstance(rubric, basestring)) + self.assertEqual(module.state, "assessing") module.handle_ajax("reset", {}) + self.assertEqual(module.current_task_number, 0) def test_open_ended_flow_correct(self): """ @@ -553,6 +561,7 @@ class OpenEndedModuleXmlTest(unittest.TestCase, DummyModulestore): #Simulate a student saving an answer module.handle_ajax("save_answer", {"student_answer": self.answer}) status = module.handle_ajax("get_status", {}) + self.assertTrue(isinstance(status, basestring)) #Mock a student submitting an assessment assessment_dict = MockQueryDict() @@ -568,6 +577,7 @@ class OpenEndedModuleXmlTest(unittest.TestCase, DummyModulestore): except GradingServiceError: #This error is okay. We don't have a grading service to connect to! pass + self.assertEqual(module.current_task_number,1) try: module.get_html() except GradingServiceError: @@ -597,16 +607,21 @@ class OpenEndedModuleXmlTest(unittest.TestCase, DummyModulestore): #Update the module with the fake queue reply module.handle_ajax("score_update", queue_reply) + self.assertFalse(module.ready_to_reset) + self.assertEqual(module.current_task_number,1) #Get html and other data client will request html = module.get_html() legend = module.handle_ajax("get_legend", {}) + self.assertTrue(isinstance(legend, basestring)) + status = module.handle_ajax("get_status", {}) - legend = module.handle_ajax("skip_post_assessment", {}) + module.handle_ajax("skip_post_assessment", {}) + self.assertTrue(isinstance(legend, basestring)) #Get all results - legend = module.handle_ajax("get_results", {}) + module.handle_ajax("get_results", {}) - log.info(module.task_states) #reset the problem module.handle_ajax("reset", {}) + self.assertEqual(module.state, "initial") From df07192a6861d2a1902f41e6fa100a849622fb0d Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Mon, 13 May 2013 12:42:30 -0400 Subject: [PATCH 19/21] Querydict comment. --- common/lib/xmodule/xmodule/tests/test_util_open_ended.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/common/lib/xmodule/xmodule/tests/test_util_open_ended.py b/common/lib/xmodule/xmodule/tests/test_util_open_ended.py index 42d6410ebd..8f3ad5b980 100644 --- a/common/lib/xmodule/xmodule/tests/test_util_open_ended.py +++ b/common/lib/xmodule/xmodule/tests/test_util_open_ended.py @@ -21,7 +21,8 @@ S3_INTERFACE = { class MockQueryDict(dict): """ - Mock a query dict so that it can be used in test classes + Mock a query dict so that it can be used in test classes. This will only work with the combinedopenended tests, + and does not mock the full query dict, only the behavior that is needed there (namely get_list). """ def getlist(self, key, default=None): try: From 6dfc490fc7feb7471ea81458beadc4ae14130c32 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Mon, 13 May 2013 12:49:01 -0400 Subject: [PATCH 20/21] Json updates --- lms/djangoapps/open_ended_grading/tests.py | 39 +++++----------------- 1 file changed, 8 insertions(+), 31 deletions(-) diff --git a/lms/djangoapps/open_ended_grading/tests.py b/lms/djangoapps/open_ended_grading/tests.py index 542c366cab..ffc02608d5 100644 --- a/lms/djangoapps/open_ended_grading/tests.py +++ b/lms/djangoapps/open_ended_grading/tests.py @@ -85,11 +85,7 @@ class TestStaffGradingService(LoginEnrollmentTestCase): r = self.check_for_post_code(200, url, data) - d = r.content - try: - d = json.loads(d) - except Exception: - pass + d = json.loads(r.content) self.assertTrue(d['success']) self.assertEquals(d['submission_id'], self.mock_service.cnt) @@ -118,11 +114,7 @@ class TestStaffGradingService(LoginEnrollmentTestCase): data.update({'skipped' : True}) r = self.check_for_post_code(200, url, data) - d = r.content - try: - d = json.loads(d) - except Exception: - pass + d = json.loads(r.content) self.assertTrue(d['success'], str(d)) self.assertEquals(d['submission_id'], self.mock_service.cnt) @@ -139,11 +131,8 @@ class TestStaffGradingService(LoginEnrollmentTestCase): data = {} r = self.check_for_post_code(200, url, data) - d = r.content - try: - d = json.loads(d) - except Exception: - pass + d = json.loads(r.content) + self.assertTrue(d['success'], str(d)) self.assertIsNotNone(d['problem_list']) @@ -194,10 +183,7 @@ class TestPeerGradingService(LoginEnrollmentTestCase): r = self.peer_module.get_next_submission(data) d = r - try: - d = json.loads(d) - except Exception: - pass + self.assertTrue(d['success']) self.assertIsNotNone(d['submission_id']) self.assertIsNotNone(d['prompt']) @@ -232,10 +218,7 @@ class TestPeerGradingService(LoginEnrollmentTestCase): r = self.peer_module.save_grade(qdict) d = r - try: - d = json.loads(d) - except Exception: - pass + self.assertTrue(d['success']) def test_save_grade_missing_keys(self): @@ -248,10 +231,7 @@ class TestPeerGradingService(LoginEnrollmentTestCase): data = {'location': self.location} r = self.peer_module.is_student_calibrated(data) d = r - try: - d = json.loads(d) - except Exception: - pass + self.assertTrue(d['success']) self.assertTrue('calibrated' in d) @@ -266,10 +246,7 @@ class TestPeerGradingService(LoginEnrollmentTestCase): r = self.peer_module.show_calibration_essay(data) d = r - try: - d = json.loads(r) - except Exception: - pass + self.assertTrue(d['success']) self.assertIsNotNone(d['submission_id']) self.assertIsNotNone(d['prompt']) From 451784dc92619eb7e0d416080098122d7caac3cd Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Mon, 13 May 2013 12:59:18 -0400 Subject: [PATCH 21/21] Fix pep8 space issues --- .../lib/xmodule/xmodule/tests/test_combined_open_ended.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py index 16406e24fc..917e90e575 100644 --- a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py +++ b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py @@ -537,7 +537,7 @@ class OpenEndedModuleXmlTest(unittest.TestCase, DummyModulestore): #Move to the next step in the problem module.handle_ajax("next_problem", {}) - self.assertEqual(module.current_task_number,0) + self.assertEqual(module.current_task_number, 0) html = module.get_html() self.assertTrue(isinstance(html, basestring)) @@ -577,7 +577,7 @@ class OpenEndedModuleXmlTest(unittest.TestCase, DummyModulestore): except GradingServiceError: #This error is okay. We don't have a grading service to connect to! pass - self.assertEqual(module.current_task_number,1) + self.assertEqual(module.current_task_number, 1) try: module.get_html() except GradingServiceError: @@ -608,7 +608,7 @@ class OpenEndedModuleXmlTest(unittest.TestCase, DummyModulestore): #Update the module with the fake queue reply module.handle_ajax("score_update", queue_reply) self.assertFalse(module.ready_to_reset) - self.assertEqual(module.current_task_number,1) + self.assertEqual(module.current_task_number, 1) #Get html and other data client will request html = module.get_html()