From b56e10d0fd85b81b4463eec3f8d3cc278b6677e4 Mon Sep 17 00:00:00 2001 From: Andreas Kloeckner Date: Mon, 21 Aug 2017 18:36:23 -0500 Subject: [PATCH 01/45] Upgrade Dulwich dep --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 036f50f2..073ab6ab 100644 --- a/requirements.txt +++ b/requirements.txt @@ -32,7 +32,7 @@ pytz pyyaml # dulwich -dulwich>=0.17.3 +dulwich>=0.18.2 ecdsa paramiko -- GitLab From 3bc9dc3269e723f7068abd438c0bce09464a9c22 Mon Sep 17 00:00:00 2001 From: Andreas Kloeckner Date: Mon, 21 Aug 2017 18:36:45 -0500 Subject: [PATCH 02/45] Remove unnecessary check for Dulwich-to-Dulwich-passed 'command' type (Fixes #347 on Github) --- course/versioning.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/course/versioning.py b/course/versioning.py index 501f2ba5..68eac1ba 100644 --- a/course/versioning.py +++ b/course/versioning.py @@ -111,9 +111,6 @@ class DulwichParamikoSSHVendor(object): def run_command(self, host, command, username=None, port=None, progress_stderr=None): - if not isinstance(command, bytes): - raise TypeError(command) - if port is None: port = 22 -- GitLab From dfb3c75239ab5ad995c572cf922774ec874e2279 Mon Sep 17 00:00:00 2001 From: Andreas Kloeckner Date: Mon, 21 Aug 2017 18:42:24 -0500 Subject: [PATCH 03/45] Catch inst id being None in user profile data cleaning (Fixes #348 on github) --- course/auth.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/course/auth.py b/course/auth.py index 1dd295fc..fadb73c3 100644 --- a/course/auth.py +++ b/course/auth.py @@ -912,7 +912,11 @@ class UserForm(StyledModelForm): Submit("submit_user", _("Update"))) def clean_institutional_id(self): - inst_id = self.cleaned_data['institutional_id'].strip() + inst_id = self.cleaned_data['institutional_id'] + + if inst_id is not None: + inst_id = inst_id.strip() + if self.is_inst_id_locked: # Disabled fields are not part of form submit--so simply # assume old value. At the same time, prevent smuggled-in -- GitLab From dda500bd7bb1388862a5b56e854a64f4f6ccf2ae Mon Sep 17 00:00:00 2001 From: Andreas Kloeckner Date: Thu, 24 Aug 2017 10:56:31 -0500 Subject: [PATCH 04/45] Fix some invalid escape sequences --- course/constants.py | 2 +- course/content.py | 2 +- course/page/inline.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/course/constants.py b/course/constants.py index aea63ae0..7c50b9f1 100644 --- a/course/constants.py +++ b/course/constants.py @@ -34,7 +34,7 @@ COURSE_ID_REGEX = "(?P[-a-zA-Z0-9]+)" FLOW_ID_REGEX = "(?P[-_a-zA-Z0-9]+)" GRADING_OPP_ID_REGEX = "(?P[-_a-zA-Z0-9]+)" # FIXME : Support page hierarchy. Add '/' here, fix validation code. -STATICPAGE_PATH_REGEX = "(?P[-\w]+)" +STATICPAGE_PATH_REGEX = r"(?P[-\w]+)" class user_status: # noqa diff --git a/course/content.py b/course/content.py index 7c215db0..17dcf70f 100644 --- a/course/content.py +++ b/course/content.py @@ -384,7 +384,7 @@ YAML_BLOCK_START_SCALAR_RE = re.compile( r"(:\s*[|>])" "(J?)" "((?:[0-9][-+]?|[-+][0-9]?)?)" - "(?:\s*\#.*)?" + r"(?:\s*\#.*)?" "$") GROUP_COMMENT_START = re.compile(r"^\s*#\s*\{\{\{") diff --git a/course/page/inline.py b/course/page/inline.py index 32ecb334..5c883f65 100644 --- a/course/page/inline.py +++ b/course/page/inline.py @@ -206,7 +206,7 @@ EM_LEN_DICT = { "%": ""} ALLOWED_LENGTH_UNIT = EM_LEN_DICT.keys() -WIDTH_STR_RE = re.compile("^(\d*\.\d+|\d+)\s*(.*)$") +WIDTH_STR_RE = re.compile(r"^(\d*\.\d+|\d+)\s*(.*)$") class ShortAnswer(AnswerBase): -- GitLab From 5bb6993756d99dcfd7efad3f4b6854e12cf28b6e Mon Sep 17 00:00:00 2001 From: Andreas Kloeckner Date: Sat, 26 Aug 2017 11:44:44 -0500 Subject: [PATCH 05/45] Validate markup on static pages --- course/content.py | 2 +- course/validation.py | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/course/content.py b/course/content.py index 7c215db0..bbe59abb 100644 --- a/course/content.py +++ b/course/content.py @@ -902,7 +902,7 @@ def markup_to_html( cache_key = None else: import hashlib - cache_key = ("markup:v6:%s:%d:%s:%s" + cache_key = ("markup:v7:%s:%d:%s:%s" % (CACHE_KEY_ROOT, course.id, str(commit_sha), hashlib.md5(text.encode("utf-8")).hexdigest())) diff --git a/course/validation.py b/course/validation.py index ee1540ee..e8cf0122 100644 --- a/course/validation.py +++ b/course/validation.py @@ -399,6 +399,8 @@ def validate_page_chunk(vctx, location, chunk): "%s, rule %d" % (location, i+1), rule) + validate_markup(vctx, location, chunk.content) + def validate_staticpage_desc(vctx, location, page_desc): validate_struct( @@ -433,6 +435,8 @@ def validate_staticpage_desc(vctx, location, page_desc): assert not hasattr(page_desc, "content") assert hasattr(page_desc, "chunks") + validate_markup(vctx, location, page_desc.content) + for i, chunk in enumerate(page_desc.chunks): validate_page_chunk(vctx, "%s, chunk %d ('%s')" -- GitLab From 75da43ebeb0a41b64d41a707825199c2d2c8249c Mon Sep 17 00:00:00 2001 From: Andreas Kloeckner Date: Sat, 26 Aug 2017 12:28:43 -0500 Subject: [PATCH 06/45] Fix static page validation --- course/validation.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/course/validation.py b/course/validation.py index e8cf0122..33ef2d4b 100644 --- a/course/validation.py +++ b/course/validation.py @@ -435,8 +435,6 @@ def validate_staticpage_desc(vctx, location, page_desc): assert not hasattr(page_desc, "content") assert hasattr(page_desc, "chunks") - validate_markup(vctx, location, page_desc.content) - for i, chunk in enumerate(page_desc.chunks): validate_page_chunk(vctx, "%s, chunk %d ('%s')" -- GitLab From 73410b312282ccf0c6d741209f169e36e47a7556 Mon Sep 17 00:00:00 2001 From: Andreas Kloeckner Date: Mon, 28 Aug 2017 14:21:39 -0500 Subject: [PATCH 07/45] Filter NULL values from access_kinds to prevent crashes in .attributes.yml checking --- course/validation.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/course/validation.py b/course/validation.py index 33ef2d4b..06c4f644 100644 --- a/course/validation.py +++ b/course/validation.py @@ -1396,6 +1396,9 @@ def validate_course_content(repo, course_file, events_file, permission=pperm.access_files_for, ) .values_list("argument", flat=True)) + + access_kinds = frozenset(k for k in access_kinds if k is not None) + else: access_kinds = ["public", "in_exam", "student", "ta", "unenrolled", "instructor"] -- GitLab From be0ac9b3ce99ab8f077016190fdf7c5e95cd895b Mon Sep 17 00:00:00 2001 From: Andreas Kloeckner Date: Mon, 28 Aug 2017 14:40:47 -0500 Subject: [PATCH 08/45] Make friendly error message when double-adding a participation --- course/enrollment.py | 87 ++++++++++++++++++++++++-------------------- 1 file changed, 47 insertions(+), 40 deletions(-) diff --git a/course/enrollment.py b/course/enrollment.py index 6e2c9146..abad3046 100644 --- a/course/enrollment.py +++ b/course/enrollment.py @@ -994,46 +994,53 @@ def edit_participation(pctx, participation_id): add_new, pctx, request.POST, instance=participation) reset_form = False - if form.is_valid(): - if "submit" in request.POST: - form.save() - - messages.add_message(request, messages.SUCCESS, - _("Changes saved.")) - - elif "approve" in request.POST: - send_enrollment_decision(participation, True, pctx.request) - - # FIXME: Double-saving - participation = form.save() - participation.status = participation_status.active - participation.save() - reset_form = True - - messages.add_message(request, messages.SUCCESS, - _("Successfully enrolled.")) - - elif "deny" in request.POST: - send_enrollment_decision(participation, False, pctx.request) - - # FIXME: Double-saving - participation = form.save() - participation.status = participation_status.denied - participation.save() - reset_form = True - - messages.add_message(request, messages.SUCCESS, - _("Successfully denied.")) - - elif "drop" in request.POST: - # FIXME: Double-saving - participation = form.save() - participation.status = participation_status.dropped - participation.save() - reset_form = True - - messages.add_message(request, messages.SUCCESS, - _("Successfully dropped.")) + try: + if form.is_valid(): + if "submit" in request.POST: + form.save() + + messages.add_message(request, messages.SUCCESS, + _("Changes saved.")) + + elif "approve" in request.POST: + send_enrollment_decision(participation, True, pctx.request) + + # FIXME: Double-saving + participation = form.save() + participation.status = participation_status.active + participation.save() + reset_form = True + + messages.add_message(request, messages.SUCCESS, + _("Successfully enrolled.")) + + elif "deny" in request.POST: + send_enrollment_decision(participation, False, pctx.request) + + # FIXME: Double-saving + participation = form.save() + participation.status = participation_status.denied + participation.save() + reset_form = True + + messages.add_message(request, messages.SUCCESS, + _("Successfully denied.")) + + elif "drop" in request.POST: + # FIXME: Double-saving + participation = form.save() + participation.status = participation_status.dropped + participation.save() + reset_form = True + + messages.add_message(request, messages.SUCCESS, + _("Successfully dropped.")) + except IntegrityError as e: + messages.add_message(request, messages.ERROR, + _("A data integrity issue was detected when saving " + "this participation. Maybe a participation for " + "this user already exists? (%s)") + % str(e)) if reset_form: form = EditParticipationForm( -- GitLab From c13c3710f0651c0ba67b7337ec567fbccc11ed76 Mon Sep 17 00:00:00 2001 From: Andreas Kloeckner Date: Mon, 28 Aug 2017 20:00:36 -0500 Subject: [PATCH 09/45] Do not assume prev_answer_visits is non-empty in, see also 63afcc4e and #351 on Github --- course/flow.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/course/flow.py b/course/flow.py index 76f78c48..9122f446 100644 --- a/course/flow.py +++ b/course/flow.py @@ -1839,7 +1839,9 @@ def view_flow_page(pctx, flow_session_id, ordinal): answer_data, answer_was_graded) = post_result - prev_visit_id = prev_answer_visits[0].id + if prev_answer_visits: + prev_visit_id = prev_answer_visits[0].id + # continue at common flow page generation below else: -- GitLab From c00efc31605a4b3ea0642435b069e00cfaf31fc4 Mon Sep 17 00:00:00 2001 From: dzhuang Date: Fri, 25 Aug 2017 15:51:40 +0800 Subject: [PATCH 10/45] system check at start up. --- course/apps.py | 5 + course/checks.py | 384 +++++++++++++++++++++++++++++++ local_settings.example.py | 17 ++ relate/settings.py | 12 +- test/base_test_mixins.py | 7 +- test/test_checks.py | 468 ++++++++++++++++++++++++++++++++++++++ 6 files changed, 882 insertions(+), 11 deletions(-) create mode 100644 course/checks.py create mode 100644 test/test_checks.py diff --git a/course/apps.py b/course/apps.py index 58f78b14..6e4d4511 100644 --- a/course/apps.py +++ b/course/apps.py @@ -1,5 +1,6 @@ from django.apps import AppConfig from django.utils.translation import ugettext_lazy as _ +from course.checks import register_startup_checks_extra, register_startup_checks class CourseConfig(AppConfig): @@ -9,3 +10,7 @@ class CourseConfig(AppConfig): def ready(self): import course.receivers # noqa + + # register all checks + register_startup_checks() + register_startup_checks_extra() diff --git a/course/checks.py b/course/checks.py new file mode 100644 index 00000000..5ecad540 --- /dev/null +++ b/course/checks.py @@ -0,0 +1,384 @@ +# -*- coding: utf-8 -*- + +from __future__ import division + +__copyright__ = "Copyright (C) 2017 Dong Zhuang" + +__license__ = """ +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in +all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +THE SOFTWARE. +""" + +import six +from django.conf import settings +from django.core.checks import Critical, Warning, register +from django.core.exceptions import ImproperlyConfigured + +REQUIRED_CONF_ERROR_PATTERN = ( + "You must configure %(location)s for RELATE to run properly.") +INSTANCE_ERROR_PATTERN = "%(location)s must be an instance of %(types)s." +GENERIC_ERROR_PATTERN = "Error in '%(location)s': %(error_type)s: %(error_str)s" + +EMAIL_CONNECTIONS = "EMAIL_CONNECTIONS" +RELATE_BASE_URL = "RELATE_BASE_URL" +RELATE_EMAIL_APPELATION_PRIORITY_LIST = "RELATE_EMAIL_APPELATION_PRIORITY_LIST" +RELATE_FACILITIES = "RELATE_FACILITIES" +RELATE_MAINTENANCE_MODE_EXCEPTIONS = "RELATE_MAINTENANCE_MODE_EXCEPTIONS" +RELATE_SESSION_RESTART_COOLDOWN_SECONDS = "RELATE_SESSION_RESTART_COOLDOWN_SECONDS" +RELATE_TICKET_MINUTES_VALID_AFTER_USE = "RELATE_TICKET_MINUTES_VALID_AFTER_USE" +GIT_ROOT = "GIT_ROOT" +RELATE_STARTUP_CHECKS = "RELATE_STARTUP_CHECKS" +RELATE_STARTUP_CHECKS_EXTRA = "RELATE_STARTUP_CHECKS_EXTRA" + +RELATE_STARTUP_CHECKS_TAG = "start_up_check" +RELATE_STARTUP_CHECKS_EXTRA_TAG = "startup_checks_extra" + + +class RelateCriticalCheckMessage(Critical): + def __init__(self, *args, **kwargs): + super(RelateCriticalCheckMessage, self).__init__(*args, **kwargs) + if not self.obj: + self.obj = ImproperlyConfigured.__name__ + + +class DeprecatedException(Exception): + pass + + +def get_ip_network(ip_range): + import ipaddress + return ipaddress.ip_network(six.text_type(ip_range)) + + +def check_relate_settings(app_configs, **kwargs): + errors = [] + + # {{{ check RELATE_BASE_URL + relate_base_url = getattr(settings, RELATE_BASE_URL, None) + if relate_base_url is None: + errors.append(RelateCriticalCheckMessage( + msg=REQUIRED_CONF_ERROR_PATTERN % {"location": RELATE_BASE_URL}, + id="relate_base_url.E001" + )) + elif not isinstance(relate_base_url, str): + errors.append(RelateCriticalCheckMessage( + msg=(INSTANCE_ERROR_PATTERN + % {"location": RELATE_BASE_URL, "types": "str"}), + id="relate_base_url.E002" + )) + elif not relate_base_url.strip(): + errors.append(RelateCriticalCheckMessage( + msg="%(location)s should not be an empty string" + % {"location": RELATE_BASE_URL}, + id="relate_base_url.E003" + )) + # }}} + + # {{{ check RELATE_EMAIL_APPELATION_PRIORITY_LIST + relate_email_appelation_priority_list = getattr( + settings, RELATE_EMAIL_APPELATION_PRIORITY_LIST, None) + if relate_email_appelation_priority_list is not None: + if not isinstance(relate_email_appelation_priority_list, (list, tuple)): + errors.append(RelateCriticalCheckMessage( + msg=( + INSTANCE_ERROR_PATTERN + % {"location": RELATE_EMAIL_APPELATION_PRIORITY_LIST, + "types": "list or tuple"}), + id="relate_email_appelation_priority_list.E002") + ) + # }}} + + # {{{ check EMAIL_CONNECTIONS + email_connections = getattr(settings, EMAIL_CONNECTIONS, None) + if email_connections is not None: + if not isinstance(email_connections, dict): + errors.append(RelateCriticalCheckMessage( + msg=( + INSTANCE_ERROR_PATTERN + % {"location": EMAIL_CONNECTIONS, + "types": "dict"}), + id="email_connections.E001" + )) + else: + for label, c in six.iteritems(email_connections): + if not isinstance(c, dict): + errors.append(RelateCriticalCheckMessage( + msg=( + INSTANCE_ERROR_PATTERN + % {"location": "'%s' in '%s'" + % (label, EMAIL_CONNECTIONS), + "types": "dict"}), + id="email_connections.E002" + )) + else: + if "backend" in c: + from django.utils.module_loading import import_string + try: + import_string(c["backend"]) + except ImportError as e: + errors.append(RelateCriticalCheckMessage( + msg=( + GENERIC_ERROR_PATTERN + % { + "location": + "'%s' in %s" + % (label, RELATE_FACILITIES), + "error_type": type(e).__name__, + "error_str": str(e) + }), + id="email_connections.E003") + ) + # }}} + + # {{{ check RELATE_FACILITIES + + relate_facilities_conf = getattr(settings, RELATE_FACILITIES, None) + if relate_facilities_conf is not None: + from course.utils import get_facilities_config + try: + facilities = get_facilities_config() + except Exception as e: + errors.append(RelateCriticalCheckMessage( + msg=( + GENERIC_ERROR_PATTERN + % { + "location": RELATE_FACILITIES, + "error_type": type(e).__name__, + "error_str": str(e) + }), + id="relate_facilities.E001") + ) + else: + if not isinstance(facilities, dict): + errors.append(RelateCriticalCheckMessage( + msg=( + "'%(location)s' must either be or return a dictionary" + % {"location": RELATE_FACILITIES}), + id="relate_facilities.E002") + ) + else: + for facility, conf in six.iteritems(facilities): + if not isinstance(conf, dict): + errors.append(RelateCriticalCheckMessage( + msg=( + INSTANCE_ERROR_PATTERN + % {"location": + "Facility `%s` in %s" + % (facility, RELATE_FACILITIES), + "types": "dict"}), + id="relate_facilities.E003") + ) + else: + ip_ranges = conf.get("ip_ranges", []) + if ip_ranges: + if not isinstance(ip_ranges, (list, tuple)): + errors.append(RelateCriticalCheckMessage( + msg=( + INSTANCE_ERROR_PATTERN + % {"location": + "'ip_ranges' in facility `%s` in %s" + % (facilities, RELATE_FACILITIES), + "types": "list or tuple"}), + id="relate_facilities.E004") + ) + else: + for ip_range in ip_ranges: + try: + get_ip_network(ip_range) + except Exception as e: + errors.append(RelateCriticalCheckMessage( + msg=( + GENERIC_ERROR_PATTERN + % { + "location": + "'ip_ranges' in " + "facility `%s` in %s" + % (facility, + RELATE_FACILITIES), + "error_type": type(e).__name__, + "error_str": str(e) + }), + id="relate_facilities.E005") + ) + else: + if not callable(relate_facilities_conf): + errors.append(Warning( + msg=( + "Faclity `%s` in %s is an open facility " + "as it doesn't configured `ip_ranges`" + % (facility, RELATE_FACILITIES) + ), + id="relate_facilities.W001" + )) + + # }}} + + # {{{ check RELATE_MAINTENANCE_MODE_EXCEPTIONS + relate_maintenance_mode_exceptions = getattr( + settings, RELATE_MAINTENANCE_MODE_EXCEPTIONS, None) + if relate_maintenance_mode_exceptions is not None: + if not isinstance(relate_maintenance_mode_exceptions, (list, tuple)): + errors.append(RelateCriticalCheckMessage( + msg=(INSTANCE_ERROR_PATTERN + % {"location": RELATE_MAINTENANCE_MODE_EXCEPTIONS, + "types": "list or tuple"}), + id="relate_maintenance_mode_exceptions.E001") + ) + else: + for ip in relate_maintenance_mode_exceptions: + try: + get_ip_network(ip) + except Exception as e: + errors.append(RelateCriticalCheckMessage( + msg=( + GENERIC_ERROR_PATTERN + % {"location": + "ip/ip_ranges '%s' in %s" + % (ip, RELATE_FACILITIES), + "error_type": type(e).__name__, + "error_str": str(e) + }), + id="relate_maintenance_mode_exceptions.E002") + ) + # }}} + + # {{{ check RELATE_SESSION_RESTART_COOLDOWN_SECONDS + relate_session_restart_cooldown_seconds = getattr( + settings, RELATE_SESSION_RESTART_COOLDOWN_SECONDS, None) + if relate_session_restart_cooldown_seconds is not None: + if not isinstance(relate_session_restart_cooldown_seconds, (int, float)): + errors.append(RelateCriticalCheckMessage( + msg=(INSTANCE_ERROR_PATTERN + % {"location": RELATE_SESSION_RESTART_COOLDOWN_SECONDS, + "types": "int or float"}), + id="relate_session_restart_cooldown_seconds.E001") + ) + else: + if relate_session_restart_cooldown_seconds < 0: + errors.append(RelateCriticalCheckMessage( + msg=( + "%(location)s must be a positive number, " + "got %(value)s instead" + % {"location": RELATE_SESSION_RESTART_COOLDOWN_SECONDS, + "value": relate_session_restart_cooldown_seconds}), + id="relate_session_restart_cooldown_seconds.E002") + ) + + # }}} + + # {{{ check RELATE_SESSION_RESTART_COOLDOWN_SECONDS + relate_ticket_minutes_valid_after_use = getattr( + settings, RELATE_TICKET_MINUTES_VALID_AFTER_USE, None) + if relate_ticket_minutes_valid_after_use is not None: + if not isinstance(relate_ticket_minutes_valid_after_use, (int, float)): + errors.append(RelateCriticalCheckMessage( + msg=(INSTANCE_ERROR_PATTERN + % {"location": RELATE_TICKET_MINUTES_VALID_AFTER_USE, + "types": "int or float"}), + id="relate_ticket_minutes_valid_after_use.E001") + ) + else: + if relate_ticket_minutes_valid_after_use < 0: + errors.append(RelateCriticalCheckMessage( + msg=( + "%(location)s must be a positive number, " + "got %(value)s instead" + % {"location": RELATE_TICKET_MINUTES_VALID_AFTER_USE, + "value": relate_ticket_minutes_valid_after_use}), + id="relate_ticket_minutes_valid_after_use.E002") + ) + + # }}} + + # {{{ check GIT_ROOT + git_root = getattr(settings, GIT_ROOT, None) + if git_root is None: + errors.append(RelateCriticalCheckMessage( + msg=REQUIRED_CONF_ERROR_PATTERN % {"location": GIT_ROOT}, + id="git_root.E001" + )) + elif not isinstance(git_root, str): + errors.append(RelateCriticalCheckMessage( + msg=INSTANCE_ERROR_PATTERN % {"location": GIT_ROOT, "types": "str"}, + id="git_root.E002" + )) + else: + import os + if not os.path.isdir(git_root): + errors.append(RelateCriticalCheckMessage( + msg=("`%(path)s` connfigured in %(location)s is not a valid path" + % {"path": git_root, "location": GIT_ROOT}), + id="git_root.E003" + )) + else: + if not os.access(git_root, os.W_OK): + errors.append(RelateCriticalCheckMessage( + msg=("`%(path)s` connfigured in %(location)s is not writable " + "by RELATE" + % {"path": git_root, "location": GIT_ROOT}), + id="git_root.E004" + )) + if not os.access(git_root, os.R_OK): + errors.append(RelateCriticalCheckMessage( + msg=("`%(path)s` connfigured in %(location)s is not readable " + "by RELATE" + % {"path": git_root, "location": GIT_ROOT}), + id="git_root.E005" + )) + + # }}} + + return errors + + +def register_startup_checks(): + register(check_relate_settings, RELATE_STARTUP_CHECKS_TAG) + + +def register_startup_checks_extra(): + """ + Register extra checks provided by user. + Here we will have to raise error for Exceptions, as that can not be done + via check: all checks, including check_relate_settings, will only be + executed after self.ready() is done. + """ + startup_checks_extra = getattr(settings, RELATE_STARTUP_CHECKS_EXTRA, None) + if startup_checks_extra: + if not isinstance(startup_checks_extra, (list, tuple)): + raise ImproperlyConfigured( + INSTANCE_ERROR_PATTERN + % {"location": RELATE_STARTUP_CHECKS_EXTRA, + "types": "list or tuple" + } + ) + from django.utils.module_loading import import_string + for c in startup_checks_extra: + try: + check_item = import_string(c) + except Exception as e: + raise ImproperlyConfigured( + GENERIC_ERROR_PATTERN + % { + "location": RELATE_STARTUP_CHECKS_EXTRA, + "error_type": type(e).__name__, + "error_str": str(e) + }) + else: + register(check_item, RELATE_STARTUP_CHECKS_EXTRA_TAG) + +# vim: foldmethod=marker diff --git a/local_settings.example.py b/local_settings.example.py index 93d468b6..a7fbdb38 100644 --- a/local_settings.example.py +++ b/local_settings.example.py @@ -231,6 +231,23 @@ RELATE_SHOW_EDITOR_FORM = True # }}} +# {{{ extra checks + +# This allow user to add customized startup checkes for user-defined modules +# using Django's system checks (https://docs.djangoproject.com/en/dev/ref/checks/) +# For example, define a `my_check_func in `my_module` with +# +# def my_check_func(app_configs, **kwargs): +# return [list of error] +# +# The configuration should be +# RELATE_STARTUP_CHECKS_EXTRA = ["my_module.my_check_func"] +# i.e., Each item should be the path to an importable check function. +#RELATE_STARTUP_CHECKS_EXTRA = [] + +# }}} + + # {{{ docker # A string containing the image ID of the docker image to be used to run diff --git a/relate/settings.py b/relate/settings.py index 88045f2f..46694888 100644 --- a/relate/settings.py +++ b/relate/settings.py @@ -56,7 +56,7 @@ INSTALLED_APPS = ( "course", ) -if local_settings["RELATE_SIGN_IN_BY_SAML2_ENABLED"]: +if local_settings.get("RELATE_SIGN_IN_BY_SAML2_ENABLED"): INSTALLED_APPS = INSTALLED_APPS + ("djangosaml2",) # type: ignore # }}} @@ -89,7 +89,7 @@ AUTHENTICATION_BACKENDS = ( "django.contrib.auth.backends.ModelBackend", ) -if local_settings["RELATE_SIGN_IN_BY_SAML2_ENABLED"]: +if local_settings.get("RELATE_SIGN_IN_BY_SAML2_ENABLED"): AUTHENTICATION_BACKENDS = AUTHENTICATION_BACKENDS + ( # type: ignore 'course.auth.Saml2Backend', ) @@ -287,12 +287,4 @@ SAML_CREATE_UNKNOWN_USER = True # }}} -# This makes sure the RELATE_BASE_URL is configured. -assert local_settings["RELATE_BASE_URL"] - -# This makes sure RELATE_EMAIL_APPELATION_PRIORITY_LIST is a list -if "RELATE_EMAIL_APPELATION_PRIORITY_LIST" in local_settings: - assert isinstance( - local_settings["RELATE_EMAIL_APPELATION_PRIORITY_LIST"], list) - # vim: foldmethod=marker diff --git a/test/base_test_mixins.py b/test/base_test_mixins.py index 6925d7e9..5d7a01cc 100644 --- a/test/base_test_mixins.py +++ b/test/base_test_mixins.py @@ -101,7 +101,12 @@ def force_remove_path(path): func(path) import shutil - shutil.rmtree(path, onerror=remove_readonly) + try: + shutil.rmtree(path, onerror=remove_readonly) + except OSError: + # let the remove_exceptionally_undelete_course_repos method to delete + # the folder for the next test. + pass class SuperuserCreateMixin(object): diff --git a/test/test_checks.py b/test/test_checks.py new file mode 100644 index 00000000..13242c58 --- /dev/null +++ b/test/test_checks.py @@ -0,0 +1,468 @@ +from __future__ import division + +__copyright__ = "Copyright (C) 2017 Dong Zhuang" + +__license__ = """ +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in +all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +THE SOFTWARE. +""" + +import os +from django.test import SimpleTestCase +from django.test.utils import override_settings +try: + from unittest import mock +except: + import mock + + +class CheckRelateSettingsBase(SimpleTestCase): + @property + def func(self): + from course.checks import check_relate_settings + return check_relate_settings + + +class CheckRelateURL(CheckRelateSettingsBase): + VALID_CONF = "example.com" + INVALID_CONF_NONE = None + INVALID_CONF_EMPTY_LIST = [] + INVALID_CONF_SPACES = " " + + @override_settings(RELATE_BASE_URL=VALID_CONF) + def test_valid_relate_base_url1(self): + self.assertEqual(self.func(None), []) + + @override_settings(RELATE_BASE_URL=INVALID_CONF_NONE) + def test_invalid_relate_base_url_none(self): + result = self.func(None) + self.assertEqual([r.id for r in result], ["relate_base_url.E001"]) + + @override_settings(RELATE_BASE_URL=INVALID_CONF_EMPTY_LIST) + def test_invalid_relate_base_url_empty_list(self): + result = self.func(None) + self.assertEqual([r.id for r in result], ["relate_base_url.E002"]) + + @override_settings(RELATE_BASE_URL=INVALID_CONF_SPACES) + def test_invalid_relate_base_url_spaces(self): + result = self.func(None) + self.assertEqual([r.id for r in result], ["relate_base_url.E003"]) + + +class CheckRelateEmailAppelationPriorityList(CheckRelateSettingsBase): + VALID_CONF_NONE = None + VALID_CONF = ["name1", "name2"] + INVALID_CONF_STR = "name1" + + @override_settings(RELATE_EMAIL_APPELATION_PRIORITY_LIST=VALID_CONF_NONE) + def test_valid_relate_email_appelation_priority_list_none(self): + self.assertEqual(self.func(None), []) + + @override_settings(RELATE_EMAIL_APPELATION_PRIORITY_LIST=VALID_CONF) + def test_valid_relate_email_appelation_priority_list(self): + self.assertEqual(self.func(None), []) + + @override_settings(RELATE_EMAIL_APPELATION_PRIORITY_LIST=INVALID_CONF_STR) + def test_invalid_relate_email_appelation_priority_list_str(self): + self.assertEqual([r.id for r in self.func(None)], + ["relate_email_appelation_priority_list.E002"]) + + +class CheckRelateEmailConnections(CheckRelateSettingsBase): + VALID_CONF_NONE = None + VALID_CONF_EMPTY_DICT = {} + VALID_CONF = { + "robot": { + 'backend': 'django.core.mail.backends.console.EmailBackend', + 'host': 'smtp.gmail.com', + 'username': 'blah@blah.com', + 'password': 'password', + 'port': 587, + 'use_tls': True, + }, + "other": {} + } + INVALID_CONF_EMPTY_LIST = [] + INVALID_CONF_LIST = [VALID_CONF] + INVALID_CONF_LIST_AS_ITEM_VALUE = { + "robot": ['blah@blah.com'], + "other": [], + "yet_another": {} + } + INVALID_CONF_INVALID_BACKEND = { + "robot": { + 'backend': 'an.invalid.emailBackend', # invalid backend + 'host': 'smtp.gmail.com', + 'username': 'blah@blah.com', + 'password': 'password', + 'port': 587, + 'use_tls': True, + }, + "other": {} + } + + @override_settings(EMAIL_CONNECTIONS=VALID_CONF_NONE) + def test_valid_email_connections_none(self): + self.assertEqual(self.func(None), []) + + @override_settings(EMAIL_CONNECTIONS=VALID_CONF_EMPTY_DICT) + def test_valid_email_connections_emtpy_dict(self): + self.assertEqual(self.func(None), []) + + @override_settings(EMAIL_CONNECTIONS=VALID_CONF) + def test_valid_email_connections(self): + self.assertEqual(self.func(None), []) + + @override_settings(EMAIL_CONNECTIONS=INVALID_CONF_EMPTY_LIST) + def test_invalid_email_connections_empty_list(self): + self.assertEqual([r.id for r in self.func(None)], + ["email_connections.E001"]) + + @override_settings(EMAIL_CONNECTIONS=INVALID_CONF_LIST) + def test_invalid_email_connections_list(self): + self.assertEqual([r.id for r in self.func(None)], + ["email_connections.E001"]) + + @override_settings(EMAIL_CONNECTIONS=INVALID_CONF_LIST_AS_ITEM_VALUE) + def test_invalid_email_connections_list_as_item_value(self): + result = self.func(None) + self.assertEqual(len(result), 2) + self.assertEqual([r.id for r in result], + ["email_connections.E002", + "email_connections.E002"]) + + @override_settings(EMAIL_CONNECTIONS=INVALID_CONF_INVALID_BACKEND) + def test_invalid_email_connections_invalid_backend(self): + self.assertEqual([r.id for r in self.func(None)], + ["email_connections.E003"]) + + +class CheckRelateFacilities(CheckRelateSettingsBase): + VALID_CONF_NONE = None + VALID_CONF = ( + { + "test_center": { + "ip_ranges": ["192.168.192.0/24"], + "exams_only": False}, + "test_center2": { + "ip_ranges": ["192.168.10.0/24"]}, + }) + + INVALID_CONF_LIST = [] + INVALID_CONF_NOT_DICT_AS_ITEM_VALUE = ( + { + "test_center": { + "ip_ranges": ["192.168.192.0/24"], + "exams_only": False}, + "test_center2": [], # not a dict + "test_center3": ("192.168.10.0/24"), # not a dict + }) + + INVALID_CONF_IP_RANGES_NOT_LIST = ( + { + "test_center": { + "ip_ranges": "192.168.192.0/24", # not a list + "exams_only": False}, + "test_center2": [], + }) + + INVALID_CONF_IP_RANGES_ITEM_NOT_IPADDRESS = ( + { + "test_center": { + "ip_ranges": ["www.example.com", "localhost"] # invalid ipadd + }, + }) + + WARNING_CONF_IP_RANGES_NOT_CONFIGURED = ( + { + "test_center": {"exams_only": False}, + "test_center2": {}, + }) + + @override_settings(RELATE_FACILITIES=VALID_CONF_NONE) + def test_valid_relate_facilities_none(self): + self.assertEqual(self.func(None), []) + + @override_settings(RELATE_FACILITIES=VALID_CONF) + def test_valid_relate_facilities(self): + self.assertEqual(self.func(None), []) + + def test_valid_relate_facilities_callable(self): + def valid_func(now_datetime): + from django.utils.timezone import now + if now_datetime > now(): + return self.VALID_CONF + else: + return {} + + with override_settings(RELATE_FACILITIES=valid_func): + self.assertEqual(self.func(None), []) + + def test_valid_relate_facilities_callable_with_empty_ip_ranges(self): + def valid_func_though_return_emtpy_ip_ranges(now_datetime): + # this won't result in warnning, because the facility is defined + # by a callable. + return self.WARNING_CONF_IP_RANGES_NOT_CONFIGURED + with override_settings( + RELATE_FACILITIES=valid_func_though_return_emtpy_ip_ranges): + self.assertEqual(self.func(None), []) + + @override_settings(RELATE_FACILITIES=INVALID_CONF_LIST) + def test_invalid_relate_facilities_callable_return_list(self): + self.assertEqual([r.id for r in self.func(None)], + ["relate_facilities.E002"]) + + @override_settings(RELATE_FACILITIES=INVALID_CONF_NOT_DICT_AS_ITEM_VALUE) + def test_invalid_relate_facilities_callable_not_dict_as_item_value(self): + result = self.func(None) + self.assertEqual(len(result), 2) + self.assertEqual([r.id for r in result], + ["relate_facilities.E003", + "relate_facilities.E003"]) + + @override_settings(RELATE_FACILITIES=INVALID_CONF_IP_RANGES_NOT_LIST) + def test_invalid_relate_facilities_ip_ranges_not_list(self): + result = self.func(None) + self.assertEqual(len(result), 2) + self.assertEqual(sorted([r.id for r in result]), + sorted(["relate_facilities.E003", + "relate_facilities.E004"])) + + @override_settings(RELATE_FACILITIES=INVALID_CONF_IP_RANGES_ITEM_NOT_IPADDRESS) + def test_invalid_relate_facilities_ip_ranges_item_not_ipaddress(self): + result = self.func(None) + self.assertEqual(len(result), 2) + self.assertEqual(sorted([r.id for r in result]), + sorted(["relate_facilities.E005", + "relate_facilities.E005"])) + + def test_invalid_relate_facilities_callable_not_return_dict(self): + def invalid_func_not_return_dict(now_datetime): + return self.INVALID_CONF_LIST + + with override_settings(RELATE_FACILITIES=invalid_func_not_return_dict): + self.assertEqual([r.id for r in self.func(None)], + ["relate_facilities.E001"]) + + def test_invalid_relate_facilities_callable_return_invalid_conf(self): + def invalid_func_return_invalid_conf(now_datetime): + return self.INVALID_CONF_NOT_DICT_AS_ITEM_VALUE + + with override_settings(RELATE_FACILITIES=invalid_func_return_invalid_conf): + result = self.func(None) + self.assertEqual(len(result), 2) + self.assertEqual([r.id for r in result], + ["relate_facilities.E003", + "relate_facilities.E003"]) + + def test_invalid_relate_facilities_callable_return_none(self): + def invalid_func_return_none(now_datetime): + return None + + with override_settings(RELATE_FACILITIES=invalid_func_return_none): + self.assertEqual([r.id for r in self.func(None)], + ["relate_facilities.E001"]) + + @override_settings(RELATE_FACILITIES=WARNING_CONF_IP_RANGES_NOT_CONFIGURED) + def test_warning_relate_facilities(self): + result = self.func(None) + self.assertEqual(len(result), 2) + self.assertEqual([r.id for r in result], + ["relate_facilities.W001", + "relate_facilities.W001"]) + + +class CheckRelateMaintenanceModeExceptions(CheckRelateSettingsBase): + VALID_CONF_NONE = None + VALID_CONF_EMPTY_LIST = [] + VALID_CONF = ["127.0.0.1", "192.168.1.1"] + INVALID_CONF_STR = "127.0.0.1" + INVALID_CONF_DICT = {"localhost": "127.0.0.1", + "www.myrelate.com": "192.168.1.1"} + INVALID_CONF_INVALID_IPS = ["localhost", "www.myrelate.com"] + + @override_settings(RELATE_MAINTENANCE_MODE_EXCEPTIONS=VALID_CONF_NONE) + def test_valid_maintenance_mode_exceptions_none(self): + self.assertEqual(self.func(None), []) + + @override_settings(RELATE_MAINTENANCE_MODE_EXCEPTIONS=VALID_CONF_EMPTY_LIST) + def test_valid_maintenance_mode_exceptions_emtpy_list(self): + self.assertEqual(self.func(None), []) + + @override_settings(RELATE_MAINTENANCE_MODE_EXCEPTIONS=VALID_CONF) + def test_valid_maintenance_mode_exceptions(self): + self.assertEqual(self.func(None), []) + + @override_settings(RELATE_MAINTENANCE_MODE_EXCEPTIONS=INVALID_CONF_STR) + def test_invalid_maintenance_mode_exceptions_str(self): + self.assertEqual([r.id for r in self.func(None)], + ["relate_maintenance_mode_exceptions.E001"]) + + @override_settings(RELATE_MAINTENANCE_MODE_EXCEPTIONS=INVALID_CONF_DICT) + def test_invalid_maintenance_mode_exceptions_dict(self): + self.assertEqual([r.id for r in self.func(None)], + ["relate_maintenance_mode_exceptions.E001"]) + + @override_settings(RELATE_MAINTENANCE_MODE_EXCEPTIONS=INVALID_CONF_INVALID_IPS) + def test_invalid_maintenance_mode_exceptions_invalid_ipaddress(self): + result = self.func(None) + self.assertEqual(len(result), 2) + self.assertEqual([r.id for r in result], + ["relate_maintenance_mode_exceptions.E002", + "relate_maintenance_mode_exceptions.E002"]) + + +class CheckRelateSessionRestartCooldownSeconds(CheckRelateSettingsBase): + VALID_CONF = 10 + VALID_CONF_BY_CALC = 2 * 5 + INVALID_CONF_STR = "10" + INVALID_CONF_LIST = [10] + INVALID_CONF_NEGATIVE = -10 + + @override_settings(RELATE_SESSION_RESTART_COOLDOWN_SECONDS=VALID_CONF) + def test_valid_relate_session_restart_cooldown_seconds(self): + self.assertEqual(self.func(None), []) + + @override_settings(RELATE_SESSION_RESTART_COOLDOWN_SECONDS=VALID_CONF_BY_CALC) + def test_valid_relate_session_restart_cooldown_seconds_by_calc(self): + self.assertEqual(self.func(None), []) + + @override_settings(RELATE_SESSION_RESTART_COOLDOWN_SECONDS=INVALID_CONF_STR) + def test_invalid_maintenance_mode_exceptions_str(self): + self.assertEqual([r.id for r in self.func(None)], + ["relate_session_restart_cooldown_seconds.E001"]) + + @override_settings(RELATE_SESSION_RESTART_COOLDOWN_SECONDS=INVALID_CONF_LIST) + def test_invalid_maintenance_mode_exceptions_list(self): + self.assertEqual([r.id for r in self.func(None)], + ["relate_session_restart_cooldown_seconds.E001"]) + + @override_settings(RELATE_SESSION_RESTART_COOLDOWN_SECONDS=INVALID_CONF_NEGATIVE) + def test_invalid_maintenance_mode_exceptions_list_negative(self): + self.assertEqual([r.id for r in self.func(None)], + ["relate_session_restart_cooldown_seconds.E002"]) + + +class CheckRelateTicketMinutesValidAfterUse(CheckRelateSettingsBase): + VALID_CONF = 10 + VALID_CONF_BY_CALC = 2 * 5 + INVALID_CONF_STR = "10" + INVALID_CONF_LIST = [10] + INVALID_CONF_NEGATIVE = -10 + + @override_settings(RELATE_TICKET_MINUTES_VALID_AFTER_USE=VALID_CONF) + def test_valid_relate_ticket_minutes_valid_after_use(self): + self.assertEqual(self.func(None), []) + + @override_settings(RELATE_TICKET_MINUTES_VALID_AFTER_USE=VALID_CONF_BY_CALC) + def test_valid_relate_ticket_minutes_valid_after_use_by_calc(self): + self.assertEqual(self.func(None), []) + + @override_settings(RELATE_TICKET_MINUTES_VALID_AFTER_USE=INVALID_CONF_STR) + def test_invalid_relate_ticket_minutes_valid_after_use_str(self): + self.assertEqual([r.id for r in self.func(None)], + ["relate_ticket_minutes_valid_after_use.E001"]) + + @override_settings(RELATE_TICKET_MINUTES_VALID_AFTER_USE=INVALID_CONF_LIST) + def test_invalid_relate_ticket_minutes_valid_after_use_list(self): + self.assertEqual([r.id for r in self.func(None)], + ["relate_ticket_minutes_valid_after_use.E001"]) + + @override_settings(RELATE_TICKET_MINUTES_VALID_AFTER_USE=INVALID_CONF_NEGATIVE) + def test_invalid_relate_ticket_minutes_valid_after_use_negative(self): + self.assertEqual([r.id for r in self.func(None)], + ["relate_ticket_minutes_valid_after_use.E002"]) + + +def side_effect_os_path_is_dir(*args, **kwargs): + if args[0].startswith("dir"): + return True + return False + + +def side_effect_os_access(*args, **kwargs): + if args[0].endswith("NEITHER"): + return False + elif args[0].endswith("W_FAIL"): + if args[1] == os.W_OK: + return False + elif args[0].endswith("R_FAIL"): + if args[1] == os.R_OK: + return False + return True + + +@mock.patch('os.access', side_effect=side_effect_os_access) +@mock.patch("os.path.isdir", side_effect=side_effect_os_path_is_dir) +class CheckGitRoot(CheckRelateSettingsBase): + VALID_GIT_ROOT = "dir/git/root/path" + + INVALID_GIT_ROOT_NONE = None + INVALID_GIT_ROOT_LIST = [VALID_GIT_ROOT] + INVALID_GIT_ROOT_SPACES = " " + INVALID_GIT_ROOT_NOT_DIR = "not_dir/git/root/path" + INVALID_GIT_ROOT_W_FAIL = "dir/git/root/path/W_FAIL" + INVALID_GIT_ROOT_R_FAIL = "dir/git/root/path/R_FAIL" + INVALID_GIT_ROOT_W_R_FAIL = "dir/git/root/path/NEITHER" + + @override_settings(GIT_ROOT=VALID_GIT_ROOT) + def test_valid_git_root(self, mocked_os_access, mocked_os_path_is_dir): + self.assertEqual(self.func(None), []) + + @override_settings(GIT_ROOT=INVALID_GIT_ROOT_NONE) + def test_invalid_git_root_none(self, mocked_os_access, mocked_os_path_is_dir): + self.assertEqual([r.id for r in self.func(None)], + ["git_root.E001"]) + + @override_settings(GIT_ROOT=INVALID_GIT_ROOT_LIST) + def test_invalid_git_root_list(self, mocked_os_access, mocked_os_path_is_dir): + self.assertEqual([r.id for r in self.func(None)], + ["git_root.E002"]) + + @override_settings(GIT_ROOT=INVALID_GIT_ROOT_SPACES) + def test_invalid_git_root_spaces(self, mocked_os_access, mocked_os_path_is_dir): + self.assertEqual([r.id for r in self.func(None)], + ["git_root.E003"]) + + @override_settings(GIT_ROOT=INVALID_GIT_ROOT_NOT_DIR) + def test_invalid_git_root(self, mocked_os_access, mocked_os_path_is_dir): + self.assertEqual([r.id for r in self.func(None)], + ["git_root.E003"]) + + @override_settings(GIT_ROOT=INVALID_GIT_ROOT_W_FAIL) + def test_invalid_git_root_no_write_perm( + self, mocked_os_access, mocked_os_path_is_dir): + # no write permission + self.assertEqual([r.id for r in self.func(None)], + ["git_root.E004"]) + + @override_settings(GIT_ROOT=INVALID_GIT_ROOT_R_FAIL) + def test_invalid_git_root_no_read_perms( + self, mocked_os_access, mocked_os_path_is_dir): + # no read permission + self.assertEqual([r.id for r in self.func(None)], + ["git_root.E005"]) + + @override_settings(GIT_ROOT=INVALID_GIT_ROOT_W_R_FAIL) + def test_invalid_git_root_no_both_perms( + self, mocked_os_access, mocked_os_path_is_dir): + # no write and read permissions + result = self.func(None) + self.assertEqual(len(result), 2) + self.assertEqual([r.id for r in result], + ["git_root.E004", "git_root.E005"]) -- GitLab From 27510be241cd046d36e9aa086349459d0cba09b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20Kl=C3=B6ckner?= Date: Tue, 29 Aug 2017 14:19:55 -0400 Subject: [PATCH 11/45] Update checks.py --- course/checks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/course/checks.py b/course/checks.py index 5ecad540..a4b59f2e 100644 --- a/course/checks.py +++ b/course/checks.py @@ -220,7 +220,7 @@ def check_relate_settings(app_configs, **kwargs): errors.append(Warning( msg=( "Faclity `%s` in %s is an open facility " - "as it doesn't configured `ip_ranges`" + "as it has no configured `ip_ranges`" % (facility, RELATE_FACILITIES) ), id="relate_facilities.W001" -- GitLab From 0eab96d9857ebdfae72dad548a5d31752acdc6c0 Mon Sep 17 00:00:00 2001 From: Andreas Kloeckner Date: Thu, 31 Aug 2017 12:08:16 -0500 Subject: [PATCH 12/45] YAML parsing: catch and prevent lines that use tabs in indentation --- course/content.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/course/content.py b/course/content.py index 5c7e8bd3..407d1ada 100644 --- a/course/content.py +++ b/course/content.py @@ -579,6 +579,9 @@ def get_raw_yaml_from_repo(repo, full_name, commit_sha): return result +LINE_HAS_INDENTING_TABS_RE = re.compile("^\s*\t\s*", re.MULTILINE) + + def get_yaml_from_repo(repo, full_name, commit_sha, cached=True): # type: (Repo_ish, Text, bytes, bool) -> Any @@ -605,10 +608,16 @@ def get_yaml_from_repo(repo, full_name, commit_sha, cached=True): if result is not None: return result + yaml_bytestream = get_repo_blob( + repo, full_name, commit_sha, allow_tree=False).data + yaml_text = yaml_bytestream.decode("utf-8") + + if LINE_HAS_INDENTING_TABS_RE.search(yaml_text): + raise ValueError("File uses tabs in indentation. " + "This is not allowed.") + expanded = expand_yaml_macros( - repo, commit_sha, - get_repo_blob(repo, full_name, commit_sha, - allow_tree=False).data) + repo, commit_sha, yaml_bytestream) result = dict_to_struct(load_yaml(expanded)) -- GitLab From ede6ec12ddf4bbd05fce194c231df6a63f690fd0 Mon Sep 17 00:00:00 2001 From: Andreas Kloeckner Date: Thu, 31 Aug 2017 12:08:58 -0500 Subject: [PATCH 13/45] Fix indentation (=nesting) bug in static pages validation, which only caused the last static page to actually be validated --- course/validation.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/course/validation.py b/course/validation.py index 06c4f644..27ed619c 100644 --- a/course/validation.py +++ b/course/validation.py @@ -1512,11 +1512,11 @@ def validate_course_content(repo, course_file, events_file, )) % entry_path) - location = "staticpages/%s" % entry_path - page_desc = get_yaml_from_repo_safely(repo, location, - commit_sha=validate_sha) + location = "staticpages/%s" % entry_path + page_desc = get_yaml_from_repo_safely(repo, location, + commit_sha=validate_sha) - validate_staticpage_desc(vctx, location, page_desc) + validate_staticpage_desc(vctx, location, page_desc) # }}} -- GitLab From 9ae39690c30ffaa74f2793fca726e2b3b5d69eb3 Mon Sep 17 00:00:00 2001 From: Andreas Kloeckner Date: Thu, 31 Aug 2017 15:43:50 -0500 Subject: [PATCH 14/45] Doc fixes (Thanks Andrew Park) --- course/page/code.py | 6 ++++-- doc/flow.rst | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/course/page/code.py b/course/page/code.py index 520fa22f..6c36b5a2 100644 --- a/course/page/code.py +++ b/course/page/code.py @@ -368,8 +368,10 @@ class PythonCodeQuestion(PageBaseWithTitle, PageBaseWithValue): .. attribute:: test_code Optional. - Symbols that the participant's code is expected to define. - These will be made available to the :attr:`test_code`. + Code that will be run to determine the correctness of a + student-provided solution. Will have access to variables in + :attr:`names_from_user` (which will be *None*) if not provided. Should + never raise an exception. This may contain the marker "###CORRECT_CODE###", which will be replaced with the contents of :attr:`correct_code`, with diff --git a/doc/flow.rst b/doc/flow.rst index abfcbd24..b1cf6ba3 100644 --- a/doc/flow.rst +++ b/doc/flow.rst @@ -891,11 +891,11 @@ The rules for this can be written as follows:: if_has_role: [student, instructor] if_after: exam 1 - 1 week if_before: end:exam 1 + 2 weeks - permissions: [view, submit_answer, end_sesion, cannot_see_flow_result, lock_down_as_exam_session] + permissions: [view, submit_answer, end_session, cannot_see_flow_result, lock_down_as_exam_session] - if_has_role: [instructor] - permissions: [view, submit_answer, end_sesion, cannot_see_flow_result, lock_down_as_exam_session] + permissions: [view, submit_answer, end_session, cannot_see_flow_result, lock_down_as_exam_session] - permissions: [] -- GitLab From 137d21a9983aeb2eecfb0a59e948f59222cd69aa Mon Sep 17 00:00:00 2001 From: dzhuang Date: Sat, 2 Sep 2017 23:59:15 +0800 Subject: [PATCH 15/45] Added .travis.yml --- .travis.yml | 11 +++++++++++ README.rst | 6 ++++++ run-coveralls.sh | 7 +++++++ run-tests-for-ci.sh | 3 +-- 4 files changed, 25 insertions(+), 2 deletions(-) create mode 100644 .travis.yml create mode 100644 run-coveralls.sh diff --git a/.travis.yml b/.travis.yml new file mode 100644 index 00000000..17906f72 --- /dev/null +++ b/.travis.yml @@ -0,0 +1,11 @@ +language: python +python: + - "2.7" + - "3.5" +install: true +script: + - bash ./run-coveralls.sh +env: + - if [[$TRAVIS_PYTHON_VERSION = "2.7"]]; then PY_EXE=python2.7; else PY_EXE=python3.5; fi +after_success: + - coveralls \ No newline at end of file diff --git a/README.rst b/README.rst index e44fa5c0..b9f9f04d 100644 --- a/README.rst +++ b/README.rst @@ -1,3 +1,9 @@ +.. image:: https://travis-ci.org/inducer/relate.svg + :target: https://travis-ci.org/inducer/relate + +.. image:: https://coveralls.io/repos/inducer/relate/badge.svg + :target: https://coveralls.io/r/inducer/relate + RELATE ====== diff --git a/run-coveralls.sh b/run-coveralls.sh new file mode 100644 index 00000000..5db8c404 --- /dev/null +++ b/run-coveralls.sh @@ -0,0 +1,7 @@ +#! /bin/bash + +source $(dirname $0)/run-tests-for-ci.sh + +$PIP install coveralls + +coverage run --source=. manage.py test test/ diff --git a/run-tests-for-ci.sh b/run-tests-for-ci.sh index 0d36dd9b..1e477cae 100644 --- a/run-tests-for-ci.sh +++ b/run-tests-for-ci.sh @@ -68,5 +68,4 @@ $PIP install -r req.txt cp local_settings.example.py local_settings.py -cd test -python ../manage.py test +python manage.py test test/ -- GitLab From a078a2da0e77067f9981672993d64ec5e1b67787 Mon Sep 17 00:00:00 2001 From: Andreas Kloeckner Date: Sun, 3 Sep 2017 11:42:48 -0500 Subject: [PATCH 16/45] Add container connection error to nuisance alerts --- course/page/code.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/course/page/code.py b/course/page/code.py index 6c36b5a2..c7ed5a21 100644 --- a/course/page/code.py +++ b/course/page/code.py @@ -270,6 +270,12 @@ def is_nuisance_failure(result): return True + if ("traceback" in result + and "requests.packages.urllib3.exceptions.NewConnectionError" + in result["traceback"]): + + return True + return False -- GitLab From 45d7f79abfbfc06ffabe8f3457de792845650e8f Mon Sep 17 00:00:00 2001 From: dzhuang Date: Sun, 3 Sep 2017 14:18:50 +0800 Subject: [PATCH 17/45] Fixed failed coverage for coveralls.io. Adding coverage report for Gitlab. --- .gitlab-ci.yml | 2 ++ .travis.yml | 3 +-- README.rst | 10 ++++++++-- run-coveralls.sh | 3 +-- run-tests-for-ci.sh | 6 +++++- setup.cfg | 41 +++++++++++++++++++++++++++++++++++++++++ 6 files changed, 58 insertions(+), 7 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 9406b9af..fb58f198 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -5,6 +5,7 @@ - python2.7 except: - tags + coverage: '/TOTAL.+ ([0-9]{1,3}%)/' Python 3.5: script: @@ -13,6 +14,7 @@ Python 3.5: - python3.5 except: - tags + coverage: '/TOTAL.+ ([0-9]{1,3}%)/' Documentation: script: diff --git a/.travis.yml b/.travis.yml index 17906f72..28d5c51b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,4 +1,5 @@ language: python +cache: pip python: - "2.7" - "3.5" @@ -7,5 +8,3 @@ script: - bash ./run-coveralls.sh env: - if [[$TRAVIS_PYTHON_VERSION = "2.7"]]; then PY_EXE=python2.7; else PY_EXE=python3.5; fi -after_success: - - coveralls \ No newline at end of file diff --git a/README.rst b/README.rst index b9f9f04d..e984c2b8 100644 --- a/README.rst +++ b/README.rst @@ -1,8 +1,14 @@ .. image:: https://travis-ci.org/inducer/relate.svg - :target: https://travis-ci.org/inducer/relate + :target: https://travis-ci.org/inducer/relate .. image:: https://coveralls.io/repos/inducer/relate/badge.svg - :target: https://coveralls.io/r/inducer/relate + :target: https://coveralls.io/r/inducer/relate + +.. image:: https://gitlab.tiker.net/inducer/relate/badges/master/build.svg + :target: https://gitlab.tiker.net/inducer/relate/commits/master + +.. image:: https://gitlab.tiker.net/inducer/relate/badges/master/coverage.svg?job=Python+3.5 + RELATE ====== diff --git a/run-coveralls.sh b/run-coveralls.sh index 5db8c404..8a8a08fd 100644 --- a/run-coveralls.sh +++ b/run-coveralls.sh @@ -3,5 +3,4 @@ source $(dirname $0)/run-tests-for-ci.sh $PIP install coveralls - -coverage run --source=. manage.py test test/ +coveralls diff --git a/run-tests-for-ci.sh b/run-tests-for-ci.sh index 1e477cae..f330de3a 100644 --- a/run-tests-for-ci.sh +++ b/run-tests-for-ci.sh @@ -68,4 +68,8 @@ $PIP install -r req.txt cp local_settings.example.py local_settings.py -python manage.py test test/ +#python manage.py test test/ + +$PIP install coverage +coverage run --source=. manage.py test test/ +coverage report -m \ No newline at end of file diff --git a/setup.cfg b/setup.cfg index 4670a3c1..05106c25 100644 --- a/setup.cfg +++ b/setup.cfg @@ -2,3 +2,44 @@ ignore = E126,E127,E128,E123,E226,E241,E242,E265,E402,W503 max-line-length=85 exclude=course/migrations,accounts/migrations,static,components,course/mdx_mathjax.py + +[coverage:run] +branch=True +cover_pylib=False +omit = + */.env/* + */__init__.py + */virtualenv*/* + */setuptools*/* + */migrations/* + */mdx_mathjax.py + contrib/* + exercise-docker.py + update-attempt-ids.py + setup.py + local_settings.example.py + course/page/code_feedback.py + course/page/code_runpy_backend.py + */wsgi.py + */test/* + */tests.py + +[coverage:report] +exclude_lines = + # Have to re-enable the standard pragma + pragma: no cover + + # Don't complain about missing debug-only code: + def __repr__ + if self.debug + if settings.Debug + + # Don't complain if tests don't hit defensive assertion code: + raise AssertionError + raise NotImplementedError + + # Don't complain if non-runnable code isn't run: + if 0: + if __name__ == .__main__.: + +ignore_errors = True -- GitLab From 56e326455f2980eac32a6fec189ed1769294def5 Mon Sep 17 00:00:00 2001 From: dzhuang Date: Mon, 4 Sep 2017 13:21:33 +0800 Subject: [PATCH 18/45] Removed unnecessary badges and updated coveralls badges. --- .travis.yml | 6 ++---- README.rst | 12 +++--------- 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/.travis.yml b/.travis.yml index 28d5c51b..63bcd6d8 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,10 +1,8 @@ language: python cache: pip -python: - - "2.7" - - "3.5" install: true script: - bash ./run-coveralls.sh env: - - if [[$TRAVIS_PYTHON_VERSION = "2.7"]]; then PY_EXE=python2.7; else PY_EXE=python3.5; fi + - PY_EXE=python2.7 + - PY_EXE=python3.5 diff --git a/README.rst b/README.rst index e984c2b8..43eee408 100644 --- a/README.rst +++ b/README.rst @@ -1,14 +1,8 @@ -.. image:: https://travis-ci.org/inducer/relate.svg +.. image:: https://travis-ci.org/inducer/relate.svg?branch=master :target: https://travis-ci.org/inducer/relate -.. image:: https://coveralls.io/repos/inducer/relate/badge.svg - :target: https://coveralls.io/r/inducer/relate - -.. image:: https://gitlab.tiker.net/inducer/relate/badges/master/build.svg - :target: https://gitlab.tiker.net/inducer/relate/commits/master - -.. image:: https://gitlab.tiker.net/inducer/relate/badges/master/coverage.svg?job=Python+3.5 - +.. image:: https://coveralls.io/repos/github/inducer/relate/badge.svg?branch=master + :target: https://coveralls.io/github/inducer/relate?branch=master RELATE ====== -- GitLab From 002af5fba8c24b3382c5acc26581015467ea321b Mon Sep 17 00:00:00 2001 From: dzhuang Date: Tue, 5 Sep 2017 18:42:56 +0800 Subject: [PATCH 19/45] Enable appveyor and switch to codecov.io for coverage. --- .gitlab-ci.yml | 4 ++ .travis.yml | 8 ++-- README.rst | 13 +++--- appveyor.yml | 72 ++++++++++++++++++++++++++++++++ appveyor/run_with_env.cmd | 88 +++++++++++++++++++++++++++++++++++++++ run-coveralls.sh | 6 --- run-tests-for-ci.sh | 5 ++- 7 files changed, 176 insertions(+), 20 deletions(-) create mode 100644 appveyor.yml create mode 100644 appveyor/run_with_env.cmd delete mode 100644 run-coveralls.sh diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index fb58f198..28ec9192 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -6,6 +6,8 @@ except: - tags coverage: '/TOTAL.+ ([0-9]{1,3}%)/' + variables: + CODECOV_TOKEN: "a0ed9aa4-e4d9-4b24-bf50-0d54e407bd92" Python 3.5: script: @@ -15,6 +17,8 @@ Python 3.5: except: - tags coverage: '/TOTAL.+ ([0-9]{1,3}%)/' + variables: + CODECOV_TOKEN: "a0ed9aa4-e4d9-4b24-bf50-0d54e407bd92" Documentation: script: diff --git a/.travis.yml b/.travis.yml index 28d5c51b..d03d74ff 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,10 +1,8 @@ language: python cache: pip -python: - - "2.7" - - "3.5" install: true script: - - bash ./run-coveralls.sh + - bash ./run-tests-for-ci.sh env: - - if [[$TRAVIS_PYTHON_VERSION = "2.7"]]; then PY_EXE=python2.7; else PY_EXE=python3.5; fi + - PY_EXE=python2.7 + - PY_EXE=python3.5 diff --git a/README.rst b/README.rst index e984c2b8..532c9150 100644 --- a/README.rst +++ b/README.rst @@ -1,14 +1,11 @@ -.. image:: https://travis-ci.org/inducer/relate.svg +.. image:: https://travis-ci.org/inducer/relate.svg?branch=master :target: https://travis-ci.org/inducer/relate -.. image:: https://coveralls.io/repos/inducer/relate/badge.svg - :target: https://coveralls.io/r/inducer/relate - -.. image:: https://gitlab.tiker.net/inducer/relate/badges/master/build.svg - :target: https://gitlab.tiker.net/inducer/relate/commits/master - -.. image:: https://gitlab.tiker.net/inducer/relate/badges/master/coverage.svg?job=Python+3.5 +.. image:: https://ci.appveyor.com/api/projects/status/54c937lry6mjvtx8/branch/master?svg=true + :target: https://ci.appveyor.com/project/inducer/relate +.. image:: https://codecov.io/gh/inducer/relate/branch/master/graph/badge.svg + :target: https://codecov.io/gh/inducer/relate/commits RELATE ====== diff --git a/appveyor.yml b/appveyor.yml new file mode 100644 index 00000000..db90d589 --- /dev/null +++ b/appveyor.yml @@ -0,0 +1,72 @@ +# +# Most of these settings are taken from here: +# https://github.com/ogrisel/python-appveyor-demo/blob/master/appveyor.yml +# + +version: 1.0.{build} + +build: off + +cache: + - '%LOCALAPPDATA%\pip\Cache' + +environment: + global: + # SDK v7.0 MSVC Express 2008's SetEnv.cmd script will fail if the + # /E:ON and /V:ON options are not enabled in the batch script intepreter + # See: http://stackoverflow.com/a/13751649/163740 + CMD_IN_ENV: "cmd /E:ON /V:ON /C .\\appveyor\\run_with_env.cmd" + + matrix: + # Pre-installed Python versions, which Appveyor may upgrade to + # a later point release. + # See: http://www.appveyor.com/docs/installed-software#python + + - PYTHON: "C:\\Python27-x64" + PYTHON_VERSION: "2.7.x" # currently 2.7.13 + PYTHON_ARCH: "64" + + - PYTHON: "C:\\Python35-x64" + PYTHON_VERSION: "3.5.x" # currently 3.5.3 + PYTHON_ARCH: "64" + +init: + - "ECHO %PYTHON%" + - ps: "ls C:/Python*" + +install: + # Prepend newly installed Python to the PATH of this build (this cannot be + # done from inside the powershell script as it would require to restart + # the parent CMD process). + - "SET PATH=%PYTHON%;%PYTHON%\\Scripts;%PATH%" + + - ECHO "Filesystem root:" + - ps: "ls \"C:/\"" + + # Check that we have the expected version and architecture for Python + - "python --version" + - "python -c \"import struct; print(struct.calcsize('P') * 8)\"" + + # Upgrade to the latest version of pip to avoid it displaying warnings + # about it being out of date. + - "pip install --disable-pip-version-check --user --upgrade pip" + + - "git submodule update --init --recursive" + + # Install requirements + - ps: "cat requirements.txt | ?{$_ -notmatch 'dnspython'} > req.txt" + - "%CMD_IN_ENV% pip install -r req.txt" + - ps: "if ($env:PYTHON -like '*Python2*') { pip install dnspython mock} else { pip install dnspython3 }" + + +before_test: + - "%CMD_IN_ENV% pip install coverage" + +test_script: + # Run the project tests + - "copy local_settings.example.py local_settings.py /Y" + - "%CMD_IN_ENV% coverage run manage.py test test/" + +after_test: + - "%CMD_IN_ENV% pip install codecov" + - "%CMD_IN_ENV% codecov -X gcov" diff --git a/appveyor/run_with_env.cmd b/appveyor/run_with_env.cmd new file mode 100644 index 00000000..5da547c4 --- /dev/null +++ b/appveyor/run_with_env.cmd @@ -0,0 +1,88 @@ +:: To build extensions for 64 bit Python 3, we need to configure environment +:: variables to use the MSVC 2010 C++ compilers from GRMSDKX_EN_DVD.iso of: +:: MS Windows SDK for Windows 7 and .NET Framework 4 (SDK v7.1) +:: +:: To build extensions for 64 bit Python 2, we need to configure environment +:: variables to use the MSVC 2008 C++ compilers from GRMSDKX_EN_DVD.iso of: +:: MS Windows SDK for Windows 7 and .NET Framework 3.5 (SDK v7.0) +:: +:: 32 bit builds, and 64-bit builds for 3.5 and beyond, do not require specific +:: environment configurations. +:: +:: Note: this script needs to be run with the /E:ON and /V:ON flags for the +:: cmd interpreter, at least for (SDK v7.0) +:: +:: More details at: +:: https://github.com/cython/cython/wiki/64BitCythonExtensionsOnWindows +:: http://stackoverflow.com/a/13751649/163740 +:: +:: Author: Olivier Grisel +:: License: CC0 1.0 Universal: http://creativecommons.org/publicdomain/zero/1.0/ +:: +:: Notes about batch files for Python people: +:: +:: Quotes in values are literally part of the values: +:: SET FOO="bar" +:: FOO is now five characters long: " b a r " +:: If you don't want quotes, don't include them on the right-hand side. +:: +:: The CALL lines at the end of this file look redundant, but if you move them +:: outside of the IF clauses, they do not run properly in the SET_SDK_64==Y +:: case, I don't know why. +@ECHO OFF + +SET COMMAND_TO_RUN=%* +SET WIN_SDK_ROOT=C:\Program Files\Microsoft SDKs\Windows +SET WIN_WDK=c:\Program Files (x86)\Windows Kits\10\Include\wdf + +:: Extract the major and minor versions, and allow for the minor version to be +:: more than 9. This requires the version number to have two dots in it. +SET MAJOR_PYTHON_VERSION=%PYTHON_VERSION:~0,1% +IF "%PYTHON_VERSION:~3,1%" == "." ( + SET MINOR_PYTHON_VERSION=%PYTHON_VERSION:~2,1% +) ELSE ( + SET MINOR_PYTHON_VERSION=%PYTHON_VERSION:~2,2% +) + +:: Based on the Python version, determine what SDK version to use, and whether +:: to set the SDK for 64-bit. +IF %MAJOR_PYTHON_VERSION% == 2 ( + SET WINDOWS_SDK_VERSION="v7.0" + SET SET_SDK_64=Y +) ELSE ( + IF %MAJOR_PYTHON_VERSION% == 3 ( + SET WINDOWS_SDK_VERSION="v7.1" + IF %MINOR_PYTHON_VERSION% LEQ 4 ( + SET SET_SDK_64=Y + ) ELSE ( + SET SET_SDK_64=N + IF EXIST "%WIN_WDK%" ( + :: See: https://connect.microsoft.com/VisualStudio/feedback/details/1610302/ + REN "%WIN_WDK%" 0wdf + ) + ) + ) ELSE ( + ECHO Unsupported Python version: "%MAJOR_PYTHON_VERSION%" + EXIT 1 + ) +) + +IF %PYTHON_ARCH% == 64 ( + IF %SET_SDK_64% == Y ( + ECHO Configuring Windows SDK %WINDOWS_SDK_VERSION% for Python %MAJOR_PYTHON_VERSION% on a 64 bit architecture + SET DISTUTILS_USE_SDK=1 + SET MSSdk=1 + "%WIN_SDK_ROOT%\%WINDOWS_SDK_VERSION%\Setup\WindowsSdkVer.exe" -q -version:%WINDOWS_SDK_VERSION% + "%WIN_SDK_ROOT%\%WINDOWS_SDK_VERSION%\Bin\SetEnv.cmd" /x64 /release + ECHO Executing: %COMMAND_TO_RUN% + call %COMMAND_TO_RUN% || EXIT 1 + ) ELSE ( + ECHO Using default MSVC build environment for 64 bit architecture + ECHO Executing: %COMMAND_TO_RUN% + call %COMMAND_TO_RUN% || EXIT 1 + ) +) ELSE ( + ECHO Using default MSVC build environment for 32 bit architecture + ECHO Executing: %COMMAND_TO_RUN% + call %COMMAND_TO_RUN% || EXIT 1 +) diff --git a/run-coveralls.sh b/run-coveralls.sh deleted file mode 100644 index 8a8a08fd..00000000 --- a/run-coveralls.sh +++ /dev/null @@ -1,6 +0,0 @@ -#! /bin/bash - -source $(dirname $0)/run-tests-for-ci.sh - -$PIP install coveralls -coveralls diff --git a/run-tests-for-ci.sh b/run-tests-for-ci.sh index f330de3a..be80ebd9 100644 --- a/run-tests-for-ci.sh +++ b/run-tests-for-ci.sh @@ -72,4 +72,7 @@ cp local_settings.example.py local_settings.py $PIP install coverage coverage run --source=. manage.py test test/ -coverage report -m \ No newline at end of file +coverage report -m + +$PIP install codecov +codecov -X gcov \ No newline at end of file -- GitLab From 5b97054aecaf44a73702fac0f3a6034bf1386785 Mon Sep 17 00:00:00 2001 From: Andreas Kloeckner Date: Tue, 5 Sep 2017 11:27:06 -0500 Subject: [PATCH 20/45] Update tokens/URLs for CI --- .gitlab-ci.yml | 4 ++-- README.rst | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 28ec9192..1d6346b1 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -7,7 +7,7 @@ - tags coverage: '/TOTAL.+ ([0-9]{1,3}%)/' variables: - CODECOV_TOKEN: "a0ed9aa4-e4d9-4b24-bf50-0d54e407bd92" + CODECOV_TOKEN: "895e3bf2-cfd0-45f8-9a14-4b7bd148f76d" Python 3.5: script: @@ -18,7 +18,7 @@ Python 3.5: - tags coverage: '/TOTAL.+ ([0-9]{1,3}%)/' variables: - CODECOV_TOKEN: "a0ed9aa4-e4d9-4b24-bf50-0d54e407bd92" + CODECOV_TOKEN: "895e3bf2-cfd0-45f8-9a14-4b7bd148f76d" Documentation: script: diff --git a/README.rst b/README.rst index 532c9150..72e7c465 100644 --- a/README.rst +++ b/README.rst @@ -1,17 +1,17 @@ +RELATE +====== + +Relate is an Environment for Learning And TEaching + .. image:: https://travis-ci.org/inducer/relate.svg?branch=master :target: https://travis-ci.org/inducer/relate -.. image:: https://ci.appveyor.com/api/projects/status/54c937lry6mjvtx8/branch/master?svg=true +.. image:: https://ci.appveyor.com/api/projects/status/d5bigdw90bxnfdgy?svg=true :target: https://ci.appveyor.com/project/inducer/relate .. image:: https://codecov.io/gh/inducer/relate/branch/master/graph/badge.svg :target: https://codecov.io/gh/inducer/relate/commits -RELATE -====== - -Relate is an Environment for Learning And TEaching - +----------------------------------------------------------------------------------------------+------------------------------------------------------------------------------------------------+ | .. image:: https://raw.githubusercontent.com/inducer/relate/master/doc/images/screenshot.png | .. image:: https://raw.githubusercontent.com/inducer/relate/master/doc/images/screenshot-2.png | +----------------------------------------------------------------------------------------------+------------------------------------------------------------------------------------------------+ -- GitLab From 45f216a3e3a197fa3ed0a9198bc8750ded19d298 Mon Sep 17 00:00:00 2001 From: Andreas Kloeckner Date: Tue, 5 Sep 2017 18:33:25 -0500 Subject: [PATCH 21/45] Revert to 45d8d7a5453df00c755f01d875483269704c5c3c --- .gitlab-ci.yml | 4 -- .travis.yml | 2 +- README.rst | 15 +++---- appveyor.yml | 72 -------------------------------- appveyor/run_with_env.cmd | 88 --------------------------------------- run-coveralls.sh | 6 +++ run-tests-for-ci.sh | 5 +-- 7 files changed, 14 insertions(+), 178 deletions(-) delete mode 100644 appveyor.yml delete mode 100644 appveyor/run_with_env.cmd create mode 100644 run-coveralls.sh diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 1d6346b1..fb58f198 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -6,8 +6,6 @@ except: - tags coverage: '/TOTAL.+ ([0-9]{1,3}%)/' - variables: - CODECOV_TOKEN: "895e3bf2-cfd0-45f8-9a14-4b7bd148f76d" Python 3.5: script: @@ -17,8 +15,6 @@ Python 3.5: except: - tags coverage: '/TOTAL.+ ([0-9]{1,3}%)/' - variables: - CODECOV_TOKEN: "895e3bf2-cfd0-45f8-9a14-4b7bd148f76d" Documentation: script: diff --git a/.travis.yml b/.travis.yml index d03d74ff..63bcd6d8 100644 --- a/.travis.yml +++ b/.travis.yml @@ -2,7 +2,7 @@ language: python cache: pip install: true script: - - bash ./run-tests-for-ci.sh + - bash ./run-coveralls.sh env: - PY_EXE=python2.7 - PY_EXE=python3.5 diff --git a/README.rst b/README.rst index 72e7c465..43eee408 100644 --- a/README.rst +++ b/README.rst @@ -1,16 +1,13 @@ -RELATE -====== - -Relate is an Environment for Learning And TEaching - .. image:: https://travis-ci.org/inducer/relate.svg?branch=master :target: https://travis-ci.org/inducer/relate -.. image:: https://ci.appveyor.com/api/projects/status/d5bigdw90bxnfdgy?svg=true - :target: https://ci.appveyor.com/project/inducer/relate +.. image:: https://coveralls.io/repos/github/inducer/relate/badge.svg?branch=master + :target: https://coveralls.io/github/inducer/relate?branch=master -.. image:: https://codecov.io/gh/inducer/relate/branch/master/graph/badge.svg - :target: https://codecov.io/gh/inducer/relate/commits +RELATE +====== + +Relate is an Environment for Learning And TEaching +----------------------------------------------------------------------------------------------+------------------------------------------------------------------------------------------------+ | .. image:: https://raw.githubusercontent.com/inducer/relate/master/doc/images/screenshot.png | .. image:: https://raw.githubusercontent.com/inducer/relate/master/doc/images/screenshot-2.png | diff --git a/appveyor.yml b/appveyor.yml deleted file mode 100644 index db90d589..00000000 --- a/appveyor.yml +++ /dev/null @@ -1,72 +0,0 @@ -# -# Most of these settings are taken from here: -# https://github.com/ogrisel/python-appveyor-demo/blob/master/appveyor.yml -# - -version: 1.0.{build} - -build: off - -cache: - - '%LOCALAPPDATA%\pip\Cache' - -environment: - global: - # SDK v7.0 MSVC Express 2008's SetEnv.cmd script will fail if the - # /E:ON and /V:ON options are not enabled in the batch script intepreter - # See: http://stackoverflow.com/a/13751649/163740 - CMD_IN_ENV: "cmd /E:ON /V:ON /C .\\appveyor\\run_with_env.cmd" - - matrix: - # Pre-installed Python versions, which Appveyor may upgrade to - # a later point release. - # See: http://www.appveyor.com/docs/installed-software#python - - - PYTHON: "C:\\Python27-x64" - PYTHON_VERSION: "2.7.x" # currently 2.7.13 - PYTHON_ARCH: "64" - - - PYTHON: "C:\\Python35-x64" - PYTHON_VERSION: "3.5.x" # currently 3.5.3 - PYTHON_ARCH: "64" - -init: - - "ECHO %PYTHON%" - - ps: "ls C:/Python*" - -install: - # Prepend newly installed Python to the PATH of this build (this cannot be - # done from inside the powershell script as it would require to restart - # the parent CMD process). - - "SET PATH=%PYTHON%;%PYTHON%\\Scripts;%PATH%" - - - ECHO "Filesystem root:" - - ps: "ls \"C:/\"" - - # Check that we have the expected version and architecture for Python - - "python --version" - - "python -c \"import struct; print(struct.calcsize('P') * 8)\"" - - # Upgrade to the latest version of pip to avoid it displaying warnings - # about it being out of date. - - "pip install --disable-pip-version-check --user --upgrade pip" - - - "git submodule update --init --recursive" - - # Install requirements - - ps: "cat requirements.txt | ?{$_ -notmatch 'dnspython'} > req.txt" - - "%CMD_IN_ENV% pip install -r req.txt" - - ps: "if ($env:PYTHON -like '*Python2*') { pip install dnspython mock} else { pip install dnspython3 }" - - -before_test: - - "%CMD_IN_ENV% pip install coverage" - -test_script: - # Run the project tests - - "copy local_settings.example.py local_settings.py /Y" - - "%CMD_IN_ENV% coverage run manage.py test test/" - -after_test: - - "%CMD_IN_ENV% pip install codecov" - - "%CMD_IN_ENV% codecov -X gcov" diff --git a/appveyor/run_with_env.cmd b/appveyor/run_with_env.cmd deleted file mode 100644 index 5da547c4..00000000 --- a/appveyor/run_with_env.cmd +++ /dev/null @@ -1,88 +0,0 @@ -:: To build extensions for 64 bit Python 3, we need to configure environment -:: variables to use the MSVC 2010 C++ compilers from GRMSDKX_EN_DVD.iso of: -:: MS Windows SDK for Windows 7 and .NET Framework 4 (SDK v7.1) -:: -:: To build extensions for 64 bit Python 2, we need to configure environment -:: variables to use the MSVC 2008 C++ compilers from GRMSDKX_EN_DVD.iso of: -:: MS Windows SDK for Windows 7 and .NET Framework 3.5 (SDK v7.0) -:: -:: 32 bit builds, and 64-bit builds for 3.5 and beyond, do not require specific -:: environment configurations. -:: -:: Note: this script needs to be run with the /E:ON and /V:ON flags for the -:: cmd interpreter, at least for (SDK v7.0) -:: -:: More details at: -:: https://github.com/cython/cython/wiki/64BitCythonExtensionsOnWindows -:: http://stackoverflow.com/a/13751649/163740 -:: -:: Author: Olivier Grisel -:: License: CC0 1.0 Universal: http://creativecommons.org/publicdomain/zero/1.0/ -:: -:: Notes about batch files for Python people: -:: -:: Quotes in values are literally part of the values: -:: SET FOO="bar" -:: FOO is now five characters long: " b a r " -:: If you don't want quotes, don't include them on the right-hand side. -:: -:: The CALL lines at the end of this file look redundant, but if you move them -:: outside of the IF clauses, they do not run properly in the SET_SDK_64==Y -:: case, I don't know why. -@ECHO OFF - -SET COMMAND_TO_RUN=%* -SET WIN_SDK_ROOT=C:\Program Files\Microsoft SDKs\Windows -SET WIN_WDK=c:\Program Files (x86)\Windows Kits\10\Include\wdf - -:: Extract the major and minor versions, and allow for the minor version to be -:: more than 9. This requires the version number to have two dots in it. -SET MAJOR_PYTHON_VERSION=%PYTHON_VERSION:~0,1% -IF "%PYTHON_VERSION:~3,1%" == "." ( - SET MINOR_PYTHON_VERSION=%PYTHON_VERSION:~2,1% -) ELSE ( - SET MINOR_PYTHON_VERSION=%PYTHON_VERSION:~2,2% -) - -:: Based on the Python version, determine what SDK version to use, and whether -:: to set the SDK for 64-bit. -IF %MAJOR_PYTHON_VERSION% == 2 ( - SET WINDOWS_SDK_VERSION="v7.0" - SET SET_SDK_64=Y -) ELSE ( - IF %MAJOR_PYTHON_VERSION% == 3 ( - SET WINDOWS_SDK_VERSION="v7.1" - IF %MINOR_PYTHON_VERSION% LEQ 4 ( - SET SET_SDK_64=Y - ) ELSE ( - SET SET_SDK_64=N - IF EXIST "%WIN_WDK%" ( - :: See: https://connect.microsoft.com/VisualStudio/feedback/details/1610302/ - REN "%WIN_WDK%" 0wdf - ) - ) - ) ELSE ( - ECHO Unsupported Python version: "%MAJOR_PYTHON_VERSION%" - EXIT 1 - ) -) - -IF %PYTHON_ARCH% == 64 ( - IF %SET_SDK_64% == Y ( - ECHO Configuring Windows SDK %WINDOWS_SDK_VERSION% for Python %MAJOR_PYTHON_VERSION% on a 64 bit architecture - SET DISTUTILS_USE_SDK=1 - SET MSSdk=1 - "%WIN_SDK_ROOT%\%WINDOWS_SDK_VERSION%\Setup\WindowsSdkVer.exe" -q -version:%WINDOWS_SDK_VERSION% - "%WIN_SDK_ROOT%\%WINDOWS_SDK_VERSION%\Bin\SetEnv.cmd" /x64 /release - ECHO Executing: %COMMAND_TO_RUN% - call %COMMAND_TO_RUN% || EXIT 1 - ) ELSE ( - ECHO Using default MSVC build environment for 64 bit architecture - ECHO Executing: %COMMAND_TO_RUN% - call %COMMAND_TO_RUN% || EXIT 1 - ) -) ELSE ( - ECHO Using default MSVC build environment for 32 bit architecture - ECHO Executing: %COMMAND_TO_RUN% - call %COMMAND_TO_RUN% || EXIT 1 -) diff --git a/run-coveralls.sh b/run-coveralls.sh new file mode 100644 index 00000000..8a8a08fd --- /dev/null +++ b/run-coveralls.sh @@ -0,0 +1,6 @@ +#! /bin/bash + +source $(dirname $0)/run-tests-for-ci.sh + +$PIP install coveralls +coveralls diff --git a/run-tests-for-ci.sh b/run-tests-for-ci.sh index be80ebd9..f330de3a 100644 --- a/run-tests-for-ci.sh +++ b/run-tests-for-ci.sh @@ -72,7 +72,4 @@ cp local_settings.example.py local_settings.py $PIP install coverage coverage run --source=. manage.py test test/ -coverage report -m - -$PIP install codecov -codecov -X gcov \ No newline at end of file +coverage report -m \ No newline at end of file -- GitLab From 7d68bf21b51387d2d85660496dbd8a09bd54f534 Mon Sep 17 00:00:00 2001 From: Andreas Kloeckner Date: Tue, 5 Sep 2017 18:34:06 -0500 Subject: [PATCH 22/45] Reapply earlier, reverted appveyor changes --- .gitlab-ci.yml | 4 ++++ .travis.yml | 2 +- README.rst | 15 +++++++++------ run-coveralls.sh | 6 ------ run-tests-for-ci.sh | 5 ++++- 5 files changed, 18 insertions(+), 14 deletions(-) delete mode 100644 run-coveralls.sh diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index fb58f198..1d6346b1 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -6,6 +6,8 @@ except: - tags coverage: '/TOTAL.+ ([0-9]{1,3}%)/' + variables: + CODECOV_TOKEN: "895e3bf2-cfd0-45f8-9a14-4b7bd148f76d" Python 3.5: script: @@ -15,6 +17,8 @@ Python 3.5: except: - tags coverage: '/TOTAL.+ ([0-9]{1,3}%)/' + variables: + CODECOV_TOKEN: "895e3bf2-cfd0-45f8-9a14-4b7bd148f76d" Documentation: script: diff --git a/.travis.yml b/.travis.yml index 63bcd6d8..d03d74ff 100644 --- a/.travis.yml +++ b/.travis.yml @@ -2,7 +2,7 @@ language: python cache: pip install: true script: - - bash ./run-coveralls.sh + - bash ./run-tests-for-ci.sh env: - PY_EXE=python2.7 - PY_EXE=python3.5 diff --git a/README.rst b/README.rst index 43eee408..72e7c465 100644 --- a/README.rst +++ b/README.rst @@ -1,14 +1,17 @@ -.. image:: https://travis-ci.org/inducer/relate.svg?branch=master - :target: https://travis-ci.org/inducer/relate - -.. image:: https://coveralls.io/repos/github/inducer/relate/badge.svg?branch=master - :target: https://coveralls.io/github/inducer/relate?branch=master - RELATE ====== Relate is an Environment for Learning And TEaching +.. image:: https://travis-ci.org/inducer/relate.svg?branch=master + :target: https://travis-ci.org/inducer/relate + +.. image:: https://ci.appveyor.com/api/projects/status/d5bigdw90bxnfdgy?svg=true + :target: https://ci.appveyor.com/project/inducer/relate + +.. image:: https://codecov.io/gh/inducer/relate/branch/master/graph/badge.svg + :target: https://codecov.io/gh/inducer/relate/commits + +----------------------------------------------------------------------------------------------+------------------------------------------------------------------------------------------------+ | .. image:: https://raw.githubusercontent.com/inducer/relate/master/doc/images/screenshot.png | .. image:: https://raw.githubusercontent.com/inducer/relate/master/doc/images/screenshot-2.png | +----------------------------------------------------------------------------------------------+------------------------------------------------------------------------------------------------+ diff --git a/run-coveralls.sh b/run-coveralls.sh deleted file mode 100644 index 8a8a08fd..00000000 --- a/run-coveralls.sh +++ /dev/null @@ -1,6 +0,0 @@ -#! /bin/bash - -source $(dirname $0)/run-tests-for-ci.sh - -$PIP install coveralls -coveralls diff --git a/run-tests-for-ci.sh b/run-tests-for-ci.sh index f330de3a..be80ebd9 100644 --- a/run-tests-for-ci.sh +++ b/run-tests-for-ci.sh @@ -72,4 +72,7 @@ cp local_settings.example.py local_settings.py $PIP install coverage coverage run --source=. manage.py test test/ -coverage report -m \ No newline at end of file +coverage report -m + +$PIP install codecov +codecov -X gcov \ No newline at end of file -- GitLab From f0db0e38b99b024c517375235f9908108b412c46 Mon Sep 17 00:00:00 2001 From: Andreas Kloeckner Date: Thu, 7 Sep 2017 16:46:34 -0500 Subject: [PATCH 23/45] Add http.client.RemoteDisconnected to code page nuisance filter (Closes #357 on Github) --- course/page/code.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/course/page/code.py b/course/page/code.py index c7ed5a21..4bacfea3 100644 --- a/course/page/code.py +++ b/course/page/code.py @@ -263,18 +263,18 @@ def is_nuisance_failure(result): return True - if ("traceback" in result - and "bind: address already in use" in result["traceback"]): + if "traceback" in result: + if "bind: address already in use" in result["traceback"]: + # https://github.com/docker/docker/issues/8714 - # https://github.com/docker/docker/issues/8714 + return True - return True + if ("requests.packages.urllib3.exceptions.NewConnectionError" + in result["traceback"]): + return True - if ("traceback" in result - and "requests.packages.urllib3.exceptions.NewConnectionError" - in result["traceback"]): - - return True + if "http.client.RemoteDisconnected" in result["traceback"]: + return True return False -- GitLab From db428e0adc4a53bb93daf0001c3536d53d1473b9 Mon Sep 17 00:00:00 2001 From: dzhuang Date: Tue, 5 Sep 2017 11:18:16 +0800 Subject: [PATCH 24/45] Make sure repo.close() --- course/content.py | 16 ++++---- course/flow.py | 89 ++++++++++++++++++++-------------------- course/models.py | 20 ++++----- course/utils.py | 51 +++++++++++++---------- course/versioning.py | 18 +++----- course/views.py | 17 ++++---- relate/utils.py | 24 +++++++++++ test/base_test_mixins.py | 34 +++++---------- 8 files changed, 141 insertions(+), 128 deletions(-) diff --git a/course/content.py b/course/content.py index 407d1ada..05106070 100644 --- a/course/content.py +++ b/course/content.py @@ -1479,14 +1479,14 @@ def get_course_commit_sha(course, participation): if participation.preview_git_commit_sha: preview_sha = participation.preview_git_commit_sha - repo = get_course_repo(course) - if isinstance(repo, SubdirRepoWrapper): - repo = repo.repo - - try: - repo[preview_sha.encode()] - except KeyError: - preview_sha = None + with get_course_repo(course) as repo: + if isinstance(repo, SubdirRepoWrapper): + repo = repo.repo + + try: + repo[preview_sha.encode()] + except KeyError: + preview_sha = None if preview_sha is not None: sha = preview_sha diff --git a/course/flow.py b/course/flow.py index 9122f446..c0324995 100644 --- a/course/flow.py +++ b/course/flow.py @@ -338,55 +338,54 @@ def grade_page_visit(visit, visit_grade_model=FlowPageVisitGrade, get_flow_page_desc, instantiate_flow_page) - repo = get_course_repo(course) + with get_course_repo(course) as repo: + course_commit_sha = get_course_commit_sha( + course, flow_session.participation if respect_preview else None) - course_commit_sha = get_course_commit_sha( - course, flow_session.participation if respect_preview else None) + flow_desc = get_flow_desc(repo, course, + flow_session.flow_id, course_commit_sha) - flow_desc = get_flow_desc(repo, course, - flow_session.flow_id, course_commit_sha) + page_desc = get_flow_page_desc( + flow_session.flow_id, + flow_desc, + page_data.group_id, page_data.page_id) - page_desc = get_flow_page_desc( - flow_session.flow_id, - flow_desc, - page_data.group_id, page_data.page_id) + page = instantiate_flow_page( + location="flow '%s', group, '%s', page '%s'" + % (flow_session.flow_id, page_data.group_id, page_data.page_id), + repo=repo, page_desc=page_desc, + commit_sha=course_commit_sha) - page = instantiate_flow_page( - location="flow '%s', group, '%s', page '%s'" - % (flow_session.flow_id, page_data.group_id, page_data.page_id), - repo=repo, page_desc=page_desc, - commit_sha=course_commit_sha) - - assert page.expects_answer() - if not page.is_answer_gradable(): - return - - from course.page import PageContext - grading_page_context = PageContext( - course=course, - repo=repo, - commit_sha=course_commit_sha, - flow_session=flow_session) - - with translation.override(settings.RELATE_ADMIN_EMAIL_LOCALE): - answer_feedback = page.grade( - grading_page_context, visit.page_data.data, - visit.answer, grade_data=grade_data) - - grade = visit_grade_model() - grade.visit = visit - grade.grade_data = grade_data - grade.max_points = page.max_points(visit.page_data) - grade.graded_at_git_commit_sha = course_commit_sha.decode() - - bulk_feedback_json = None - if answer_feedback is not None: - grade.correctness = answer_feedback.correctness - grade.feedback, bulk_feedback_json = answer_feedback.as_json() - - grade.save() - - update_bulk_feedback(page_data, grade, bulk_feedback_json) + assert page.expects_answer() + if not page.is_answer_gradable(): + return + + from course.page import PageContext + grading_page_context = PageContext( + course=course, + repo=repo, + commit_sha=course_commit_sha, + flow_session=flow_session) + + with translation.override(settings.RELATE_ADMIN_EMAIL_LOCALE): + answer_feedback = page.grade( + grading_page_context, visit.page_data.data, + visit.answer, grade_data=grade_data) + + grade = visit_grade_model() + grade.visit = visit + grade.grade_data = grade_data + grade.max_points = page.max_points(visit.page_data) + grade.graded_at_git_commit_sha = course_commit_sha.decode() + + bulk_feedback_json = None + if answer_feedback is not None: + grade.correctness = answer_feedback.correctness + grade.feedback, bulk_feedback_json = answer_feedback.as_json() + + grade.save() + + update_bulk_feedback(page_data, grade, bulk_feedback_json) # }}} diff --git a/course/models.py b/course/models.py index 6a0f84c3..3bc1d4ff 100644 --- a/course/models.py +++ b/course/models.py @@ -1280,16 +1280,16 @@ class FlowRuleException(models.Model): from relate.utils import dict_to_struct rule = dict_to_struct(self.rule) - repo = get_course_repo(self.participation.course) - commit_sha = get_course_commit_sha( - self.participation.course, self.participation) - ctx = ValidationContext( - repo=repo, - commit_sha=commit_sha) - - flow_desc = get_flow_desc(repo, - self.participation.course, - self.flow_id, commit_sha) + with get_course_repo(self.participation.course) as repo: + commit_sha = get_course_commit_sha( + self.participation.course, self.participation) + ctx = ValidationContext( + repo=repo, + commit_sha=commit_sha) + + flow_desc = get_flow_desc(repo, + self.participation.course, + self.flow_id, commit_sha) tags = None grade_identifier = None diff --git a/course/utils.py b/course/utils.py index 2ff1f3f5..f6c172a3 100644 --- a/course/utils.py +++ b/course/utils.py @@ -577,24 +577,25 @@ class CoursePageContext(object): if self.participation.preview_git_commit_sha: preview_sha = self.participation.preview_git_commit_sha.encode() - repo = get_course_repo(self.course) - - from relate.utils import SubdirRepoWrapper - if isinstance(repo, SubdirRepoWrapper): - true_repo = repo.repo - else: - true_repo = cast(dulwich.repo.Repo, repo) - - try: - true_repo[preview_sha] - except KeyError: - from django.contrib import messages - messages.add_message(request, messages.ERROR, - _("Preview revision '%s' does not exist--" - "showing active course content instead.") - % preview_sha.decode()) - - preview_sha = None + with get_course_repo(self.course) as repo: + from relate.utils import SubdirRepoWrapper + if isinstance(repo, SubdirRepoWrapper): + true_repo = repo.repo + else: + true_repo = cast(dulwich.repo.Repo, repo) + + try: + true_repo[preview_sha] + except KeyError: + from django.contrib import messages + messages.add_message(request, messages.ERROR, + _("Preview revision '%s' does not exist--" + "showing active course content instead.") + % preview_sha.decode()) + + preview_sha = None + finally: + true_repo.close() if preview_sha is not None: sha = preview_sha @@ -634,6 +635,12 @@ class CoursePageContext(object): else: return (perm, argument) in self.permissions() + def __enter__(self): + return self + + def __exit__(self, exc_type, exc_val, exc_tb): + self.repo.close() + class FlowContext(object): def __init__(self, repo, course, flow_id, participation=None): @@ -752,10 +759,10 @@ def instantiate_flow_page_with_ctx(fctx, page_data): # {{{ utilties for course-based views def course_view(f): def wrapper(request, course_identifier, *args, **kwargs): - pctx = CoursePageContext(request, course_identifier) - response = f(pctx, *args, **kwargs) - pctx.repo.close() - return response + with CoursePageContext(request, course_identifier) as pctx: + response = f(pctx, *args, **kwargs) + pctx.repo.close() + return response from functools import update_wrapper update_wrapper(wrapper, f) diff --git a/course/versioning.py b/course/versioning.py index 68eac1ba..2c9c6ca7 100644 --- a/course/versioning.py +++ b/course/versioning.py @@ -281,24 +281,14 @@ def set_up_new_course(request): # Don't coalesce this handler with the one below. We only want # to delete the directory if we created it. Trust me. - # Work around read-only files on Windows. - # https://docs.python.org/3.5/library/shutil.html#rmtree-example - - import os - import stat - import shutil - # Make sure files opened for 'repo' above are actually closed. if repo is not None: # noqa repo.close() # noqa - def remove_readonly(func, path, _): # noqa - "Clear the readonly bit and reattempt the removal" - os.chmod(path, stat.S_IWRITE) - func(path) + from relate.utils import force_remove_path try: - shutil.rmtree(repo_path, onerror=remove_readonly) + force_remove_path(repo_path) except OSError: messages.add_message(request, messages.WARNING, ugettext("Failed to delete unused " @@ -307,6 +297,10 @@ def set_up_new_course(request): raise + finally: + if repo is not None: + repo.close() + except Exception as e: from traceback import print_exc print_exc() diff --git a/course/views.py b/course/views.py index 5128d3e2..bf9c9334 100644 --- a/course/views.py +++ b/course/views.py @@ -252,8 +252,9 @@ def media_etag_func(request, course_identifier, commit_sha, media_path): def get_media(request, course_identifier, commit_sha, media_path): course = get_object_or_404(Course, identifier=course_identifier) - repo = get_course_repo(course) - return get_repo_file_response(repo, "media/" + media_path, commit_sha.encode()) + with get_course_repo(course) as repo: + return get_repo_file_response( + repo, "media/" + media_path, commit_sha.encode()) def repo_file_etag_func(request, course_identifier, commit_sha, path): @@ -320,9 +321,6 @@ def get_repo_file_backend( # check to see if the course is hidden check_course_state(course, participation) - # retrieve local path for the repo for the course - repo = get_course_repo(course) - # set access to public (or unenrolled), student, etc if request.relate_exam_lockdown: access_kinds = ["in_exam"] @@ -335,10 +333,13 @@ def get_repo_file_backend( and arg is not None] from course.content import is_repo_file_accessible_as - if not is_repo_file_accessible_as(access_kinds, repo, commit_sha, path): - raise PermissionDenied() - return get_repo_file_response(repo, path, commit_sha) + # retrieve local path for the repo for the course + with get_course_repo(course) as repo: + if not is_repo_file_accessible_as(access_kinds, repo, commit_sha, path): + raise PermissionDenied() + + return get_repo_file_response(repo, path, commit_sha) def get_repo_file_response(repo, path, commit_sha): diff --git a/relate/utils.py b/relate/utils.py index 7dbae3f4..3a06b272 100644 --- a/relate/utils.py +++ b/relate/utils.py @@ -115,6 +115,12 @@ class SubdirRepoWrapper(object): def close(self): self.repo.close() + def __enter__(self): + return self + + def __exit__(self, exc_type, exc_val, exc_tb): + self.close() + Repo_ish = Union[dulwich.repo.Repo, SubdirRepoWrapper] @@ -425,4 +431,22 @@ def ignore_no_such_table(f, *args): raise +def force_remove_path(path): + # type: (Text) -> None + """ + Work around deleting read-only path on Windows. + Ref: https://docs.python.org/3.5/library/shutil.html#rmtree-example + """ + import os + import stat + import shutil + + def remove_readonly(func, path, _): # noqa + "Clear the readonly bit and reattempt the removal" + os.chmod(path, stat.S_IWRITE) + func(path) + + shutil.rmtree(path, onerror=remove_readonly) + + # vim: foldmethod=marker diff --git a/test/base_test_mixins.py b/test/base_test_mixins.py index 5d7a01cc..a9932c7e 100644 --- a/test/base_test_mixins.py +++ b/test/base_test_mixins.py @@ -27,6 +27,7 @@ from django.conf import settings from django.test import Client from django.urls import reverse from django.contrib.auth import get_user_model +from relate.utils import force_remove_path from course.models import Course, Participation, ParticipationRole from course.constants import participation_status @@ -90,25 +91,6 @@ SINGLE_COURSE_SETUP_LIST = [ ] -def force_remove_path(path): - # shutil.rmtree won't work when delete course repo folder, on Windows, - # so it cause all testcases failed. - # Though this work around (copied from http://bit.ly/2usqGxr) still fails - # for some tests, this enables **some other** tests on Windows. - import stat - def remove_readonly(func, path, _): # noqa - os.chmod(path, stat.S_IWRITE) - func(path) - - import shutil - try: - shutil.rmtree(path, onerror=remove_readonly) - except OSError: - # let the remove_exceptionally_undelete_course_repos method to delete - # the folder for the next test. - pass - - class SuperuserCreateMixin(object): create_superuser_kwargs = CREATE_SUPERUSER_KWARGS @@ -178,12 +160,18 @@ class CoursesTestMixinBase(SuperuserCreateMixin): @classmethod def remove_exceptionally_undelete_course_repos(cls, course_identifier): - # Remove undelete course repo folders coursed by - # unexpected exceptions in previous tests. + """ + Remove existing course repo folders resulted in unexpected + exceptions in previous tests. + """ + repo_path = os.path.join(settings.GIT_ROOT, course_identifier) try: - force_remove_path(os.path.join(settings.GIT_ROOT, course_identifier)) + force_remove_path(repo_path) except OSError: - pass + if not os.path.isdir(repo_path): + # The repo path does not exist, that's good! + return + raise @classmethod def remove_course_repo(cls, course): -- GitLab From af46e5f222896305a10a366fe783001dcc0073ab Mon Sep 17 00:00:00 2001 From: Andreas Kloeckner Date: Thu, 7 Sep 2017 22:22:38 -0500 Subject: [PATCH 25/45] Define interaction between use_last_activity_as_completion_time and if_completed_before (See also #358 on Github) --- course/utils.py | 18 +++++++++++++++--- doc/flow.rst | 6 ++++++ 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/course/utils.py b/course/utils.py index 2ff1f3f5..acb1bc7a 100644 --- a/course/utils.py +++ b/course/utils.py @@ -493,9 +493,21 @@ def get_session_grading_rule( if hasattr(rule, "if_completed_before"): ds = parse_date_spec(session.course, rule.if_completed_before) - if session.in_progress and now_datetime > ds: - continue - if not session.in_progress and session.completion_time > ds: + + use_last_activity_as_completion_time = False + if hasattr(rule, "use_last_activity_as_completion_time"): + use_last_activity_as_completion_time = \ + rule.use_last_activity_as_completion_time + + if use_last_activity_as_completion_time: + completion_time = session.last_activity() + else: + if session.in_progress: + completion_time = now_datetime + else: + completion_time = session.completion_time + + if completion_time > ds: continue due = parse_date_spec(session.course, getattr(rule, "due", None)) diff --git a/doc/flow.rst b/doc/flow.rst index b1cf6ba3..a9886a90 100644 --- a/doc/flow.rst +++ b/doc/flow.rst @@ -510,6 +510,12 @@ Determining how final (overall) grades of flows are computed (Optional) A :ref:`datespec `. Rule applies if the session was completed before this time. + When evaluating this condition for in-progress sessions, the current time, + or, if :attr:`use_last_activity_as_completion_time` is set, the time of the + last activity is used. + + Since September 2017, this respects :attr:`use_last_activity_as_completion_time`. + .. rubric:: Rules specified .. attribute:: credit_percent -- GitLab From 437eccc7fd44cdcbca4a91e653761e8d5f4f4e19 Mon Sep 17 00:00:00 2001 From: Andreas Kloeckner Date: Thu, 7 Sep 2017 22:42:17 -0500 Subject: [PATCH 26/45] Stop using mypy option --fast-parser --- run-mypy.sh | 1 - 1 file changed, 1 deletion(-) diff --git a/run-mypy.sh b/run-mypy.sh index 6896ff9a..e867afda 100755 --- a/run-mypy.sh +++ b/run-mypy.sh @@ -1,7 +1,6 @@ #! /bin/bash mypy \ - --fast-parser \ --strict-optional \ --ignore-missing-imports \ --follow-imports=skip \ -- GitLab From 6c58c6b05188fa821f9666b2394d0f8f177f5fa6 Mon Sep 17 00:00:00 2001 From: Andreas Kloeckner Date: Thu, 7 Sep 2017 22:42:47 -0500 Subject: [PATCH 27/45] Fix type annotations: frozenset[type] -> FrozenSet[type] --- course/constants.py | 4 +++- course/content.py | 6 +++--- course/enrollment.py | 4 ++-- course/flow.py | 14 +++++++------- course/page/base.py | 4 ++-- course/utils.py | 12 ++++++------ 6 files changed, 23 insertions(+), 21 deletions(-) diff --git a/course/constants.py b/course/constants.py index 7c50b9f1..f418746b 100644 --- a/course/constants.py +++ b/course/constants.py @@ -24,6 +24,8 @@ OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. """ +if False: + import typing # noqa from django.utils.translation import pgettext_lazy, ugettext # Allow 10x extra credit at the very most. @@ -286,7 +288,7 @@ FLOW_SESSION_EXPIRATION_MODE_CHOICES = ( def is_expiration_mode_allowed(expmode, permissions): - # type: (str, frozenset[str]) -> bool + # type: (str, typing.FrozenSet[str]) -> bool if expmode == flow_session_expiration_mode.roll_over: if (flow_permission.set_roll_over_expiration_mode in permissions): diff --git a/course/content.py b/course/content.py index 407d1ada..9d320f6c 100644 --- a/course/content.py +++ b/course/content.py @@ -24,7 +24,7 @@ OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. """ -from typing import cast, Union +from typing import cast, Union, FrozenSet from django.conf import settings from django.utils.translation import ugettext as _ @@ -1215,7 +1215,7 @@ def compute_chunk_weight_and_shown( chunk, # type: ChunkDesc roles, # type: List[Text] now_datetime, # type: datetime.datetime - facilities, # type: frozenset[Text] + facilities, # type: FrozenSet[Text] ): # type: (...) -> Tuple[float, bool] if not hasattr(chunk, "rules"): @@ -1274,7 +1274,7 @@ def get_processed_page_chunks( page_desc, # type: StaticPageDesc roles, # type: List[Text] now_datetime, # type: datetime.datetime - facilities, # type: frozenset[Text] + facilities, # type: FrozenSet[Text] ): # type: (...) -> List[ChunkDesc] for chunk in page_desc.chunks: diff --git a/course/enrollment.py b/course/enrollment.py index abad3046..cd8ee4d3 100644 --- a/course/enrollment.py +++ b/course/enrollment.py @@ -71,7 +71,7 @@ from pytools.lex import RE as REBase # noqa # {{{ for mypy if False: - from typing import Any, Tuple, Text, Optional, List # noqa + from typing import Any, Tuple, Text, Optional, List, FrozenSet # noqa from course.utils import CoursePageContext # noqa # }}} @@ -137,7 +137,7 @@ def get_participation_permissions( course, # type: Course participation, # type: Optional[Participation] ): - # type: (...) -> frozenset[Tuple[Text, Optional[Text]]] + # type: (...) -> FrozenSet[Tuple[Text, Optional[Text]]] if participation is not None: return participation.permissions() diff --git a/course/flow.py b/course/flow.py index 9122f446..bdac6e17 100644 --- a/course/flow.py +++ b/course/flow.py @@ -88,7 +88,7 @@ from relate.utils import retry_transaction_decorator # {{{ mypy if False: - from typing import Any, Optional, Iterable, Tuple, Text, List # noqa + from typing import Any, Optional, Iterable, Tuple, Text, List, FrozenSet # noqa import datetime # noqa from course.models import Course # noqa from course.utils import ( # noqa @@ -1347,7 +1347,7 @@ def recalculate_session_grade(repo, course, session): def lock_down_if_needed( request, # type: http.HttpRequest - permissions, # type: frozenset[Text] + permissions, # type: FrozenSet[Text] flow_session, # type: FlowSession ): # type: (...) -> None @@ -1607,7 +1607,7 @@ def get_and_check_flow_session(pctx, flow_session_id): def will_receive_feedback(permissions): - # type: (frozenset[Text]) -> bool + # type: (FrozenSet[Text]) -> bool return ( flow_permission.see_correctness in permissions @@ -1615,14 +1615,14 @@ def will_receive_feedback(permissions): def may_send_email_about_flow_page(permissions): - # type: (frozenset[Text]) -> bool + # type: (FrozenSet[Text]) -> bool return flow_permission.send_email_about_flow_page in permissions def get_page_behavior( page, # type: PageBase - permissions, # type: frozenset[Text] + permissions, # type: FrozenSet[Text] session_in_progress, # type: bool answer_was_graded, # type: bool generates_grade, # type: bool @@ -1679,7 +1679,7 @@ def get_page_behavior( def add_buttons_to_form(form, fpctx, flow_session, permissions): - # type: (StyledForm, FlowPageContext, FlowSession, frozenset[Text]) -> StyledForm + # type: (StyledForm, FlowPageContext, FlowSession, FrozenSet[Text]) -> StyledForm from crispy_forms.layout import Submit show_save_button = getattr(form, "show_save_button", True) @@ -2137,7 +2137,7 @@ def post_flow_page( flow_session, # type: FlowSession fpctx, # type: FlowPageContext request, # type: http.HttpRequest - permissions, # type: frozenset[Text] + permissions, # type: FrozenSet[Text] generates_grade, # type: bool ): # type: (...) -> Tuple[PageBehavior, List[FlowPageVisit], forms.Form, Optional[AnswerFeedback], Any, bool] # noqa diff --git a/course/page/base.py b/course/page/base.py index 4bc5c7eb..2a5c3b6e 100644 --- a/course/page/base.py +++ b/course/page/base.py @@ -44,7 +44,7 @@ from django.conf import settings # {{{ mypy if False: - from typing import Text, Optional, Any, Tuple, Dict, Callable # noqa + from typing import Text, Optional, Any, Tuple, Dict, Callable, FrozenSet # noqa from django import http # noqa from course.models import ( # noqa Course, @@ -371,7 +371,7 @@ class PageBase(object): ) def get_modified_permissions_for_page(self, permissions): - # type: (frozenset[Text]) -> frozenset[Text] + # type: (FrozenSet[Text]) -> FrozenSet[Text] rw_permissions = set(permissions) if hasattr(self.page_desc, "access_rules"): diff --git a/course/utils.py b/course/utils.py index 2ff1f3f5..2be2311b 100644 --- a/course/utils.py +++ b/course/utils.py @@ -58,7 +58,7 @@ from course.page.base import ( # noqa # {{{ mypy if False: - from typing import Tuple, List, Text, Iterable, Any, Optional, Union, Dict # noqa + from typing import Tuple, List, Text, Iterable, Any, Optional, Union, Dict, FrozenSet # noqa from relate.utils import Repo_ish # noqa from course.models import ( # noqa Course, @@ -105,7 +105,7 @@ class FlowSessionStartRule(FlowSessionRuleBase): class FlowSessionAccessRule(FlowSessionRuleBase): def __init__( self, - permissions, # type: frozenset[Text] + permissions, # type: FrozenSet[Text] message=None, # type: Optional[Text] ): # type: (...) -> None @@ -280,7 +280,7 @@ def get_session_start_rule( flow_id, # type: Text flow_desc, # type: FlowDesc now_datetime, # type: datetime.datetime - facilities=None, # type: Optional[frozenset[Text]] + facilities=None, # type: Optional[FrozenSet[Text]] for_rollover=False, # type: bool login_exam_ticket=None, # type: Optional[ExamTicket] ): @@ -373,7 +373,7 @@ def get_session_access_rule( session, # type: FlowSession flow_desc, # type: FlowDesc now_datetime, # type: datetime.datetime - facilities=None, # type: Optional[frozenset[Text]] + facilities=None, # type: Optional[FrozenSet[Text]] login_exam_ticket=None, # type: Optional[ExamTicket] ): # type: (...) -> FlowSessionAccessRule @@ -552,7 +552,7 @@ class CoursePageContext(object): self.request = request self.course_identifier = course_identifier - self._permissions_cache = None # type: Optional[frozenset[Tuple[Text, Optional[Text]]]] # noqa + self._permissions_cache = None # type: Optional[FrozenSet[Tuple[Text, Optional[Text]]]] # noqa self._role_identifiers_cache = None # type: Optional[List[Text]] from course.models import Course # noqa @@ -612,7 +612,7 @@ class CoursePageContext(object): return self._role_identifiers_cache def permissions(self): - # type: () -> frozenset[Tuple[Text, Optional[Text]]] + # type: () -> FrozenSet[Tuple[Text, Optional[Text]]] if self.participation is None: if self._permissions_cache is not None: return self._permissions_cache -- GitLab From 4b4103130db8fc121e6fa421364785fc2cfd67e9 Mon Sep 17 00:00:00 2001 From: Andreas Kloeckner Date: Thu, 7 Sep 2017 22:45:13 -0500 Subject: [PATCH 28/45] Add missing typing.List import --- course/grading.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/course/grading.py b/course/grading.py index 910edc9a..b20b0154 100644 --- a/course/grading.py +++ b/course/grading.py @@ -58,7 +58,7 @@ from course.constants import ( # {{{ for mypy if False: - from typing import Text, Any, Optional, Dict # noqa + from typing import Text, Any, Optional, Dict, List # noqa from course.models import ( # noqa GradingOpportunity) from course.utils import ( # noqa -- GitLab From 10125d0d131afc23dba89cd345ecf59d02d391eb Mon Sep 17 00:00:00 2001 From: Andreas Kloeckner Date: Thu, 7 Sep 2017 22:59:01 -0500 Subject: [PATCH 29/45] Fix Flake8 complaint about type import --- course/content.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/course/content.py b/course/content.py index 9d320f6c..fc276860 100644 --- a/course/content.py +++ b/course/content.py @@ -24,7 +24,7 @@ OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. """ -from typing import cast, Union, FrozenSet +from typing import cast, Union from django.conf import settings from django.utils.translation import ugettext as _ @@ -62,7 +62,7 @@ else: if False: # for mypy from typing import ( # noqa - Any, List, Tuple, Optional, Callable, Text, Dict) + Any, List, Tuple, Optional, Callable, Text, Dict, FrozenSet) from course.models import Course, Participation # noqa import dulwich # noqa from course.validation import ValidationContext # noqa -- GitLab From 43174f39956bb186f2e9f4910c3c797061eef69d Mon Sep 17 00:00:00 2001 From: Andreas Kloeckner Date: Thu, 7 Sep 2017 22:59:22 -0500 Subject: [PATCH 30/45] Update call to prepare-and-run-mypy.sh to include desired mypy version --- .gitlab-ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index fb58f198..22d961db 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -30,9 +30,9 @@ Mypy: script: - curl -L -O -k https://gitlab.tiker.net/inducer/ci-support/raw/master/prepare-and-run-mypy.sh - "cp local_settings.example.py local_settings.py" - - ". ./prepare-and-run-mypy.sh" + - ". ./prepare-and-run-mypy.sh python3.6 mypy==0.521 typed-ast==1.0.4" tags: - - python3.5 + - python3.6 except: - tags -- GitLab From 1bb0cae4c6b74630d7b6b4bc18a694b48d6969f8 Mon Sep 17 00:00:00 2001 From: Andreas Kloeckner Date: Thu, 7 Sep 2017 22:59:29 -0500 Subject: [PATCH 31/45] Add type annotation for permissions functions --- course/models.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/course/models.py b/course/models.py index 6a0f84c3..45fa1d67 100644 --- a/course/models.py +++ b/course/models.py @@ -61,7 +61,7 @@ from course.page.base import AnswerFeedback # {{{ mypy if False: - from typing import List, Dict, Any, Optional, Text, Iterable # noqa # noqa + from typing import List, Dict, Any, Optional, Text, Iterable, Tuple, FrozenSet # noqa from course.content import FlowDesc # noqa # }}} @@ -468,7 +468,10 @@ class Participation(models.Model): # {{{ permissions handling + _permissions_cache = None # type: FrozenSet[Tuple[Text, Optional[Text]]] + def permissions(self): + # type: () -> FrozenSet[Tuple[Text, Optional[Text]]] try: return self._permissions_cache except AttributeError: @@ -486,14 +489,15 @@ class Participation(models.Model): participation=self) .values_list("permission", "argument"))) - perm = frozenset( + fset_perm = frozenset( (permission, argument) if argument else (permission, None) for permission, argument in perm) - self._permissions_cache = perm - return perm + self._permissions_cache = fset_perm + return fset_perm def has_permission(self, perm, argument=None): + # type: (Text, Optional[Text]) -> bool return (perm, argument) in self.permissions() # }}} -- GitLab From 236f20b5eaff1f4e47eb61e7ac4d20443e74183c Mon Sep 17 00:00:00 2001 From: Andreas Kloeckner Date: Thu, 7 Sep 2017 23:04:42 -0500 Subject: [PATCH 32/45] Fix typing of get_repo_blob_data_cached --- course/content.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/course/content.py b/course/content.py index fc276860..6e798c38 100644 --- a/course/content.py +++ b/course/content.py @@ -312,7 +312,7 @@ def get_repo_blob_data_cached(repo, full_name, commit_sha): def_cache = cache.caches["default"] - result = None + result = None # type: Optional[bytes] # Memcache is apparently limited to 250 characters. if len(cache_key) < 240: result = def_cache.get(cache_key) @@ -323,6 +323,7 @@ def get_repo_blob_data_cached(repo, full_name, commit_sha): result = get_repo_blob(repo, full_name, commit_sha, allow_tree=False).data + assert result is not None if len(result) <= getattr(settings, "RELATE_CACHE_MAX_BYTES", 0): def_cache.add(cache_key, (result,), None) -- GitLab From fad7c4f9095c81100fecdbdc418580e9ccd8485b Mon Sep 17 00:00:00 2001 From: Andreas Kloeckner Date: Thu, 7 Sep 2017 23:06:29 -0500 Subject: [PATCH 33/45] Add type to UnsubmitFlowPageForm constructor --- course/flow.py | 1 + 1 file changed, 1 insertion(+) diff --git a/course/flow.py b/course/flow.py index bdac6e17..8c680a67 100644 --- a/course/flow.py +++ b/course/flow.py @@ -2832,6 +2832,7 @@ def regrade_flows_view(pctx): class UnsubmitFlowPageForm(forms.Form): def __init__(self, *args, **kwargs): + # type: (*Any, **Any) -> None self.helper = FormHelper() super(UnsubmitFlowPageForm, self).__init__(*args, **kwargs) -- GitLab From 6007347581c4b8d58ba1eb1ae086291e333afd25 Mon Sep 17 00:00:00 2001 From: Andreas Kloeckner Date: Thu, 7 Sep 2017 23:09:34 -0500 Subject: [PATCH 34/45] Re-Fix typing of get_repo_blob_data_cached --- course/content.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/course/content.py b/course/content.py index 6e798c38..a011cd5a 100644 --- a/course/content.py +++ b/course/content.py @@ -300,6 +300,7 @@ def get_repo_blob_data_cached(repo, full_name, commit_sha): except ImproperlyConfigured: cache_key = None + result = None # type: Optional[bytes] if cache_key is None: result = get_repo_blob(repo, full_name, commit_sha, allow_tree=False).data @@ -312,14 +313,14 @@ def get_repo_blob_data_cached(repo, full_name, commit_sha): def_cache = cache.caches["default"] - result = None # type: Optional[bytes] # Memcache is apparently limited to 250 characters. if len(cache_key) < 240: - result = def_cache.get(cache_key) - if result is not None: - (result,) = result - assert isinstance(result, six.binary_type), cache_key - return result + cached_result = def_cache.get(cache_key) + + if cached_result is not None: + (result,) = cached_result + assert isinstance(result, six.binary_type), cache_key + return result result = get_repo_blob(repo, full_name, commit_sha, allow_tree=False).data -- GitLab From 18cd7a4d19154fb374af5ea1271df892ce03f1b8 Mon Sep 17 00:00:00 2001 From: Andreas Kloeckner Date: Thu, 7 Sep 2017 23:39:24 -0500 Subject: [PATCH 35/45] Get mypy-clean on mypy 0.521 --- course/content.py | 13 ++++++++----- course/enrollment.py | 2 ++ course/exam.py | 2 +- course/flow.py | 3 ++- course/grading.py | 5 ++--- course/models.py | 2 +- course/page/base.py | 4 +++- course/sandbox.py | 9 +++++++-- course/utils.py | 10 +++++----- course/validation.py | 3 ++- course/views.py | 9 ++++++--- 11 files changed, 39 insertions(+), 23 deletions(-) diff --git a/course/content.py b/course/content.py index a011cd5a..294d871d 100644 --- a/course/content.py +++ b/course/content.py @@ -563,18 +563,20 @@ def get_raw_yaml_from_repo(repo, full_name, commit_sha): import django.core.cache as cache def_cache = cache.caches["default"] - result = None + + result = None # type: Optional[Any] # Memcache is apparently limited to 250 characters. if len(cache_key) < 240: result = def_cache.get(cache_key) if result is not None: return result - result = load_yaml( - expand_yaml_macros( + yaml_str = expand_yaml_macros( repo, commit_sha, get_repo_blob(repo, full_name, commit_sha, - allow_tree=False).data)) + allow_tree=False).data) + + result = load_yaml(yaml_str) # type: ignore def_cache.add(cache_key, result, None) @@ -621,7 +623,8 @@ def get_yaml_from_repo(repo, full_name, commit_sha, cached=True): expanded = expand_yaml_macros( repo, commit_sha, yaml_bytestream) - result = dict_to_struct(load_yaml(expanded)) + yaml_data = load_yaml(expanded) # type:ignore + result = dict_to_struct(yaml_data) if cached: def_cache.add(cache_key, result, None) diff --git a/course/enrollment.py b/course/enrollment.py index cd8ee4d3..cfbc7802 100644 --- a/course/enrollment.py +++ b/course/enrollment.py @@ -233,6 +233,8 @@ def enroll_view(request, course_identifier): course, user, participation_status.requested, roles, request) + assert participation is not None + with translation.override(settings.RELATE_ADMIN_EMAIL_LOCALE): from django.template.loader import render_to_string message = render_to_string("course/enrollment-request-email.txt", { diff --git a/course/exam.py b/course/exam.py index 6cd666c7..ee6d5391 100644 --- a/course/exam.py +++ b/course/exam.py @@ -615,7 +615,7 @@ def is_from_exams_only_facility(request): def get_login_exam_ticket(request): - # type: (http.HttpRequest) -> ExamTicket + # type: (http.HttpRequest) -> Optional[ExamTicket] exam_ticket_pk = request.session.get("relate_exam_ticket_pk_used_for_login") if exam_ticket_pk is None: diff --git a/course/flow.py b/course/flow.py index 8c680a67..23e178e6 100644 --- a/course/flow.py +++ b/course/flow.py @@ -88,7 +88,7 @@ from relate.utils import retry_transaction_decorator # {{{ mypy if False: - from typing import Any, Optional, Iterable, Tuple, Text, List, FrozenSet # noqa + from typing import Any, Optional, Iterable, Sequence, Tuple, Text, List, FrozenSet # noqa import datetime # noqa from course.models import Course # noqa from course.utils import ( # noqa @@ -494,6 +494,7 @@ def get_prev_answer_visits_qset(page_data): def get_first_from_qset(qset): + # type: (Sequence) -> Optional[Any] for item in qset[:1]: return item diff --git a/course/grading.py b/course/grading.py index b20b0154..5691309d 100644 --- a/course/grading.py +++ b/course/grading.py @@ -320,6 +320,8 @@ def grade_flow_page(pctx, flow_session_id, page_ordinal): else: grading_form = None + grading_form_html = None # type: Optional[Text] + if grading_form is not None: from crispy_forms.layout import Submit grading_form.helper.form_class += " relate-grading-form" @@ -332,9 +334,6 @@ def grade_flow_page(pctx, flow_session_id, page_ordinal): grading_form_html = fpctx.page.grading_form_to_html( pctx.request, fpctx.page_context, grading_form, grade_data) - else: - grading_form_html = None - # }}} # {{{ compute points_awarded diff --git a/course/models.py b/course/models.py index 45fa1d67..aa3f9306 100644 --- a/course/models.py +++ b/course/models.py @@ -797,7 +797,7 @@ class FlowSession(models.Model): __str__ = __unicode__ def append_comment(self, s): - # type: (Text) -> None + # type: (Optional[Text]) -> None if s is None: return diff --git a/course/page/base.py b/course/page/base.py index 2a5c3b6e..bfb3c81c 100644 --- a/course/page/base.py +++ b/course/page/base.py @@ -216,7 +216,7 @@ class AnswerFeedback(object): @staticmethod def from_json(json, bulk_json): - # type: (Any, Any) -> AnswerFeedback + # type: (Any, Any) -> Optional[AnswerFeedback] if json is None: return json @@ -387,9 +387,11 @@ class PageBase(object): return frozenset(rw_permissions) def make_page_data(self): + # type: () -> Dict return {} def initialize_page_data(self, page_context): + # type: (PageContext) -> Dict """Return (possibly randomly generated) data that is used to generate the content on this page. This is passed to methods below as the *page_data* argument. One possible use for this argument would be a random permutation diff --git a/course/sandbox.py b/course/sandbox.py index 3ffd16b6..23258395 100644 --- a/course/sandbox.py +++ b/course/sandbox.py @@ -240,7 +240,9 @@ def view_page_sandbox(pctx): from course.content import expand_yaml_macros new_page_source = expand_yaml_macros( pctx.repo, pctx.course_commit_sha, new_page_source) - page_desc = dict_to_struct(yaml.load(new_page_source)) + + yaml_data = yaml.load(new_page_source) # type: ignore + page_desc = dict_to_struct(yaml_data) if not isinstance(page_desc, Struct): raise ValidationError("Provided page source code is not " @@ -295,7 +297,8 @@ def view_page_sandbox(pctx): have_valid_page = page_source is not None if have_valid_page: - page_desc = cast(FlowPageDesc, dict_to_struct(yaml.load(page_source))) + yaml_data = yaml.load(page_source) # type: ignore + page_desc = cast(FlowPageDesc, dict_to_struct(yaml_data)) from course.content import instantiate_flow_page try: @@ -313,6 +316,8 @@ def view_page_sandbox(pctx): have_valid_page = False if have_valid_page: + page_desc = cast(FlowPageDesc, page_desc) + # Try to recover page_data, answer_data page_data = get_sandbox_data_for_page( pctx, page_desc, PAGE_DATA_SESSION_KEY) diff --git a/course/utils.py b/course/utils.py index 2be2311b..944fbcab 100644 --- a/course/utils.py +++ b/course/utils.py @@ -24,11 +24,11 @@ OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. """ +from typing import cast, Text + import six import datetime # noqa -from typing import cast - from django.shortcuts import ( # noqa render, get_object_or_404) from django import http @@ -58,7 +58,7 @@ from course.page.base import ( # noqa # {{{ mypy if False: - from typing import Tuple, List, Text, Iterable, Any, Optional, Union, Dict, FrozenSet # noqa + from typing import Tuple, List, Iterable, Any, Optional, Union, Dict, FrozenSet # noqa from relate.utils import Repo_ish # noqa from course.models import ( # noqa Course, @@ -518,7 +518,7 @@ def get_session_grading_rule( return FlowSessionGradingRule( grade_identifier=grade_identifier, - grade_aggregation_strategy=grade_aggregation_strategy, + grade_aggregation_strategy=cast(Text, grade_aggregation_strategy), due=due, generates_grade=generates_grade, description=getattr(rule, "description", None), @@ -966,7 +966,7 @@ def get_codemirror_widget( # {{{ facility processing def get_facilities_config(request=None): - # type: (Optional[http.HttpRequest]) -> Dict[Text, Dict[Text, Any]] + # type: (Optional[http.HttpRequest]) -> Optional[Dict[Text, Dict[Text, Any]]] from django.conf import settings # This is called during offline validation, where Django isn't really set up. diff --git a/course/validation.py b/course/validation.py index 27ed619c..76c1739a 100644 --- a/course/validation.py +++ b/course/validation.py @@ -1219,7 +1219,8 @@ def check_attributes_yml(vctx, repo, path, tree, access_kinds): from relate.utils import dict_to_struct from yaml import load as load_yaml - att_yml = dict_to_struct(load_yaml(true_repo[attr_blob_sha].data)) + yaml_data = load_yaml(true_repo[attr_blob_sha].data) # type: ignore + att_yml = dict_to_struct(yaml_data) if path: loc = path + "/" + ATTRIBUTES_FILENAME diff --git a/course/views.py b/course/views.py index 5128d3e2..40c5727e 100644 --- a/course/views.py +++ b/course/views.py @@ -24,6 +24,8 @@ OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. """ +from typing import cast, List, Text + import datetime from django.shortcuts import ( # noqa @@ -83,7 +85,7 @@ from course.utils import ( # noqa # {{{ for mypy if False: - from typing import Tuple, List, Text, Optional, Any, Iterable, Dict # noqa + from typing import Tuple, Any, Iterable, Dict, Optional # noqa from course.content import ( # noqa FlowDesc, @@ -1130,9 +1132,10 @@ def grant_exception_stage_3(pctx, participation_id, flow_id, session_id): flow_desc = get_flow_desc(pctx.repo, pctx.course, flow_id, pctx.course_commit_sha) - tags = None + + tags = [] # type: List[Text] if hasattr(flow_desc, "rules"): - tags = getattr(flow_desc.rules, "tags", None) + tags = cast(List[Text], getattr(flow_desc.rules, "tags", [])) # {{{ put together access rule -- GitLab From afd6b0628fda61e4f5ecb036203a5d583b6fbbd8 Mon Sep 17 00:00:00 2001 From: Andreas Kloeckner Date: Fri, 8 Sep 2017 00:01:23 -0500 Subject: [PATCH 36/45] Fix permission cache retrieval after type annotation --- course/models.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/course/models.py b/course/models.py index aa3f9306..f1f2fc87 100644 --- a/course/models.py +++ b/course/models.py @@ -472,10 +472,9 @@ class Participation(models.Model): def permissions(self): # type: () -> FrozenSet[Tuple[Text, Optional[Text]]] - try: + + if self._permissions_cache is not None: return self._permissions_cache - except AttributeError: - pass perm = ( list( -- GitLab From 8e70efd3cfe4efc49cafe6614a40e9d53d5a2594 Mon Sep 17 00:00:00 2001 From: Andreas Kloeckner Date: Fri, 8 Sep 2017 00:10:40 -0500 Subject: [PATCH 37/45] Get if_completed_before check to be mypy-clean --- course/models.py | 2 ++ course/utils.py | 6 +++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/course/models.py b/course/models.py index f1f2fc87..0a3dfe54 100644 --- a/course/models.py +++ b/course/models.py @@ -63,6 +63,7 @@ from course.page.base import AnswerFeedback if False: from typing import List, Dict, Any, Optional, Text, Iterable, Tuple, FrozenSet # noqa from course.content import FlowDesc # noqa + import datetime # noqa # }}} @@ -819,6 +820,7 @@ class FlowSession(models.Model): return assemble_answer_visits(self) def last_activity(self): + # type: () -> Optional[datetime.datetime] for visit in (FlowPageVisit.objects .filter( flow_session=self, diff --git a/course/utils.py b/course/utils.py index 373f29ad..ffbee615 100644 --- a/course/utils.py +++ b/course/utils.py @@ -500,7 +500,11 @@ def get_session_grading_rule( rule.use_last_activity_as_completion_time if use_last_activity_as_completion_time: - completion_time = session.last_activity() + last_activity = session.last_activity() + if last_activity is not None: + completion_time = last_activity + else: + completion_time = now_datetime else: if session.in_progress: completion_time = now_datetime -- GitLab From b6c139350f9ee86c57e0b2d6705f1be6fe214002 Mon Sep 17 00:00:00 2001 From: dzhuang Date: Fri, 8 Sep 2017 15:47:53 +0800 Subject: [PATCH 38/45] Fix build failure on Traivs-ci --- .travis.yml | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index 63bcd6d8..d3d9998f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,8 +1,11 @@ language: python cache: pip install: true +matrix: + include: + - python: "2.7" + env: PY_EXE=python2.7 + - python: "3.5" + env: PY_EXE=python3.5 script: - bash ./run-coveralls.sh -env: - - PY_EXE=python2.7 - - PY_EXE=python3.5 -- GitLab From f9c83ed1ab202dd07877870bea4561ce3032c033 Mon Sep 17 00:00:00 2001 From: dzhuang Date: Sun, 10 Sep 2017 22:40:53 +0800 Subject: [PATCH 39/45] Added Flake8 and Mypy tests via Travis-CI. --- .travis.yml | 10 +++-- appveyor.yml | 72 ++++++++++++++++++++++++++++++++ appveyor/run_with_env.cmd | 88 +++++++++++++++++++++++++++++++++++++++ run-tests-for-ci.sh | 10 ++--- run-travis-ci.sh | 20 +++++++++ setup.cfg | 8 +++- 6 files changed, 197 insertions(+), 11 deletions(-) create mode 100644 appveyor.yml create mode 100644 appveyor/run_with_env.cmd create mode 100644 run-travis-ci.sh diff --git a/.travis.yml b/.travis.yml index 3f73e830..66dc043e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,8 +4,12 @@ install: true matrix: include: - python: "2.7" - env: PY_EXE=python2.7 + env: PY=true PY_EXE=python2.7 - python: "3.5" - env: PY_EXE=python3.5 + env: PY=true PY_EXE=python3.5 + - python: "3.5" + env: Flake8=true PY_EXE=python3.5 + - python: "3.6" + env: Mypy=true PY_EXE=python3.6 script: - - bash ./run-tests-for-ci.sh + - bash ./run-travis-ci.sh diff --git a/appveyor.yml b/appveyor.yml new file mode 100644 index 00000000..db90d589 --- /dev/null +++ b/appveyor.yml @@ -0,0 +1,72 @@ +# +# Most of these settings are taken from here: +# https://github.com/ogrisel/python-appveyor-demo/blob/master/appveyor.yml +# + +version: 1.0.{build} + +build: off + +cache: + - '%LOCALAPPDATA%\pip\Cache' + +environment: + global: + # SDK v7.0 MSVC Express 2008's SetEnv.cmd script will fail if the + # /E:ON and /V:ON options are not enabled in the batch script intepreter + # See: http://stackoverflow.com/a/13751649/163740 + CMD_IN_ENV: "cmd /E:ON /V:ON /C .\\appveyor\\run_with_env.cmd" + + matrix: + # Pre-installed Python versions, which Appveyor may upgrade to + # a later point release. + # See: http://www.appveyor.com/docs/installed-software#python + + - PYTHON: "C:\\Python27-x64" + PYTHON_VERSION: "2.7.x" # currently 2.7.13 + PYTHON_ARCH: "64" + + - PYTHON: "C:\\Python35-x64" + PYTHON_VERSION: "3.5.x" # currently 3.5.3 + PYTHON_ARCH: "64" + +init: + - "ECHO %PYTHON%" + - ps: "ls C:/Python*" + +install: + # Prepend newly installed Python to the PATH of this build (this cannot be + # done from inside the powershell script as it would require to restart + # the parent CMD process). + - "SET PATH=%PYTHON%;%PYTHON%\\Scripts;%PATH%" + + - ECHO "Filesystem root:" + - ps: "ls \"C:/\"" + + # Check that we have the expected version and architecture for Python + - "python --version" + - "python -c \"import struct; print(struct.calcsize('P') * 8)\"" + + # Upgrade to the latest version of pip to avoid it displaying warnings + # about it being out of date. + - "pip install --disable-pip-version-check --user --upgrade pip" + + - "git submodule update --init --recursive" + + # Install requirements + - ps: "cat requirements.txt | ?{$_ -notmatch 'dnspython'} > req.txt" + - "%CMD_IN_ENV% pip install -r req.txt" + - ps: "if ($env:PYTHON -like '*Python2*') { pip install dnspython mock} else { pip install dnspython3 }" + + +before_test: + - "%CMD_IN_ENV% pip install coverage" + +test_script: + # Run the project tests + - "copy local_settings.example.py local_settings.py /Y" + - "%CMD_IN_ENV% coverage run manage.py test test/" + +after_test: + - "%CMD_IN_ENV% pip install codecov" + - "%CMD_IN_ENV% codecov -X gcov" diff --git a/appveyor/run_with_env.cmd b/appveyor/run_with_env.cmd new file mode 100644 index 00000000..5da547c4 --- /dev/null +++ b/appveyor/run_with_env.cmd @@ -0,0 +1,88 @@ +:: To build extensions for 64 bit Python 3, we need to configure environment +:: variables to use the MSVC 2010 C++ compilers from GRMSDKX_EN_DVD.iso of: +:: MS Windows SDK for Windows 7 and .NET Framework 4 (SDK v7.1) +:: +:: To build extensions for 64 bit Python 2, we need to configure environment +:: variables to use the MSVC 2008 C++ compilers from GRMSDKX_EN_DVD.iso of: +:: MS Windows SDK for Windows 7 and .NET Framework 3.5 (SDK v7.0) +:: +:: 32 bit builds, and 64-bit builds for 3.5 and beyond, do not require specific +:: environment configurations. +:: +:: Note: this script needs to be run with the /E:ON and /V:ON flags for the +:: cmd interpreter, at least for (SDK v7.0) +:: +:: More details at: +:: https://github.com/cython/cython/wiki/64BitCythonExtensionsOnWindows +:: http://stackoverflow.com/a/13751649/163740 +:: +:: Author: Olivier Grisel +:: License: CC0 1.0 Universal: http://creativecommons.org/publicdomain/zero/1.0/ +:: +:: Notes about batch files for Python people: +:: +:: Quotes in values are literally part of the values: +:: SET FOO="bar" +:: FOO is now five characters long: " b a r " +:: If you don't want quotes, don't include them on the right-hand side. +:: +:: The CALL lines at the end of this file look redundant, but if you move them +:: outside of the IF clauses, they do not run properly in the SET_SDK_64==Y +:: case, I don't know why. +@ECHO OFF + +SET COMMAND_TO_RUN=%* +SET WIN_SDK_ROOT=C:\Program Files\Microsoft SDKs\Windows +SET WIN_WDK=c:\Program Files (x86)\Windows Kits\10\Include\wdf + +:: Extract the major and minor versions, and allow for the minor version to be +:: more than 9. This requires the version number to have two dots in it. +SET MAJOR_PYTHON_VERSION=%PYTHON_VERSION:~0,1% +IF "%PYTHON_VERSION:~3,1%" == "." ( + SET MINOR_PYTHON_VERSION=%PYTHON_VERSION:~2,1% +) ELSE ( + SET MINOR_PYTHON_VERSION=%PYTHON_VERSION:~2,2% +) + +:: Based on the Python version, determine what SDK version to use, and whether +:: to set the SDK for 64-bit. +IF %MAJOR_PYTHON_VERSION% == 2 ( + SET WINDOWS_SDK_VERSION="v7.0" + SET SET_SDK_64=Y +) ELSE ( + IF %MAJOR_PYTHON_VERSION% == 3 ( + SET WINDOWS_SDK_VERSION="v7.1" + IF %MINOR_PYTHON_VERSION% LEQ 4 ( + SET SET_SDK_64=Y + ) ELSE ( + SET SET_SDK_64=N + IF EXIST "%WIN_WDK%" ( + :: See: https://connect.microsoft.com/VisualStudio/feedback/details/1610302/ + REN "%WIN_WDK%" 0wdf + ) + ) + ) ELSE ( + ECHO Unsupported Python version: "%MAJOR_PYTHON_VERSION%" + EXIT 1 + ) +) + +IF %PYTHON_ARCH% == 64 ( + IF %SET_SDK_64% == Y ( + ECHO Configuring Windows SDK %WINDOWS_SDK_VERSION% for Python %MAJOR_PYTHON_VERSION% on a 64 bit architecture + SET DISTUTILS_USE_SDK=1 + SET MSSdk=1 + "%WIN_SDK_ROOT%\%WINDOWS_SDK_VERSION%\Setup\WindowsSdkVer.exe" -q -version:%WINDOWS_SDK_VERSION% + "%WIN_SDK_ROOT%\%WINDOWS_SDK_VERSION%\Bin\SetEnv.cmd" /x64 /release + ECHO Executing: %COMMAND_TO_RUN% + call %COMMAND_TO_RUN% || EXIT 1 + ) ELSE ( + ECHO Using default MSVC build environment for 64 bit architecture + ECHO Executing: %COMMAND_TO_RUN% + call %COMMAND_TO_RUN% || EXIT 1 + ) +) ELSE ( + ECHO Using default MSVC build environment for 32 bit architecture + ECHO Executing: %COMMAND_TO_RUN% + call %COMMAND_TO_RUN% || EXIT 1 +) diff --git a/run-tests-for-ci.sh b/run-tests-for-ci.sh index be80ebd9..848e5369 100644 --- a/run-tests-for-ci.sh +++ b/run-tests-for-ci.sh @@ -68,11 +68,7 @@ $PIP install -r req.txt cp local_settings.example.py local_settings.py -#python manage.py test test/ - -$PIP install coverage -coverage run --source=. manage.py test test/ -coverage report -m - $PIP install codecov -codecov -X gcov \ No newline at end of file +coverage run manage.py test test/ +coverage report -m +codecov diff --git a/run-travis-ci.sh b/run-travis-ci.sh new file mode 100644 index 00000000..b2a25525 --- /dev/null +++ b/run-travis-ci.sh @@ -0,0 +1,20 @@ +#! /bin/bash + +# before_script +if [[ $Flake8 == true ]]; then + curl -L -O -k https://gitlab.tiker.net/inducer/ci-support/raw/master/prepare-and-run-flake8.sh +fi + +if [[ $Mypy == true ]]; then + curl -L -O -k https://gitlab.tiker.net/inducer/ci-support/raw/master/prepare-and-run-mypy.sh + cp local_settings.example.py local_settings.py +fi + +# run ci according to env variables +if [[ $PY == true ]]; then + . ./run-tests-for-ci.sh +elif [[ $Mypy == true ]]; then + . ./prepare-and-run-mypy.sh python3.6 mypy==0.521 typed-ast==1.0.4 +elif [[ $Flake8 == true ]]; then + . ./prepare-and-run-flake8.sh relate course accounts test bin +fi \ No newline at end of file diff --git a/setup.cfg b/setup.cfg index 05106c25..b5bc2109 100644 --- a/setup.cfg +++ b/setup.cfg @@ -4,11 +4,11 @@ max-line-length=85 exclude=course/migrations,accounts/migrations,static,components,course/mdx_mathjax.py [coverage:run] +source = . branch=True cover_pylib=False omit = */.env/* - */__init__.py */virtualenv*/* */setuptools*/* */migrations/* @@ -33,6 +33,9 @@ exclude_lines = def __repr__ if self.debug if settings.Debug + if debug + debug_print + if show_log # Don't complain if tests don't hit defensive assertion code: raise AssertionError @@ -42,4 +45,7 @@ exclude_lines = if 0: if __name__ == .__main__.: + # mypy import + if False: + ignore_errors = True -- GitLab From bc3c48a987f67dcf57db9ef713601fb7c174f75d Mon Sep 17 00:00:00 2001 From: Andreas Kloeckner Date: Sun, 10 Sep 2017 22:17:23 -0500 Subject: [PATCH 40/45] Gitignore mypy cache --- .gitignore | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.gitignore b/.gitignore index d8bbde7c..1121f351 100644 --- a/.gitignore +++ b/.gitignore @@ -21,3 +21,5 @@ saml_config.py *.pem git-roots + +.mypy_cache -- GitLab From 6ec7376cf6ba83baadc43a85661ccc6f902061ad Mon Sep 17 00:00:00 2001 From: dzhuang Date: Mon, 11 Sep 2017 23:21:18 +0800 Subject: [PATCH 41/45] refactor tests for pages. --- test/base_test_mixins.py | 74 ++++++++++++++++++++- test/test_pages.py | 140 +++++++++++++-------------------------- 2 files changed, 119 insertions(+), 95 deletions(-) diff --git a/test/base_test_mixins.py b/test/base_test_mixins.py index a9932c7e..0906087d 100644 --- a/test/base_test_mixins.py +++ b/test/base_test_mixins.py @@ -25,10 +25,10 @@ THE SOFTWARE. import os from django.conf import settings from django.test import Client -from django.urls import reverse +from django.urls import reverse, resolve from django.contrib.auth import get_user_model from relate.utils import force_remove_path -from course.models import Course, Participation, ParticipationRole +from course.models import Course, Participation, ParticipationRole, FlowSession from course.constants import participation_status CREATE_SUPERUSER_KWARGS = { @@ -248,3 +248,73 @@ class SingleCourseTestMixin(CoursesTestMixinBase): @classmethod def tearDownClass(cls): super(SingleCourseTestMixin, cls).tearDownClass() + + +class SingleCoursePageTestMixin(SingleCourseTestMixin): + @property + def flow_id(self): + raise NotImplementedError + + @classmethod + def setUpTestData(cls): # noqa + super(SingleCoursePageTestMixin, cls).setUpTestData() + cls.c.force_login(cls.student_participation.user) + cls.start_quiz(cls.flow_id) + + @classmethod + def start_quiz(cls, flow_id): + existing_quiz_count = FlowSession.objects.all().count() + params = {"course_identifier": cls.course.identifier, + "flow_id": flow_id} + resp = cls.c.post(reverse("relate-view_start_flow", kwargs=params)) + assert resp.status_code == 302 + new_quiz_count = FlowSession.objects.all().count() + assert new_quiz_count == existing_quiz_count + 1 + + # Yep, no regax! + _, _, kwargs = resolve(resp.url) + # Should be in correct course + assert kwargs["course_identifier"] == cls.course.identifier + # Should redirect us to welcome page + assert int(kwargs["ordinal"]) == 0 + cls.page_params = kwargs + + @classmethod + def end_quiz(cls): + from copy import deepcopy + page_params = deepcopy(cls.page_params) + del page_params["ordinal"] + resp = cls.c.post(reverse("relate-finish_flow_session_view", + kwargs=page_params), {'submit': ['']}) + return resp + + @classmethod + def get_ordinal_via_page_id(cls, page_id): + from course.models import FlowPageData + flow_page_data = FlowPageData.objects.get( + flow_session__id=cls.page_params["flow_session_id"], + page_id=page_id + ) + return flow_page_data.ordinal + + @classmethod + def client_post_answer_by_page_id(cls, page_id, answer_data): + page_ordinal = cls.get_ordinal_via_page_id(page_id) + return cls.client_post_answer_by_ordinal(page_ordinal, answer_data) + + @classmethod + def client_post_answer_by_ordinal(cls, page_ordinal, answer_data): + from copy import deepcopy + page_params = deepcopy(cls.page_params) + page_params.update({"ordinal": str(page_ordinal)}) + submit_data = answer_data + submit_data.update({"submit": ["Submit final answer"]}) + resp = cls.c.post( + reverse("relate-view_flow_page", kwargs=page_params), + submit_data) + return resp + + def assertSessionScoreEqual(self, expect_score): # noqa + from decimal import Decimal + self.assertEqual(FlowSession.objects.all()[0].points, + Decimal(str(expect_score))) diff --git a/test/test_pages.py b/test/test_pages.py index 04bdf7db..07acc1cf 100644 --- a/test/test_pages.py +++ b/test/test_pages.py @@ -23,18 +23,16 @@ THE SOFTWARE. """ from django.test import TestCase -from django.urls import resolve, reverse +from django.urls import reverse from django.contrib.auth import get_user_model -from course.models import FlowSession, FlowPageVisit, Course -from decimal import Decimal -from base_test_mixins import SingleCourseTestMixin +from course.models import FlowPageVisit, Course +from base_test_mixins import SingleCoursePageTestMixin +QUIZ_FLOW_ID = "quiz-test" -class SingleCoursePageTest(SingleCourseTestMixin, TestCase): - @classmethod - def setUpTestData(cls): # noqa - super(SingleCoursePageTest, cls).setUpTestData() - cls.c.force_login(cls.student_participation.user) + +class SingleCourseQuizPageTest(SingleCoursePageTestMixin, TestCase): + flow_id = QUIZ_FLOW_ID # TODO: This should be moved to tests for auth module def test_user_creation(self): @@ -60,122 +58,78 @@ class SingleCoursePageTest(SingleCourseTestMixin, TestCase): self.assertEqual(resp.status_code, 200) def test_quiz_no_answer(self): - params = self.start_quiz() - self.end_quiz(params, 0) + self.assertEqual(self.end_quiz().status_code, 200) + self.assertSessionScoreEqual(0) def test_quiz_text(self): - params = self.start_quiz() - params["ordinal"] = '1' - resp = self.c.post(reverse("relate-view_flow_page", kwargs=params), - {"answer": ['0.5'], "submit": ["Submit final answer"]}) + resp = self.client_post_answer_by_ordinal(1, {"answer": ['0.5']}) self.assertEqual(resp.status_code, 200) - self.end_quiz(params, 5) + self.assertEqual(self.end_quiz().status_code, 200) + self.assertSessionScoreEqual(5) def test_quiz_choice(self): - params = self.start_quiz() - params["ordinal"] = '2' - resp = self.c.post(reverse("relate-view_flow_page", kwargs=params), - {"choice": ['0'], "submit": ["Submit final answer"]}) + resp = self.client_post_answer_by_ordinal(2, {"choice": ['0']}) self.assertEqual(resp.status_code, 200) - self.end_quiz(params, 2) + self.assertEqual(self.end_quiz().status_code, 200) + self.assertSessionScoreEqual(2) def test_quiz_multi_choice_exact_correct(self): - params = self.start_quiz() - params["ordinal"] = '3' - resp = self.c.post(reverse("relate-view_flow_page", kwargs=params), - {"choice": ['0', '1', '4'], "submit": ["Submit final answer"]}) + resp = self.client_post_answer_by_ordinal(3, {"choice": ['0', '1', '4']}) self.assertEqual(resp.status_code, 200) - self.end_quiz(params, 1) + self.assertEqual(self.end_quiz().status_code, 200) + self.assertSessionScoreEqual(1) def test_quiz_multi_choice_exact_wrong(self): - params = self.start_quiz() - params["ordinal"] = '3' - resp = self.c.post(reverse("relate-view_flow_page", kwargs=params), - {"choice": ['0', '1'], "submit": ["Submit final answer"]}) + resp = self.client_post_answer_by_ordinal(3, {"choice": ['0', '1']}) self.assertEqual(resp.status_code, 200) - self.end_quiz(params, 0) + self.assertEqual(self.end_quiz().status_code, 200) + self.assertSessionScoreEqual(0) - def test_quiz_multi_choice_propotion_partial(self): - params = self.start_quiz() - params["ordinal"] = '4' - resp = self.c.post(reverse("relate-view_flow_page", kwargs=params), - {"choice": ['0'], "submit": ["Submit final answer"]}) + def test_quiz_multi_choice_proportion_partial(self): + resp = self.client_post_answer_by_ordinal(4, {"choice": ['0']}) self.assertEqual(resp.status_code, 200) - self.end_quiz(params, 0.8) + self.assertEqual(self.end_quiz().status_code, 200) + self.assertSessionScoreEqual(0.8) - def test_quiz_multi_choice_propotion_correct(self): - params = self.start_quiz() - params["ordinal"] = '4' - resp = self.c.post(reverse("relate-view_flow_page", kwargs=params), - {"choice": ['0', '3'], "submit": ["Submit final answer"]}) + def test_quiz_multi_choice_proportion_correct(self): + resp = self.client_post_answer_by_ordinal(4, {"choice": ['0', '3']}) self.assertEqual(resp.status_code, 200) - self.end_quiz(params, 1) + self.assertEqual(self.end_quiz().status_code, 200) + self.assertSessionScoreEqual(1) def test_quiz_inline(self): - params = self.start_quiz() - params["ordinal"] = '5' - data = {'blank1': ['Bar'], 'blank_2': ['0.2'], 'blank3': ['1'], - 'blank4': ['5'], 'blank5': ['Bar'], 'choice2': ['0'], - 'choice_a': ['0'], 'submit': ['Submit final answer']} - resp = self.c.post(reverse("relate-view_flow_page", kwargs=params), data) + answer_data = { + 'blank1': ['Bar'], 'blank_2': ['0.2'], 'blank3': ['1'], + 'blank4': ['5'], 'blank5': ['Bar'], 'choice2': ['0'], + 'choice_a': ['0']} + resp = self.client_post_answer_by_ordinal(5, answer_data) self.assertEqual(resp.status_code, 200) - self.end_quiz(params, 10) + self.assertEqual(self.end_quiz().status_code, 200) + self.assertSessionScoreEqual(10) # All I can do for now since db do not store ordinal value def test_quiz_survey_choice(self): - params = self.start_quiz() - params["ordinal"] = '6' - resp = self.c.post(reverse("relate-view_flow_page", kwargs=params), - {"answer": ["NOTHING!!!"], "submit": ["Submit final answer"]}) + resp = self.client_post_answer_by_ordinal(6, {"answer": ["NOTHING!!!"]}) self.assertEqual(resp.status_code, 200) - self.end_quiz(params, 0) + self.assertEqual(self.end_quiz().status_code, 200) + self.assertSessionScoreEqual(0) query = FlowPageVisit.objects.filter( - flow_session__exact=params["flow_session_id"], - answer__isnull=False) - self.assertEqual(len(query), 1) + flow_session__exact=self.page_params["flow_session_id"], + answer__isnull=False) + self.assertEqual(query.count(), 1) record = query[0] self.assertEqual(record.answer["answer"], "NOTHING!!!") def test_quiz_survey_text(self): - params = self.start_quiz() - params["ordinal"] = '7' - resp = self.c.post(reverse("relate-view_flow_page", kwargs=params), - {"choice": ['8'], "submit": ["Submit final answer"]}) + resp = self.client_post_answer_by_ordinal(7, {"choice": ['8']}) self.assertEqual(resp.status_code, 200) - self.end_quiz(params, 0) + self.assertEqual(self.end_quiz().status_code, 200) + self.assertSessionScoreEqual(0) query = FlowPageVisit.objects.filter( - flow_session__exact=params["flow_session_id"], + flow_session__exact=self.page_params["flow_session_id"], answer__isnull=False) - self.assertEqual(len(query), 1) + self.assertEqual(query.count(), 1) record = query[0] self.assertEqual(record.answer["choice"], 8) - - # Decorator won't work here :( - def start_quiz(self): - self.assertEqual(len(FlowSession.objects.all()), 0) - params = {"course_identifier": self.course.identifier, - "flow_id": "quiz-test"} - resp = self.c.post(reverse("relate-view_start_flow", kwargs=params)) - self.assertEqual(resp.status_code, 302) - self.assertEqual(FlowSession.objects.all().count(), 1) - - # Yep, no regax! - _, _, kwargs = resolve(resp.url) - # Should be in correct course - self.assertEqual(kwargs["course_identifier"], self.course.identifier) - # Should redirect us to welcome page - self.assertEqual(kwargs["ordinal"], '0') - - return kwargs - - def end_quiz(self, params, expect_score): - # Let it raise error - # Use pop() will not - del params["ordinal"] - resp = self.c.post(reverse("relate-finish_flow_session_view", - kwargs=params), {'submit': ['']}) - self.assertEqual(resp.status_code, 200) - self.assertEqual(FlowSession.objects.all()[0].points, - Decimal(str(expect_score))) -- GitLab From 91275639b4be2ff4e2df04e25bc3f5b46cb9e02a Mon Sep 17 00:00:00 2001 From: dzhuang Date: Tue, 12 Sep 2017 13:38:51 +0800 Subject: [PATCH 42/45] ImportError for typing.Text PY3.5 (win). followed #306 --- course/utils.py | 14 +++++++++++--- course/views.py | 10 +++++++--- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/course/utils.py b/course/utils.py index 8133a79f..baa1b6cb 100644 --- a/course/utils.py +++ b/course/utils.py @@ -24,7 +24,7 @@ OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. """ -from typing import cast, Text +from typing import cast import six import datetime # noqa @@ -58,7 +58,8 @@ from course.page.base import ( # noqa # {{{ mypy if False: - from typing import Tuple, List, Iterable, Any, Optional, Union, Dict, FrozenSet # noqa + from typing import ( # noqa + Tuple, List, Iterable, Any, Optional, Union, Dict, FrozenSet, Text) from relate.utils import Repo_ish # noqa from course.models import ( # noqa Course, @@ -532,9 +533,16 @@ def get_session_grading_rule( max_points_enforced_cap = getattr_with_fallback( (rule, flow_desc), "max_points_enforced_cap", None) + try: + from typing import Text # noqa + except ImportError: + Text = None # noqa + + grade_aggregation_strategy = cast(Text, grade_aggregation_strategy) # type: ignore # noqa + return FlowSessionGradingRule( grade_identifier=grade_identifier, - grade_aggregation_strategy=cast(Text, grade_aggregation_strategy), + grade_aggregation_strategy=grade_aggregation_strategy, due=due, generates_grade=generates_grade, description=getattr(rule, "description", None), diff --git a/course/views.py b/course/views.py index 714028ee..788e9182 100644 --- a/course/views.py +++ b/course/views.py @@ -24,7 +24,7 @@ OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. """ -from typing import cast, List, Text +from typing import cast, List import datetime @@ -85,7 +85,7 @@ from course.utils import ( # noqa # {{{ for mypy if False: - from typing import Tuple, Any, Iterable, Dict, Optional # noqa + from typing import Tuple, Text, Any, Iterable, Dict, Optional # noqa from course.content import ( # noqa FlowDesc, @@ -1136,7 +1136,11 @@ def grant_exception_stage_3(pctx, participation_id, flow_id, session_id): tags = [] # type: List[Text] if hasattr(flow_desc, "rules"): - tags = cast(List[Text], getattr(flow_desc.rules, "tags", [])) + try: + from typing import Text # noqa + except ImportError: + Text = None # noqa + tags = cast(List[Text], getattr(flow_desc.rules, "tags", [])) # type: ignore # noqa # {{{ put together access rule -- GitLab From 06c0887199dfdb6f495e3972e9557f83aacd995d Mon Sep 17 00:00:00 2001 From: dzhuang Date: Wed, 13 Sep 2017 23:47:42 +0800 Subject: [PATCH 43/45] Fix Participation not imported --- course/utils.py | 1 + 1 file changed, 1 insertion(+) diff --git a/course/utils.py b/course/utils.py index baa1b6cb..18403204 100644 --- a/course/utils.py +++ b/course/utils.py @@ -1087,6 +1087,7 @@ def will_use_masked_profile_for_email(recipient_email): return False if not isinstance(recipient_email, list): recipient_email = [recipient_email] + from course.models import Participation # noqa recepient_participations = ( Participation.objects.filter( user__email__in=recipient_email -- GitLab From 8f2a7212a49f1a827636c6550ac21d7ec091a6ff Mon Sep 17 00:00:00 2001 From: dzhuang Date: Thu, 14 Sep 2017 13:47:53 +0800 Subject: [PATCH 44/45] Re-organize calendar view. --- course/calendar.py | 11 +- course/templates/course/calendar.html | 145 ++++++++++++++--- course/templates/course/calender_edit.html | 181 --------------------- 3 files changed, 129 insertions(+), 208 deletions(-) delete mode 100644 course/templates/course/calender_edit.html diff --git a/course/calendar.py b/course/calendar.py index bb838fed..faed041b 100644 --- a/course/calendar.py +++ b/course/calendar.py @@ -411,6 +411,7 @@ def view_calendar(pctx): "events_json": dumps(events_json), "event_info_list": event_info_list, "default_date": default_date.isoformat(), + "edit_view": False }) @@ -432,14 +433,13 @@ class EditEventForm(StyledModelForm): @login_required @course_view def edit_calendar(pctx): + if not pctx.has_permission(pperm.edit_events): + raise PermissionDenied(_("may not edit events")) + from course.content import markup_to_html, parse_date_spec from course.views import get_now_or_fake_time now = get_now_or_fake_time(pctx.request) - - if not pctx.has_permission(pperm.edit_events): - raise PermissionDenied(_("may not edit events")) - request = pctx.request edit_existing_event_flag = False @@ -609,13 +609,14 @@ def edit_calendar(pctx): default_date = pctx.course.end_date from json import dumps - return render_course_page(pctx, "course/calender_edit.html", { + return render_course_page(pctx, "course/calendar.html", { "form": edit_event_form, "events_json": dumps(events_json), "event_info_list": event_info_list, "default_date": default_date.isoformat(), "edit_existing_event_flag": edit_existing_event_flag, "id_to_edit": id_to_edit, + "edit_view": True }) # }}} diff --git a/course/templates/course/calendar.html b/course/templates/course/calendar.html index ec4f99fa..8cfc99ca 100644 --- a/course/templates/course/calendar.html +++ b/course/templates/course/calendar.html @@ -8,41 +8,45 @@ {% trans "Calendar" %} - {% trans "RELATE" %} {% endblock %} -{%block header_extra %} +{% block header_extra %} {# load calendar with local language #} {% get_current_language as LANGUAGE_CODE %} + {% if edit_view %} + + + {% endif %} {% endblock %} {% block content %}

{{ course.number}} {% trans "Calendar" %}

- + {% if pperm.edit_events %} + + {% endif %}
+ {% trans "Note" %}: {% if edit_view %}{% trans "Different from the students' calendar, this calender shows all events. " %}{% endif %}{% trans "Some calendar entries are clickable and link to entries below." %} - - -{% blocktrans trimmed %} - Note: Some calendar entries are clickable and link to entries - below. -{% endblocktrans %}
{% for event_info in event_info_list %} @@ -58,5 +62,102 @@ {% endfor%}
+ {% if edit_view %} + +
+ {% csrf_token %} + +
+
+ {% csrf_token %} + +
+ {% endif %} {% endblock %} +{% block page_bottom_javascript_extra %} + + {{ block.super }} +{% endblock %} \ No newline at end of file diff --git a/course/templates/course/calender_edit.html b/course/templates/course/calender_edit.html deleted file mode 100644 index 9a17b712..00000000 --- a/course/templates/course/calender_edit.html +++ /dev/null @@ -1,181 +0,0 @@ -{% extends "course/course-base.html" %} -{% load i18n %} - -{% load static %} - -{% block title %} - {{ course.number}} - {% trans "Calendar" %} - {% trans "RELATE" %} -{% endblock %} - -{%block header_extra %} - - - - {# load calendar with local language #} - {% get_current_language as LANGUAGE_CODE %} - - - - - - -{% endblock %} - - - - - -{% block content %} - -{% blocktrans trimmed %} - Note: Different from the students' calendar, this calender shows all events. -{% endblocktrans %} - - - - -
-
- -
- -
-
-
- -
- - - - -{% blocktrans trimmed %} - Note: Some calendar entries are clickable and link to entries - below. -{% endblocktrans %} - -
- {% for event_info in event_info_list %} -
-
- {{ event_info.human_title }} - ({{ event_info.start_time }}{% if event_info.end_time %} - {{ event_info.end_time }}{% endif %}) -
-
- {{ event_info.description|safe }} -
-
- {% endfor%} -
- - {% endif %} @@ -88,14 +88,14 @@ {% endif %} - -
- - -
+
+ + +
+
{% csrf_token %} @@ -121,8 +121,8 @@ events: {{ events_json|safe }}, {% if edit_view %} eventRender: function(event, element) { - element.find('.fc-title').append( ''); - element.find('.fc-title').append( ''); + element.find('.fc-title').append( ''); + element.find('.fc-title').append( ' '); element.find("#edit").click(function() { edit_existing_event(event.id); }); @@ -136,28 +136,28 @@ {% if edit_existing_event_flag %} $('#editModal').modal('show'); {% endif %} + {% endif %} + }); - $(function () { - $('#start_time').datetimepicker({"format": "YYYY-MM-DD HH:mm", "sideBySide": true}); - $('#end_time').datetimepicker({"format": "YYYY-MM-DD HH:mm", "sideBySide": true}); - }); + $(function () { + $('#start_time').datetimepicker({"format": "YYYY-MM-DD HH:mm", "sideBySide": true}); + $('#end_time').datetimepicker({"format": "YYYY-MM-DD HH:mm", "sideBySide": true}); + }); - function edit_existing_event(event_id) { - $('#id_to_edit').val(event_id); - document.existing_event_edit_form.submit(); - } + function edit_existing_event(event_id) { + $('#id_to_edit').val(event_id); + document.existing_event_edit_form.submit(); + } - function delete_existing_event(event_id) { - $('#id_to_delete').val(event_id); - document.existing_event_delete_form.submit(); - } + function delete_existing_event(event_id) { + $('#id_to_delete').val(event_id); + document.existing_event_delete_form.submit(); + } - function remove_event_id_field() { - $('#existing_event_to_save').remove() - } + function remove_event_id_field() { + $('#existing_event_to_save').remove() + } - {% endif %} - }); {{ block.super }} {% endblock %} \ No newline at end of file diff --git a/test/base_test_mixins.py b/test/base_test_mixins.py index 0906087d..bf0bcee2 100644 --- a/test/base_test_mixins.py +++ b/test/base_test_mixins.py @@ -190,13 +190,13 @@ class CoursesTestMixinBase(SuperuserCreateMixin): @classmethod def create_participation( cls, course, create_user_kwargs, role_identifier, status): - try: - # TODO: why pop failed here? - password = create_user_kwargs["password"] - except: - raise user, created = get_user_model().objects.get_or_create(**create_user_kwargs) if created: + try: + # TODO: why pop failed here? + password = create_user_kwargs["password"] + except: + raise user.set_password(password) user.save() participation, p_created = Participation.objects.get_or_create( @@ -215,6 +215,29 @@ class CoursesTestMixinBase(SuperuserCreateMixin): cls.c.force_login(cls.superuser) cls.c.post(reverse("relate-set_up_new_course"), create_course_kwargs) + def assertMessageContains(self, resp, expected_message): # noqa + """ + :param resp: response + :param expected_message: message string or list containing message string + """ + if isinstance(expected_message, list): + self.assertTrue(set(expected_message).issubset( + set([m.message for m in list(resp.context['messages'])]))) + if isinstance(expected_message, str): + self.assertIn(expected_message, + [m.message for m in list(resp.context['messages'])]) + + def debug_print_response_messages(self, resp): + """ + For debugging :class:`django.contrib.messages` objects in post response + :param resp: response + """ + print("\n") + print("-----------message start-------------") + for m in list(resp.context['messages']): + print(m.message) + print("-----------message end-------------") + class SingleCourseTestMixin(CoursesTestMixinBase): courses_setup_list = SINGLE_COURSE_SETUP_LIST diff --git a/test/test_calendar.py b/test/test_calendar.py new file mode 100644 index 00000000..4f59556f --- /dev/null +++ b/test/test_calendar.py @@ -0,0 +1,424 @@ +from __future__ import division + +__copyright__ = "Copyright (C) 2017 Dong Zhuang" + +__license__ = """ +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in +all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +THE SOFTWARE. +""" + +import json +from django.test import TestCase +from django.urls import reverse +from course.models import Event +from base_test_mixins import SingleCourseTestMixin +from django.utils.timezone import now + +SHOWN_EVENT_KIND = "test_open_event" +HIDDEN_EVENT_KIND = "test_secret_event" +OPEN_EVENT_NO_ORDINAL = "test_open_no_ordinal" +HIDDEN_EVENT_NO_ORDINAL = "test_secret_no_ordinal" +FAILURE_EVENT_KIND = "never_created_event_kind" + +TEST_EVENTS = ( + {"kind": SHOWN_EVENT_KIND, "ordinal": "1", + "shown_in_calendar": True, "time": now()}, + {"kind": SHOWN_EVENT_KIND, "ordinal": "2", + "shown_in_calendar": True, "time": now()}, + {"kind": SHOWN_EVENT_KIND, "ordinal": "3", + "shown_in_calendar": True, "time": now()}, + {"kind": HIDDEN_EVENT_KIND, "ordinal": "1", + "shown_in_calendar": False, "time": now()}, + {"kind": HIDDEN_EVENT_KIND, "ordinal": "2", + "shown_in_calendar": False, "time": now()}, + {"kind": OPEN_EVENT_NO_ORDINAL, + "shown_in_calendar": True, "time": now()}, + {"kind": HIDDEN_EVENT_NO_ORDINAL, + "shown_in_calendar": False, "time": now()}, +) + +N_TEST_EVENTS = len(TEST_EVENTS) # 7 events +N_HIDDEN_EVENTS = len([event + for event in TEST_EVENTS + if not event["shown_in_calendar"]]) # 3 events +N_SHOWN_EVENTS = N_TEST_EVENTS - N_HIDDEN_EVENTS # 4 events + +# html literals (from template) +MENU_EDIT_EVENTS_ADMIN = "Edit events (admin)" +MENU_VIEW_EVENTS_CALENDAR = "Edit events (calendar)" +MENU_CREATE_RECURRING_EVENTS = "Create recurring events" +MENU_RENUMBER_EVENTS = "Renumber events" + +HTML_SWITCH_TO_STUDENT_VIEW = "Switch to Student View" +HTML_SWITCH_TO_EDIT_VIEW = "Switch to Edit View" +HTML_CREATE_NEW_EVENT_BUTTON_TITLE = "create a new event" +HTML_EVENT_CREATED_MSG = "Event created." + +MSG_EVENT_NOT_CREATED = "No event created." +MSG_PREFIX_EVENT_ALREAD_EXIST_FAILURE_MSG = "EventAlreadyExists:" +MSG_HTML_EVENT_DELETED = "Event deleted." +MSG_EVENT_UPDATED = "Event updated." +MSG_EVENT_NOT_UPDATED = "Event not updated." + + +class CalendarTest(SingleCourseTestMixin, TestCase): + @classmethod + def setUpTestData(cls): # noqa + super(CalendarTest, cls).setUpTestData() + + # superuser was previously removed from participation, now we add him back + from course.constants import participation_status + + cls.create_participation( + course=cls.course, + create_user_kwargs={"id": cls.superuser.id}, + role_identifier="instructor", + status=participation_status.active) + + for event in TEST_EVENTS: + event.update({ + "course": cls.course, + }) + Event.objects.create(**event) + assert Event.objects.count() == N_TEST_EVENTS + + def assertShownEventsCountEqual(self, resp, expected_shown_events_count): # noqa + self.assertEqual( + len(json.loads(resp.context["events_json"])), + expected_shown_events_count) + + def assertTotalEventsCountEqual(self, expected_total_events_count): # noqa + self.assertEqual(Event.objects.count(), expected_total_events_count) + + def test_superuser_instructor_calendar_get(self): + self.c.force_login(self.superuser) + resp = self.c.get( + reverse("relate-view_calendar", args=[self.course.identifier])) + self.assertEqual(resp.status_code, 200) + + # menu items + self.assertContains(resp, MENU_EDIT_EVENTS_ADMIN) + self.assertContains(resp, MENU_VIEW_EVENTS_CALENDAR) + self.assertContains(resp, MENU_CREATE_RECURRING_EVENTS) + self.assertContains(resp, MENU_RENUMBER_EVENTS) + + # rendered page html + self.assertNotContains(resp, HTML_SWITCH_TO_STUDENT_VIEW) + self.assertContains(resp, HTML_SWITCH_TO_EDIT_VIEW) + self.assertNotContains(resp, HTML_CREATE_NEW_EVENT_BUTTON_TITLE) + + # see only shown events + self.assertShownEventsCountEqual(resp, N_SHOWN_EVENTS) + + def test_non_superuser_instructor_calendar_get(self): + self.c.force_login(self.instructor_participation.user) + resp = self.c.get( + reverse("relate-view_calendar", args=[self.course.identifier])) + self.assertEqual(resp.status_code, 200) + + # menu items + self.assertNotContains(resp, MENU_EDIT_EVENTS_ADMIN) + self.assertContains(resp, MENU_VIEW_EVENTS_CALENDAR) + self.assertContains(resp, MENU_CREATE_RECURRING_EVENTS) + self.assertContains(resp, MENU_RENUMBER_EVENTS) + + # rendered page html + self.assertNotContains(resp, HTML_SWITCH_TO_STUDENT_VIEW) + self.assertContains(resp, HTML_SWITCH_TO_EDIT_VIEW) + self.assertNotContains(resp, HTML_CREATE_NEW_EVENT_BUTTON_TITLE) + + # see only shown events + self.assertShownEventsCountEqual(resp, N_SHOWN_EVENTS) + + def test_student_calendar_get(self): + self.c.force_login(self.student_participation.user) + resp = self.c.get( + reverse("relate-view_calendar", args=[self.course.identifier])) + self.assertEqual(resp.status_code, 200) + + # menu items + self.assertNotContains(resp, MENU_EDIT_EVENTS_ADMIN) + self.assertNotContains(resp, MENU_VIEW_EVENTS_CALENDAR) + self.assertNotContains(resp, MENU_CREATE_RECURRING_EVENTS) + self.assertNotContains(resp, MENU_RENUMBER_EVENTS) + + # rendered page html + self.assertNotContains(resp, HTML_SWITCH_TO_STUDENT_VIEW) + self.assertNotContains(resp, HTML_SWITCH_TO_EDIT_VIEW) + self.assertNotContains(resp, HTML_CREATE_NEW_EVENT_BUTTON_TITLE) + + # see only shown events + self.assertShownEventsCountEqual(resp, N_SHOWN_EVENTS) + + def test_superuser_instructor_calendar_edit_get(self): + self.c.force_login(self.superuser) + resp = self.c.get( + reverse("relate-edit_calendar", args=[self.course.identifier])) + self.assertEqual(resp.status_code, 200) + + # menu items + self.assertContains(resp, MENU_EDIT_EVENTS_ADMIN) + self.assertContains(resp, MENU_VIEW_EVENTS_CALENDAR) + self.assertContains(resp, MENU_CREATE_RECURRING_EVENTS) + self.assertContains(resp, MENU_RENUMBER_EVENTS) + + # rendered page html + self.assertNotContains(resp, HTML_SWITCH_TO_EDIT_VIEW) + self.assertContains(resp, HTML_SWITCH_TO_STUDENT_VIEW) + self.assertContains(resp, HTML_CREATE_NEW_EVENT_BUTTON_TITLE) + + # see all events (including hidden ones) + self.assertShownEventsCountEqual(resp, N_TEST_EVENTS) + + def test_non_superuser_instructor_calendar_edit_get(self): + self.c.force_login(self.instructor_participation.user) + resp = self.c.get( + reverse("relate-edit_calendar", args=[self.course.identifier])) + self.assertEqual(resp.status_code, 200) + + # menu items + self.assertNotContains(resp, MENU_EDIT_EVENTS_ADMIN) + self.assertContains(resp, MENU_VIEW_EVENTS_CALENDAR) + self.assertContains(resp, MENU_CREATE_RECURRING_EVENTS) + self.assertContains(resp, MENU_RENUMBER_EVENTS) + + # rendered page html + self.assertNotContains(resp, HTML_SWITCH_TO_EDIT_VIEW) + self.assertContains(resp, HTML_SWITCH_TO_STUDENT_VIEW) + self.assertContains(resp, HTML_CREATE_NEW_EVENT_BUTTON_TITLE) + + # see all events (including hidden ones) + self.assertShownEventsCountEqual(resp, N_TEST_EVENTS) + + def test_student_calendar_edit_get(self): + self.c.force_login(self.student_participation.user) + resp = self.c.get( + reverse("relate-edit_calendar", args=[self.course.identifier])) + self.assertEqual(resp.status_code, 403) + + def test_instructor_calendar_edit_create_exist_failure(self): + self.c.force_login(self.instructor_participation.user) + # Failing to create events already exist + post_data = { + "kind": SHOWN_EVENT_KIND, + "ordinal": "3", + "time": "2017-09-14 17:14", # forgive me + "shown_in_calendar": True, + 'submit': [''] + } + + resp = self.c.post( + reverse("relate-edit_calendar", args=[self.course.identifier]), + post_data + ) + self.assertEqual(resp.status_code, 200) + self.assertContains(resp, MSG_PREFIX_EVENT_ALREAD_EXIST_FAILURE_MSG) + self.assertContains(resp, MSG_EVENT_NOT_CREATED) + self.assertTotalEventsCountEqual(N_TEST_EVENTS) + self.assertShownEventsCountEqual(resp, N_TEST_EVENTS) + + def test_instructor_calendar_edit_post_create_success(self): + # Successfully create new event + self.c.force_login(self.instructor_participation.user) + post_data = { + "kind": SHOWN_EVENT_KIND, + "ordinal": "4", + "time": "2017-09-14 17:14", + "shown_in_calendar": True, + 'submit': [''] + } + resp = self.c.post( + reverse("relate-edit_calendar", args=[self.course.identifier]), + post_data + ) + self.assertEqual(resp.status_code, 200) + self.assertMessageContains(resp, HTML_EVENT_CREATED_MSG) + self.assertTotalEventsCountEqual(N_TEST_EVENTS + 1) + self.assertShownEventsCountEqual(resp, N_TEST_EVENTS + 1) + + def test_instructor_calendar_edit_delete_success(self): + # Successfully remove an existing event + self.c.force_login(self.instructor_participation.user) + id_to_delete = Event.objects.filter(kind=SHOWN_EVENT_KIND).first().id + post_data = { + "id_to_delete": id_to_delete, + 'submit': [''] + } + resp = self.c.post( + reverse("relate-edit_calendar", args=[self.course.identifier]), + post_data + ) + self.assertEqual(resp.status_code, 200) + self.assertMessageContains(resp, MSG_HTML_EVENT_DELETED) + self.assertMessageContains(resp, [MSG_HTML_EVENT_DELETED]) + self.assertTotalEventsCountEqual(N_TEST_EVENTS - 1) + self.assertShownEventsCountEqual(resp, N_TEST_EVENTS - 1) + + def test_instructor_calendar_edit_delete_non_exist(self): + # Successfully remove an existing event + self.c.force_login(self.instructor_participation.user) + post_data = { + "id_to_delete": 1000, # forgive me + 'submit': [''] + } + resp = self.c.post( + reverse("relate-edit_calendar", args=[self.course.identifier]), + post_data + ) + self.assertEqual(resp.status_code, 404) + self.assertTotalEventsCountEqual(N_TEST_EVENTS) + + def test_instructor_calendar_edit_update_success(self): + # Successfully update an existing event + self.c.force_login(self.instructor_participation.user) + all_hidden_events = Event.objects.filter(kind=HIDDEN_EVENT_KIND) + hidden_count_before_update = all_hidden_events.count() + shown_count_before_update = ( + Event.objects.filter(kind=SHOWN_EVENT_KIND).count()) + event_to_edit = all_hidden_events.first() + id_to_edit = event_to_edit.id + post_data = { + "existing_event_to_save": id_to_edit, + "kind": SHOWN_EVENT_KIND, + "ordinal": 10, + "time": "2017-09-14 17:14", # forgive me + "shown_in_calendar": True, + 'submit': [''] + } + resp = self.c.post( + reverse("relate-edit_calendar", args=[self.course.identifier]), + post_data + ) + self.assertEqual(resp.status_code, 200) + self.assertMessageContains(resp, MSG_EVENT_UPDATED) + self.assertTotalEventsCountEqual(N_TEST_EVENTS) + self.assertShownEventsCountEqual(resp, N_TEST_EVENTS) + self.assertEqual( + Event.objects.filter(kind=HIDDEN_EVENT_KIND).count(), + hidden_count_before_update - 1) + self.assertEqual( + Event.objects.filter(kind=SHOWN_EVENT_KIND).count(), + shown_count_before_update + 1) + + def test_instructor_calendar_edit_update_non_exist_id_to_edit_failure(self): + self.c.force_login(self.instructor_participation.user) + id_to_edit = 1000 # forgive me + post_data = { + "id_to_edit": id_to_edit, + } + resp = self.c.post( + reverse("relate-edit_calendar", args=[self.course.identifier]), + post_data + ) + self.assertEqual(resp.status_code, 404) + + post_data = { + "existing_event_to_save": id_to_edit, + "kind": SHOWN_EVENT_KIND, + "ordinal": 1000, + "time": "2017-09-14 17:14", # forgive me + "shown_in_calendar": True, + 'submit': [''] + } + resp = self.c.post( + reverse("relate-edit_calendar", args=[self.course.identifier]), + post_data + ) + self.assertEqual(resp.status_code, 404) + + def test_instructor_calendar_edit_update_overwrite_exist_id_failure(self): + # Failure to update an existing event to overwrite and existing event + self.c.force_login(self.instructor_participation.user) + event_to_edit = Event.objects.filter(kind=HIDDEN_EVENT_KIND).first() + id_to_edit = event_to_edit.id # forgive me + post_data = { + "existing_event_to_save": id_to_edit, + "kind": HIDDEN_EVENT_NO_ORDINAL, + "ordinal": "", + "time": "2017-09-14 17:14", # forgive me + "shown_in_calendar": False, + 'submit': [''] + } + resp = self.c.post( + reverse("relate-edit_calendar", args=[self.course.identifier]), + post_data + ) + self.assertEqual(resp.status_code, 200) + self.assertContains(resp, MSG_PREFIX_EVENT_ALREAD_EXIST_FAILURE_MSG) + self.assertContains(resp, MSG_EVENT_NOT_UPDATED) + self.assertTotalEventsCountEqual(N_TEST_EVENTS) + + def test_no_pperm_edit_event_post_create_fail(self): + self.c.force_login(self.student_participation.user) + post_data = { + "kind": FAILURE_EVENT_KIND, + "ordinal": "1", + "time": "2017-09-14 17:14", # forgive me + "shown_in_calendar": True, + 'submit': [''] + } + + resp = self.c.post( + reverse("relate-edit_calendar", args=[self.course.identifier]), + post_data + ) + self.assertEqual(resp.status_code, 403) + self.assertTotalEventsCountEqual(N_TEST_EVENTS) + self.assertEqual(Event.objects.filter(kind=FAILURE_EVENT_KIND).count(), 0) + + def test_no_pperm_edit_event_post_delete_fail(self): + self.c.force_login(self.student_participation.user) + id_to_delete = Event.objects.filter(kind=SHOWN_EVENT_KIND).first().id + post_data = { + "id_to_delete": id_to_delete, + 'submit': [''] + } + resp = self.c.post( + reverse("relate-edit_calendar", args=[self.course.identifier]), + post_data + ) + self.assertEqual(resp.status_code, 403) + self.assertTotalEventsCountEqual(N_TEST_EVENTS) + + def test_no_pperm_edit_event_post_edit(self): + self.c.force_login(self.student_participation.user) + id_to_edit = 1 + self.assertIsNotNone(Event.objects.get(id=id_to_edit)) + post_data = { + "id_to_edit": id_to_edit, + } + resp = self.c.post( + reverse("relate-edit_calendar", args=[self.course.identifier]), + post_data + ) + self.assertEqual(resp.status_code, 403) + + post_data = { + "existing_event_to_save": id_to_edit, + "kind": FAILURE_EVENT_KIND, + "ordinal": 1000, + "time": "2017-09-14 17:14", # forgive me + "shown_in_calendar": True, + 'submit': [''] + } + resp = self.c.post( + reverse("relate-edit_calendar", args=[self.course.identifier]), + post_data + ) + self.assertEqual(resp.status_code, 403) + self.assertEqual(Event.objects.filter(kind=FAILURE_EVENT_KIND).count(), 0) -- GitLab