%s
" + % get_auto_feedback(correctness)) + except Exception as e: + correctness = None + response_dict["result"] = "setup_error" + response_dict["message"] = ( + "%s: %s" % (type(e).__name__, str(e)) + ) + # {{{ send email if the grading code broke if response_dict["result"] in [ @@ -747,13 +762,6 @@ class PythonCodeQuestion(PageBaseWithTitle, PageBaseWithValue): response = dict_to_struct(response_dict) bulk_feedback_bits = [] - if hasattr(response, "points"): - correctness = response.points - feedback_bits.append( - "%s
" - % get_auto_feedback(correctness)) - else: - correctness = None if response.result == "success": pass diff --git a/course/page/code_runpy_backend.py b/course/page/code_runpy_backend.py index 4fc7bed139760fcd66a6ded0034400d9fdc0d30f..a06b45fc4be49a50db600dbc5accae5b82ae2e17 100644 --- a/course/page/code_runpy_backend.py +++ b/course/page/code_runpy_backend.py @@ -24,7 +24,6 @@ OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. """ -import math import sys import traceback @@ -301,13 +300,6 @@ def run_code(result, run_req): package_exception(result, "test_error") return - if feedback.points is not None and math.isclose(feedback.points, 1): - feedback.points = 1 - - if not (feedback.points is None or 0 <= feedback.points <= 1): - raise ValueError("grade point value is invalid: %s" - % feedback.points) - result["points"] = feedback.points result["feedback"] = feedback.feedback_items diff --git a/tests/test_pages/markdowns.py b/tests/test_pages/markdowns.py index b4d857480e6c64219dff998db5d1c82f4e5720cc..feb78710bbc3fca88665020e65d70c30426df867 100644 --- a/tests/test_pages/markdowns.py +++ b/tests/test_pages/markdowns.py @@ -171,7 +171,43 @@ test_code: | """ -MAX_AUTO_FEEDBACK_POINTS_CODE_MARKDWON_PATTERN = """ +FEEDBACK_POINTS_CODE_MARKDWON_PATTERN = """ +type: PythonCodeQuestion +id: addition +value: 1 +timeout: 10 +prompt: | + + # Adding two numbers in Python + +setup_code: | + import random + + a = random.uniform(-10, 10) + b = random.uniform(-10, 10) + +names_for_user: [a, b] + +names_from_user: [c] + +test_code: | + if not isinstance(c, float): + feedback.finish(0, "Your computed c is not a float.") + + correct_c = a + b + rel_err = abs(correct_c-c)/abs(correct_c) + + if rel_err < 1e-7: + feedback.finish(%(full_points)s, "Your computed c was correct.") + else: + feedback.finish(%(min_points)s, "Your computed c was incorrect.") + +correct_code: | + + c = a + b +""" # noqa + +FEEDBACK_POINTS_CODE_MARKDWON_PATTERN = """ type: PythonCodeQuestion id: addition value: 1 diff --git a/tests/test_pages/test_code.py b/tests/test_pages/test_code.py index 35894aa93ace42ae8a7e41b1d529da32f384e5e8..0d62aca52826f58170b4e45b365b77cf4964cccf 100644 --- a/tests/test_pages/test_code.py +++ b/tests/test_pages/test_code.py @@ -35,6 +35,8 @@ from course.page.code import ( RUNPY_PORT, request_python_run_with_retries, InvalidPingResponse, is_nuisance_failure) +from course.constants import MAX_EXTRA_CREDIT_FACTOR + from tests.base_test_mixins import ( SubprocessRunpyContainerMixin, SingleCoursePageTestMixin) @@ -75,6 +77,12 @@ GRADE_CODE_FAILING_MSG = ( RUNPY_WITH_RETRIES_PATH = "course.page.code.request_python_run_with_retries" +AUTO_FEEDBACK_POINTS_OUT_OF_RANGE_ERROR_MSG_PATTERN = ( + "'correctness' is invalid: expecting " + "a value within [0, %s] or None, " + "got %s." +) + class RealDockerTestMixin(object): @classmethod @@ -354,6 +362,207 @@ class CodeQuestionTest(SingleCoursePageSandboxTestBaseMixin, stderr="some stderr" ) + # {{{ https://github.com/inducer/relate/pull/448 + def test_feedback_points_close_to_1(self): + markdown = (markdowns.FEEDBACK_POINTS_CODE_MARKDWON_PATTERN + % { + "full_points": 1.000000000002, + "min_points": 0 + }) + resp = self.get_page_sandbox_preview_response(markdown) + self.assertEqual(resp.status_code, 200) + self.assertSandboxHaveValidPage(resp) + + resp = self.get_page_sandbox_submit_answer_response( + markdown, + answer_data={"answer": ['c = b + a\r']}) + self.assertEqual(resp.status_code, 200) + self.assertResponseContextAnswerFeedbackCorrectnessEquals(resp, 1) + + def test_feedback_code_exceed_1(self): + feedback_points = 1.1 + markdown = (markdowns.FEEDBACK_POINTS_CODE_MARKDWON_PATTERN + % { + "full_points": feedback_points, + "min_points": 0 + }) + resp = self.get_page_sandbox_preview_response(markdown) + self.assertEqual(resp.status_code, 200) + self.assertSandboxHaveValidPage(resp) + + resp = self.get_page_sandbox_submit_answer_response( + markdown, + answer_data={"answer": ['c = b + a\r']}) + self.assertEqual(resp.status_code, 200) + self.assertResponseContextAnswerFeedbackCorrectnessEquals(resp, 1.1) + + expected_feedback = "Your answer is correct and earned bonus points." + + self.assertResponseContextAnswerFeedbackContainsFeedback( + resp, expected_feedback) + + def test_feedback_code_positive_close_to_0(self): + # https://github.com/inducer/relate/pull/448#issuecomment-363655132 + markdown = (markdowns.FEEDBACK_POINTS_CODE_MARKDWON_PATTERN + % { + "full_points": 1, + "min_points": 0.00000000001 + }) + resp = self.get_page_sandbox_preview_response(markdown) + self.assertEqual(resp.status_code, 200) + self.assertSandboxHaveValidPage(resp) + + # Post a wrong answer + resp = self.get_page_sandbox_submit_answer_response( + markdown, + answer_data={"answer": ['c = b - a\r']}) + self.assertEqual(resp.status_code, 200) + self.assertResponseContextAnswerFeedbackCorrectnessEquals(resp, 0) + + def test_feedback_code_negative_close_to_0(self): + # https://github.com/inducer/relate/pull/448#issuecomment-363655132 + markdown = (markdowns.FEEDBACK_POINTS_CODE_MARKDWON_PATTERN + % { + "full_points": 1, + "min_points": -0.00000000001 + }) + resp = self.get_page_sandbox_preview_response(markdown) + self.assertEqual(resp.status_code, 200) + self.assertSandboxHaveValidPage(resp) + + # Post a wrong answer + resp = self.get_page_sandbox_submit_answer_response( + markdown, + answer_data={"answer": ['c = b - a\r']}) + self.assertEqual(resp.status_code, 200) + self.assertResponseContextAnswerFeedbackCorrectnessEquals(resp, 0) + + def test_feedback_code_error_close_below_max_auto_feedback_points(self): + feedback_points = MAX_EXTRA_CREDIT_FACTOR - 1e-6 + markdown = (markdowns.FEEDBACK_POINTS_CODE_MARKDWON_PATTERN + % { + "full_points": feedback_points, + "min_points": 0 + }) + resp = self.get_page_sandbox_preview_response(markdown) + self.assertEqual(resp.status_code, 200) + self.assertSandboxHaveValidPage(resp) + + resp = self.get_page_sandbox_submit_answer_response( + markdown, + answer_data={"answer": ['c = b + a\r']}) + self.assertEqual(resp.status_code, 200) + self.assertResponseContextAnswerFeedbackCorrectnessEquals( + resp, MAX_EXTRA_CREDIT_FACTOR) + + def test_feedback_code_error_close_above_max_auto_feedback_points(self): + feedback_points = MAX_EXTRA_CREDIT_FACTOR + 1e-6 + markdown = (markdowns.FEEDBACK_POINTS_CODE_MARKDWON_PATTERN + % { + "full_points": feedback_points, + "min_points": 0 + }) + resp = self.get_page_sandbox_preview_response(markdown) + self.assertEqual(resp.status_code, 200) + self.assertSandboxHaveValidPage(resp) + + resp = self.get_page_sandbox_submit_answer_response( + markdown, + answer_data={"answer": ['c = b + a\r']}) + self.assertEqual(resp.status_code, 200) + self.assertResponseContextAnswerFeedbackCorrectnessEquals( + resp, MAX_EXTRA_CREDIT_FACTOR) + + def test_feedback_code_error_negative_feedback_points(self): + invalid_feedback_points = -0.1 + markdown = (markdowns.FEEDBACK_POINTS_CODE_MARKDWON_PATTERN + % { + "full_points": 1, + "min_points": invalid_feedback_points + }) + resp = self.get_page_sandbox_preview_response(markdown) + self.assertEqual(resp.status_code, 200) + self.assertSandboxHaveValidPage(resp) + + # Post a wrong answer + resp = self.get_page_sandbox_submit_answer_response( + markdown, + answer_data={"answer": ['c = b - a\r']}) + self.assertEqual(resp.status_code, 200) + self.assertResponseContextAnswerFeedbackCorrectnessEquals(resp, None) + + error_msg = (AUTO_FEEDBACK_POINTS_OUT_OF_RANGE_ERROR_MSG_PATTERN + % (MAX_EXTRA_CREDIT_FACTOR, invalid_feedback_points)) + + self.assertResponseContextAnswerFeedbackNotContainsFeedback( + resp, error_msg) + + self.assertResponseContextAnswerFeedbackContainsFeedback( + resp, GRADE_CODE_FAILING_MSG) + + def test_feedback_code_error_exceed_max_extra_credit_factor(self): + invalid_feedback_points = 10.1 + markdown = (markdowns.FEEDBACK_POINTS_CODE_MARKDWON_PATTERN + % { + "full_points": invalid_feedback_points, + "min_points": 0 + }) + resp = self.get_page_sandbox_preview_response(markdown) + self.assertEqual(resp.status_code, 200) + self.assertSandboxHaveValidPage(resp) + + resp = self.get_page_sandbox_submit_answer_response( + markdown, + answer_data={"answer": ['c = b + a\r']}) + self.assertEqual(resp.status_code, 200) + self.assertResponseContextAnswerFeedbackCorrectnessEquals(resp, None) + error_msg = (AUTO_FEEDBACK_POINTS_OUT_OF_RANGE_ERROR_MSG_PATTERN + % (MAX_EXTRA_CREDIT_FACTOR, invalid_feedback_points)) + + self.assertResponseContextAnswerFeedbackNotContainsFeedback( + resp, error_msg) + + self.assertResponseContextAnswerFeedbackContainsFeedback( + resp, GRADE_CODE_FAILING_MSG) + + def test_feedback_code_error_exceed_max_extra_credit_factor_email(self): + invalid_feedback_points = 10.1 + markdown = (markdowns.FEEDBACK_POINTS_CODE_MARKDWON_PATTERN + % { + "full_points": invalid_feedback_points, + "min_points": 0 + }) + resp = self.get_page_sandbox_preview_response(markdown) + self.assertEqual(resp.status_code, 200) + self.assertSandboxHaveValidPage(resp) + + with mock.patch("course.page.PageContext") as mock_page_context: + mock_page_context.return_value.in_sandbox = False + + # This remove the warning caused by mocked commit_sha value + # "CacheKeyWarning: Cache key contains characters that + # will cause errors ..." + mock_page_context.return_value.commit_sha = b"1234" + + resp = self.get_page_sandbox_submit_answer_response( + markdown, + answer_data={"answer": ['c = b + a\r']}) + self.assertEqual(resp.status_code, 200) + self.assertResponseContextAnswerFeedbackCorrectnessEquals(resp, None) + error_msg = (AUTO_FEEDBACK_POINTS_OUT_OF_RANGE_ERROR_MSG_PATTERN + % (MAX_EXTRA_CREDIT_FACTOR, invalid_feedback_points)) + + self.assertResponseContextAnswerFeedbackNotContainsFeedback( + resp, error_msg) + + self.assertResponseContextAnswerFeedbackContainsFeedback( + resp, GRADE_CODE_FAILING_MSG) + self.assertEqual(len(mail.outbox), 1) + + self.assertIn(error_msg, mail.outbox[0].body) + + # }}} + class RequestPythonRunWithRetriesTest(unittest.TestCase): # Testing course.page.code.request_python_run_with_retries, diff --git a/tests/test_pages/test_generic.py b/tests/test_pages/test_generic.py index f3dde3ba18b0595cdc55912604ac89fe4ed1bff3..f62d9e087257f4dca5dd32422c7dff8417c9b110 100644 --- a/tests/test_pages/test_generic.py +++ b/tests/test_pages/test_generic.py @@ -32,12 +32,14 @@ from django.core import mail from course.models import FlowSession from course.constants import MAX_EXTRA_CREDIT_FACTOR -from course.page.base import AnswerFeedback +from course.page.base import ( + AnswerFeedback, get_auto_feedback, + validate_point_count, InvalidFeedbackPointsError) from tests.base_test_mixins import ( SingleCoursePageTestMixin, FallBackStorageMessageTestMixin, SubprocessRunpyContainerMixin) -from tests.utils import LocmemBackendTestsMixin +from tests.utils import LocmemBackendTestsMixin, mock QUIZ_FLOW_ID = "quiz-test" @@ -758,16 +760,77 @@ class SingleCourseQuizPageCodeQuestionTest( self.assertSessionScoreEqual(4) +class ValidatePointCountTest(unittest.TestCase): + """ + test course.page.base.validate_point_count + """ + def test_none(self): + self.assertIsNone(validate_point_count(None)) + + def test_negative_error(self): + with self.assertRaises(InvalidFeedbackPointsError): + validate_point_count(-0.0001) + + def test_above_max_extra_credit_factor_error(self): + with self.assertRaises(InvalidFeedbackPointsError): + validate_point_count(MAX_EXTRA_CREDIT_FACTOR + 0.0001) + + def test_close_0_negative(self): + self.assertEqual(validate_point_count(-0.000009), 0) + + def test_close_0_positive(self): + self.assertEqual(validate_point_count(0.000009), 0) + + def test_above_close_max_extra_credit_factor(self): + self.assertEqual(validate_point_count( + MAX_EXTRA_CREDIT_FACTOR + 0.000009), MAX_EXTRA_CREDIT_FACTOR) + + def test_below_close_max_extra_credit_factor(self): + self.assertEqual(validate_point_count( + MAX_EXTRA_CREDIT_FACTOR - 0.000009), MAX_EXTRA_CREDIT_FACTOR) + + def test_int(self): + self.assertEqual(validate_point_count(5), 5) + + def test_gt_close_int(self): + self.assertEqual(validate_point_count(5.000009), 5) + + def test_lt_close_int(self): + self.assertEqual(validate_point_count(4.9999999), 5) + + def test_quarter(self): + self.assertEqual(validate_point_count(1.25), 1.25) + + def test_gt_quarter(self): + self.assertEqual(validate_point_count(0.2500009), 0.25) + + def test_lt_quarter(self): + self.assertEqual(validate_point_count(9.7499999), 9.75) + + def test_half(self): + self.assertEqual(validate_point_count(1.5), 1.5) + + def test_gt_half(self): + self.assertEqual(validate_point_count(3.500001), 3.5) + + def test_lt_half(self): + self.assertEqual(validate_point_count(0.4999999), 0.5) + + class AnswerFeedBackTest(unittest.TestCase): + """ + test course.page.base.AnswerFeedBack + """ + # TODO: more tests def test_correctness_negative(self): correctness = -0.1 - with self.assertRaises(ValueError): + with self.assertRaises(InvalidFeedbackPointsError): AnswerFeedback(correctness) def test_correctness_exceed_max_extra_credit_factor(self): correctness = MAX_EXTRA_CREDIT_FACTOR + 0.1 - with self.assertRaises(ValueError): + with self.assertRaises(InvalidFeedbackPointsError): AnswerFeedback(correctness) def test_correctness_can_be_none(self): @@ -788,4 +851,112 @@ class AnswerFeedBackTest(unittest.TestCase): af = AnswerFeedback.from_json(None, None) self.assertIsNone(af) + def test_validate_point_count_called(self): + import random + with mock.patch("course.page.base.validate_point_count")\ + as mock_validate_point_count,\ + mock.patch("course.page.base.get_auto_feedback")\ + as mock_get_auto_feedback: + mock_validate_point_count.side_effect = lambda x: x + + mock_get_auto_feedback.side_effect = lambda x: x + for i in range(10): + correctness = random.uniform(0, 15) + feedback = "some feedback" + AnswerFeedback(correctness, feedback) + mock_validate_point_count.assert_called_once_with(correctness) + + # because feedback is not None + self.assertEqual(mock_get_auto_feedback.call_count, 0) + mock_validate_point_count.reset_mock() + + for i in range(10): + correctness = random.uniform(0, 15) + AnswerFeedback(correctness) + + # because get_auto_feedback is mocked, the call_count of + # mock_validate_point_count is only once + mock_validate_point_count.assert_called_once_with(correctness) + mock_validate_point_count.reset_mock() + + # because feedback is None + self.assertEqual(mock_get_auto_feedback.call_count, 1) + mock_get_auto_feedback.reset_mock() + + AnswerFeedback(correctness=None) + mock_validate_point_count.assert_called_once_with(None) + + +class GetAutoFeedbackTest(unittest.TestCase): + """ + test course.page.base.get_auto_feedback + """ + def test_none(self): + self.assertIn("No information", get_auto_feedback(None)) + + def test_not_correct(self): + self.assertIn("not correct", get_auto_feedback(0.000001)) + self.assertIn("not correct", get_auto_feedback(-0.000001)) + + def test_correct(self): + result = get_auto_feedback(0.999999) + self.assertIn("is correct", result) + self.assertNotIn("bonus", result) + + result = get_auto_feedback(1) + self.assertIn("is correct", result) + self.assertNotIn("bonus", result) + + result = get_auto_feedback(1.000001) + self.assertIn("is correct", result) + self.assertNotIn("bonus", result) + + def test_correct_with_bonus(self): + result = get_auto_feedback(1.01) + self.assertIn("is correct", result) + self.assertIn("bonus", result) + + result = get_auto_feedback(2) + self.assertIn("is correct", result) + self.assertIn("bonus", result) + + result = get_auto_feedback(9.99999) + self.assertIn("is correct", result) + self.assertIn("bonus", result) + + def test_with_mostly_correct(self): + self.assertIn("mostly correct", get_auto_feedback(0.51)) + self.assertIn("mostly correct", get_auto_feedback(0.999)) + + def test_with_somewhat_correct(self): + self.assertIn("somewhat correct", get_auto_feedback(0.5)) + self.assertIn("somewhat correct", get_auto_feedback(0.5000001)) + self.assertIn("somewhat correct", get_auto_feedback(0.001)) + self.assertIn("somewhat correct", get_auto_feedback(0.2)) + + def test_correctness_negative(self): + correctness = -0.1 + with self.assertRaises(InvalidFeedbackPointsError): + get_auto_feedback(correctness) + + def test_correctness_exceed_max_extra_credit_factor(self): + correctness = MAX_EXTRA_CREDIT_FACTOR + 0.1 + with self.assertRaises(InvalidFeedbackPointsError): + get_auto_feedback(correctness) + + def test_validate_point_count_called(self): + import random + with mock.patch("course.page.base.validate_point_count") \ + as mock_validate_point_count: + mock_validate_point_count.side_effect = lambda x: x + for i in range(10): + correctness = random.uniform(0, 15) + get_auto_feedback(correctness) + mock_validate_point_count.assert_called_once_with(correctness) + mock_validate_point_count.reset_mock() + + get_auto_feedback(correctness=None) + mock_validate_point_count.assert_called_once_with(None) + + # vim: fdm=marker