From d8817f33676183d5cfd351042f6fce3c191fcb74 Mon Sep 17 00:00:00 2001 From: dzhuang <dzhuang.scut@gmail.com> Date: Sun, 18 Feb 2018 17:59:47 +0800 Subject: [PATCH] Add more tests for code.py --- course/page/code.py | 20 +- tests/test_pages/test_code.py | 334 +++++++++++++++++++++++++++++++++- 2 files changed, 340 insertions(+), 14 deletions(-) diff --git a/course/page/code.py b/course/page/code.py index f7f0f39c..a19a4698 100644 --- a/course/page/code.py +++ b/course/page/code.py @@ -76,6 +76,7 @@ class PythonCodeForm(StyledForm): RUNPY_PORT = 9941 +DOCKER_TIMEOUT = 15 class InvalidPingResponse(RuntimeError): @@ -98,8 +99,6 @@ def request_python_run(run_req, run_timeout, image=None): def debug_print(s): pass - docker_timeout = 15 - if SPAWN_CONTAINERS_FOR_RUNPY: docker_url = getattr(settings, "RELATE_DOCKER_URL", "unix://var/run/docker.sock") @@ -108,7 +107,7 @@ def request_python_run(run_req, run_timeout, image=None): docker_cnx = docker.Client( base_url=docker_url, tls=docker_tls, - timeout=docker_timeout, + timeout=DOCKER_TIMEOUT, version="1.19") if image is None: @@ -160,7 +159,7 @@ def request_python_run(run_req, run_timeout, image=None): from traceback import format_exc def check_timeout(): - if time() - start_time < docker_timeout: + if time() - start_time < DOCKER_TIMEOUT: sleep(0.1) # and retry else: @@ -258,16 +257,15 @@ def is_nuisance_failure(result): if result["result"] != "uncaught_error": return False - if ("traceback" in result - and "BadStatusLine" in result["traceback"]): + if "traceback" in result: + if "BadStatusLine" in result["traceback"]: - # Occasionally, we fail to send a POST to the container, even after - # the inital ping GET succeeded, for (for now) mysterious reasons. - # Just try again. + # Occasionally, we fail to send a POST to the container, even after + # the inital ping GET succeeded, for (for now) mysterious reasons. + # Just try again. - return True + return True - if "traceback" in result: if "bind: address already in use" in result["traceback"]: # https://github.com/docker/docker/issues/8714 diff --git a/tests/test_pages/test_code.py b/tests/test_pages/test_code.py index 33e2f6b4..35894aa9 100644 --- a/tests/test_pages/test_code.py +++ b/tests/test_pages/test_code.py @@ -22,24 +22,31 @@ OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. """ +import six +import unittest from unittest import skipIf - from django.test import TestCase, override_settings +from docker.errors import APIError as DockerAPIError +from socket import error as socket_error, timeout as sock_timeout +import errno + +from course.page.code import ( + RUNPY_PORT, request_python_run_with_retries, InvalidPingResponse, + is_nuisance_failure) + from tests.base_test_mixins import ( SubprocessRunpyContainerMixin, SingleCoursePageTestMixin) from tests.test_sandbox import ( SingleCoursePageSandboxTestBaseMixin, PAGE_ERRORS ) - from tests.test_pages import QUIZ_FLOW_ID from tests.test_pages.utils import ( skip_real_docker_test, SKIP_REAL_DOCKER_REASON, REAL_RELATE_DOCKER_URL, REAL_RELATE_DOCKER_RUNPY_IMAGE, REAL_RELATE_DOCKER_TLS_CONFIG ) - from tests.utils import LocmemBackendTestsMixin, mock, mail from . import markdowns @@ -319,6 +326,13 @@ class CodeQuestionTest(SingleCoursePageSandboxTestBaseMixin, html="<p>some html</p>" ) + def test_html_non_text_bleached_in_feedback(self): + self.assert_runpy_result_and_response( + "user_error", + "(Non-string in 'HTML' output filtered out)", + html=b"not string" + ) + def test_traceback_in_feedback(self): self.assert_runpy_result_and_response( "user_error", @@ -341,4 +355,318 @@ class CodeQuestionTest(SingleCoursePageSandboxTestBaseMixin, ) +class RequestPythonRunWithRetriesTest(unittest.TestCase): + # Testing course.page.code.request_python_run_with_retries, + # adding tests for use cases that didn't cover in other tests + + @override_settings(RELATE_DOCKER_RUNPY_IMAGE="some_other_image") + def test_image_none(self): + # Testing if image is None, settings.RELATE_DOCKER_RUNPY_IMAGE is used + with mock.patch("docker.client.Client.create_container") as mock_create_ctn: + + # this will raise KeyError + mock_create_ctn.return_value = {} + + with self.assertRaises(KeyError): + request_python_run_with_retries( + run_req={}, run_timeout=0.1) + self.assertEqual(mock_create_ctn.call_count, 1) + self.assertIn("some_other_image", mock_create_ctn.call_args[0]) + + @override_settings(RELATE_DOCKER_RUNPY_IMAGE="some_other_image") + def test_image_not_none(self): + # Testing if image is None, settings.RELATE_DOCKER_RUNPY_IMAGE is used + with mock.patch("docker.client.Client.create_container") as mock_create_ctn: + + # this will raise KeyError + mock_create_ctn.return_value = {} + + my_image = "my_runpy_image" + + with self.assertRaises(KeyError): + request_python_run_with_retries( + run_req={}, image=my_image, run_timeout=0.1) + self.assertEqual(mock_create_ctn.call_count, 1) + self.assertIn(my_image, mock_create_ctn.call_args[0]) + + @skipIf(six.PY2, "PY2 doesn't support subTest") + def test_docker_container_ping_failure(self): + with ( + mock.patch("docker.client.Client.create_container")) as mock_create_ctn, ( # noqa + mock.patch("docker.client.Client.start")) as mock_ctn_start, ( + mock.patch("docker.client.Client.logs")) as mock_ctn_logs, ( + mock.patch("docker.client.Client.remove_container")) as mock_remove_ctn, ( # noqa + mock.patch("docker.client.Client.inspect_container")) as mock_inpect_ctn, ( # noqa + mock.patch("six.moves.http_client.HTTPConnection.request")) as mock_ctn_request: # noqa + + mock_create_ctn.return_value = {"Id": "someid"} + mock_ctn_start.side_effect = lambda x: None + mock_ctn_logs.side_effect = lambda x: None + mock_remove_ctn.return_value = None + fake_host_ip = "192.168.1.100" + fake_host_port = "69999" + + mock_inpect_ctn.return_value = { + "NetworkSettings": { + "Ports": {"%d/tcp" % RUNPY_PORT: ( + {"HostIp": fake_host_ip, "HostPort": fake_host_port}, + )} + }} + + with self.subTest(case="Docker ping timeout with BadStatusLine Error"): + from six.moves.http_client import BadStatusLine + fake_bad_statusline_msg = "my custom bad status" + mock_ctn_request.side_effect = BadStatusLine(fake_bad_statusline_msg) + + # force timeout + with mock.patch("course.page.code.DOCKER_TIMEOUT", 0.0001): + res = request_python_run_with_retries( + run_req={}, run_timeout=0.1, retry_count=0) + self.assertEqual(res["result"], "uncaught_error") + self.assertEqual(res['message'], + "Timeout waiting for container.") + self.assertEqual(res["exec_host"], fake_host_ip) + self.assertIn(fake_bad_statusline_msg, res["traceback"]) + + with self.subTest( + case="Docker ping timeout with InvalidPingResponse Error"): + invalid_ping_resp_msg = "my custom invalid ping response exception" + mock_ctn_request.side_effect = ( + InvalidPingResponse(invalid_ping_resp_msg)) + + # force timeout + with mock.patch("course.page.code.DOCKER_TIMEOUT", 0.0001): + res = request_python_run_with_retries( + run_req={}, run_timeout=0.1, retry_count=0) + self.assertEqual(res["result"], "uncaught_error") + self.assertEqual(res['message'], + "Timeout waiting for container.") + self.assertEqual(res["exec_host"], fake_host_ip) + self.assertIn(InvalidPingResponse.__name__, res["traceback"]) + self.assertIn(invalid_ping_resp_msg, res["traceback"]) + + with self.subTest( + case="Docker ping socket error with erron ECONNRESET"): + my_socket_error = socket_error() + my_socket_error.errno = errno.ECONNRESET + mock_ctn_request.side_effect = my_socket_error + + # force timeout + with mock.patch("course.page.code.DOCKER_TIMEOUT", 0.0001): + res = request_python_run_with_retries( + run_req={}, run_timeout=0.1, retry_count=0) + self.assertEqual(res["result"], "uncaught_error") + self.assertEqual(res['message'], + "Timeout waiting for container.") + self.assertEqual(res["exec_host"], fake_host_ip) + self.assertIn(type(my_socket_error).__name__, res["traceback"]) + + with self.subTest( + case="Docker ping socket error with erron ECONNREFUSED"): + my_socket_error = socket_error() + my_socket_error.errno = errno.ECONNREFUSED + mock_ctn_request.side_effect = my_socket_error + + # force timeout + with mock.patch("course.page.code.DOCKER_TIMEOUT", 0.0001): + res = request_python_run_with_retries( + run_req={}, run_timeout=0.1, retry_count=0) + self.assertEqual(res["result"], "uncaught_error") + self.assertEqual(res['message'], + "Timeout waiting for container.") + self.assertEqual(res["exec_host"], fake_host_ip) + self.assertIn(type(my_socket_error).__name__, res["traceback"]) + + with self.subTest( + case="Docker ping socket error with erron EAFNOSUPPORT"): + my_socket_error = socket_error() + + # This errno should raise error + my_socket_error.errno = errno.EAFNOSUPPORT + mock_ctn_request.side_effect = my_socket_error + + # force timeout + with mock.patch("course.page.code.DOCKER_TIMEOUT", 0.0001): + with self.assertRaises(socket_error) as e: + request_python_run_with_retries( + run_req={}, run_timeout=0.1, retry_count=0) + self.assertEqual(e.exception.errno, my_socket_error.errno) + + with self.assertRaises(socket_error) as e: + request_python_run_with_retries( + run_req={}, run_timeout=0.1, retry_count=0) + self.assertEqual(e.exception.errno, my_socket_error.errno) + + # This should be the last subTest, because this will the behavior of + # change mock_remove_ctn + with self.subTest( + case="Docker ping timeout with InvalidPingResponse and " + "remove container failed with APIError"): + invalid_ping_resp_msg = "my custom invalid ping response exception" + mock_ctn_request.side_effect = ( + InvalidPingResponse(invalid_ping_resp_msg)) + mock_remove_ctn.reset_mock() + from django.http import HttpResponse + fake_response_content = "this should not appear" + mock_remove_ctn.side_effect = DockerAPIError( + message="my custom docker api error", + response=HttpResponse(content=fake_response_content)) + + # force timeout + with mock.patch("course.page.code.DOCKER_TIMEOUT", 0.0001): + res = request_python_run_with_retries( + run_req={}, run_timeout=0.1, retry_count=0) + self.assertEqual(res["result"], "uncaught_error") + self.assertEqual(res['message'], + "Timeout waiting for container.") + self.assertEqual(res["exec_host"], fake_host_ip) + self.assertIn(InvalidPingResponse.__name__, res["traceback"]) + self.assertIn(invalid_ping_resp_msg, res["traceback"]) + + # No need to bother the students with this nonsense. + self.assertNotIn(DockerAPIError.__name__, res["traceback"]) + self.assertNotIn(fake_response_content, res["traceback"]) + + @skipIf(six.PY2, "PY2 doesn't support subTest") + def test_docker_container_ping_return_not_ok(self): + with ( + mock.patch("docker.client.Client.create_container")) as mock_create_ctn, ( # noqa + mock.patch("docker.client.Client.start")) as mock_ctn_start, ( + mock.patch("docker.client.Client.logs")) as mock_ctn_logs, ( + mock.patch("docker.client.Client.remove_container")) as mock_remove_ctn, ( # noqa + mock.patch("docker.client.Client.inspect_container")) as mock_inpect_ctn, ( # noqa + mock.patch("six.moves.http_client.HTTPConnection.request")) as mock_ctn_request, ( # noqa + mock.patch("six.moves.http_client.HTTPConnection.getresponse")) as mock_ctn_get_response: # noqa + + mock_create_ctn.return_value = {"Id": "someid"} + mock_ctn_start.side_effect = lambda x: None + mock_ctn_logs.side_effect = lambda x: None + mock_remove_ctn.return_value = None + fake_host_ip = "192.168.1.100" + fake_host_port = "69999" + + mock_inpect_ctn.return_value = { + "NetworkSettings": { + "Ports": {"%d/tcp" % RUNPY_PORT: ( + {"HostIp": fake_host_ip, "HostPort": fake_host_port}, + )} + }} + + # force timeout + with mock.patch("course.page.code.DOCKER_TIMEOUT", 0.0001): + with self.subTest( + case="Docker ping response not OK"): + mock_ctn_request.side_effect = lambda x, y: None + mock_ctn_get_response.return_value = six.BytesIO(b"NOT OK") + + res = request_python_run_with_retries( + run_req={}, run_timeout=0.1, retry_count=0) + self.assertEqual(res["result"], "uncaught_error") + self.assertEqual(res['message'], + "Timeout waiting for container.") + self.assertEqual(res["exec_host"], fake_host_ip) + self.assertIn(InvalidPingResponse.__name__, res["traceback"]) + + @skipIf(six.PY2, "PY2 doesn't support subTest") + def test_docker_container_runpy_timeout(self): + with ( + mock.patch("docker.client.Client.create_container")) as mock_create_ctn, ( # noqa + mock.patch("docker.client.Client.start")) as mock_ctn_start, ( + mock.patch("docker.client.Client.logs")) as mock_ctn_logs, ( + mock.patch("docker.client.Client.remove_container")) as mock_remove_ctn, ( # noqa + mock.patch("docker.client.Client.inspect_container")) as mock_inpect_ctn, ( # noqa + mock.patch("six.moves.http_client.HTTPConnection.request")) as mock_ctn_request, ( # noqa + mock.patch("six.moves.http_client.HTTPConnection.getresponse")) as mock_ctn_get_response: # noqa + + mock_create_ctn.return_value = {"Id": "someid"} + mock_ctn_start.side_effect = lambda x: None + mock_ctn_logs.side_effect = lambda x: None + mock_remove_ctn.return_value = None + fake_host_ip = "192.168.1.100" + fake_host_port = "69999" + + mock_inpect_ctn.return_value = { + "NetworkSettings": { + "Ports": {"%d/tcp" % RUNPY_PORT: ( + {"HostIp": fake_host_ip, "HostPort": fake_host_port}, + )} + }} + + with self.subTest( + case="Docker ping passed by runpy timed out"): + + # first request is ping, second request raise socket.timeout + mock_ctn_request.side_effect = [None, sock_timeout] + mock_ctn_get_response.return_value = six.BytesIO(b"OK") + + res = request_python_run_with_retries( + run_req={}, run_timeout=0.1, retry_count=0) + self.assertEqual(res["result"], "timeout") + self.assertEqual(res["exec_host"], fake_host_ip) + + @skipIf(six.PY2, "PY2 doesn't support subTest") + def test_docker_container_runpy_retries_count(self): + with ( + mock.patch("course.page.code.request_python_run")) as mock_req_run, ( # noqa + mock.patch("course.page.code.is_nuisance_failure")) as mock_is_nuisance_failure: # noqa + expected_result = "this is my custom result" + mock_req_run.return_value = {"result": expected_result} + with self.subTest(actual_retry_count=4): + mock_is_nuisance_failure.side_effect = [True, True, True, False] + res = request_python_run_with_retries( + run_req={}, run_timeout=0.1, retry_count=5) + self.assertEqual(res["result"], expected_result) + self.assertEqual(mock_req_run.call_count, 4) + self.assertEqual(mock_is_nuisance_failure.call_count, 4) + + mock_req_run.reset_mock() + mock_is_nuisance_failure.reset_mock() + with self.subTest(actual_retry_count=2): + mock_is_nuisance_failure.side_effect = [True, True, True, False] + res = request_python_run_with_retries( + run_req={}, run_timeout=0.1, retry_count=1) + self.assertEqual(res["result"], expected_result) + self.assertEqual(mock_req_run.call_count, 2) + self.assertEqual(mock_is_nuisance_failure.call_count, 1) + + +class IsNuisanceFailureTest(unittest.TestCase): + # Testing is_nuisance_failure + + def test_not_uncaught_error(self): + result = {"result": "not_uncaught_error"} + self.assertFalse(is_nuisance_failure(result)) + + def test_no_traceback(self): + result = {"result": "uncaught_error"} + self.assertFalse(is_nuisance_failure(result)) + + def test_traceback_unkown(self): + result = {"result": "uncaught_error", + "traceback": "unknow traceback"} + self.assertFalse(is_nuisance_failure(result)) + + def test_traceback_has_badstatusline(self): + result = {"result": "uncaught_error", + "traceback": "BadStatusLine: \nfoo"} + self.assertTrue(is_nuisance_failure(result)) + + def test_traceback_address_already_in_use(self): + result = {"result": "uncaught_error", + "traceback": "\nbind: address already in use \nfoo"} + self.assertTrue(is_nuisance_failure(result)) + + def test_traceback_new_connection_error(self): + result = {"result": "uncaught_error", + "traceback": + "\nrequests.packages.urllib3.exceptions." + "NewConnectionError: \nfoo"} + self.assertTrue(is_nuisance_failure(result)) + + def test_traceback_remote_disconnected(self): + result = {"result": "uncaught_error", + "traceback": + "\nhttp.client.RemoteDisconnected: \nfoo"} + self.assertTrue(is_nuisance_failure(result)) + # vim: fdm=marker -- GitLab