From 920d6c40c26c0a0fdd27017a35dd895b58fcaa12 Mon Sep 17 00:00:00 2001 From: Mees Date: Fri, 27 Sep 2019 18:21:35 +0200 Subject: [PATCH 01/64] Small changes to check.stdin en check.stdout, to enable working with very large input/output, and human-unreadable input. --- check50/_api.py | 12 ++++-- tests/check50_tests.py | 41 +++++++++++++++++++ .../checks/stdin_human_readable_py/.cs50.yaml | 2 + tests/checks/stdin_human_readable_py/check.py | 11 +++++ 4 files changed, 63 insertions(+), 3 deletions(-) create mode 100644 tests/checks/stdin_human_readable_py/.cs50.yaml create mode 100644 tests/checks/stdin_human_readable_py/check.py diff --git a/check50/_api.py b/check50/_api.py index 7c6424e4..df612a41 100644 --- a/check50/_api.py +++ b/check50/_api.py @@ -163,12 +163,15 @@ def __init__(self, command, env={}): command = "bash -c {}".format(shlex.quote(command)) self.process = pexpect.spawn(command, encoding="utf-8", echo=False, env=full_env) - def stdin(self, line, prompt=True, timeout=3): + def stdin(self, line, str_line=None, prompt=True, timeout=3): """ Send line to stdin, optionally expect a prompt. :param line: line to be send to stdin :type line: str + :param str_line: what will be displayed as the delivered input, a human \ + readable form of ``line`` + :type str_line: str :param prompt: boolean indicating whether a prompt is expected, if True absorbs \ all of stdout before inserting line into stdin and raises \ :class:`check50.Failure` if stdout is empty @@ -178,10 +181,13 @@ def stdin(self, line, prompt=True, timeout=3): :raises check50.Failure: if ``prompt`` is set to True and no prompt is given """ + if str_line is None: + str_line = line + if line == EOF: log("sending EOF...") else: - log(_("sending input {}...").format(line)) + log(_("sending input {}...").format(str_line)) if prompt: try: @@ -263,7 +269,7 @@ def stdout(self, output=None, str_output=None, regex=True, timeout=3): result += self.process.after raise Mismatch(str_output, result.replace("\r\n", "\n")) except TIMEOUT: - raise Failure(_("did not find {}").format(_raw(str_output))) + raise Failure(_("did not find {} within {} seconds").format(_raw(str_output), timeout)) except UnicodeDecodeError: raise Failure(_("output not valid ASCII text")) except Exception: diff --git a/tests/check50_tests.py b/tests/check50_tests.py index c8a8b2e2..352ef2ab 100644 --- a/tests/check50_tests.py +++ b/tests/check50_tests.py @@ -102,6 +102,17 @@ def test_with_correct_file(self): process.expect_exact("prints hello") process.close(force=True) +class TestStdoutTimeout(Base): + def test_stdout_timeout(self): + with open("foo.py", "w") as f: + f.write("while True: pass") + + process = pexpect.spawn(f"check50 --dev {CHECKS_DIRECTORY}/stdout_py") + process.expect_exact(":)") + process.expect_exact("foo.py exists") + process.expect_exact(":(") + process.expect_exact("did not find \"hello\" within 3 seconds") + process.close(force=True) class TestStdinPy(Base): def test_no_file(self): @@ -230,6 +241,36 @@ def test_with_correct_file(self): process.expect_exact("prints hello name (chaining) (order)") process.close(force=True) +class TestStdinHumanReadable(Base): + def test_without_human_readable_string(self): + with open("foo.py", "w") as f: + f.write('name = input()\nprint("hello", name)') + + process = pexpect.spawn(f"check50 --dev {CHECKS_DIRECTORY}/stdin_py") + process.expect_exact(":)") + process.expect_exact("foo.py exists") + process.expect_exact("checking that foo.py exists...") + process.expect_exact(":)") + process.expect_exact("prints hello name") + process.expect_exact("running python3 foo.py...") + process.expect_exact("sending input bar...") + process.expect_exact("checking for output \"hello bar\"...") + process.close(force=True) + + def test_with_human_readable_string(self): + with open("foo.py", "w") as f: + f.write('name = input("prompt")') + + process = pexpect.spawn(f"check50 --dev {CHECKS_DIRECTORY}/stdin_human_readable_py") + process.expect_exact(":)") + process.expect_exact("foo.py exists") + process.expect_exact("checking that foo.py exists...") + process.expect_exact(":)") + process.expect_exact("takes input") + process.expect_exact("running python3 foo.py...") + process.expect_exact("sending input bbb...") + process.close(force=True) + class TestCompileExit(SimpleBase): compiled_loc = CHECKS_DIRECTORY / "compile_exit" / "__init__.py" diff --git a/tests/checks/stdin_human_readable_py/.cs50.yaml b/tests/checks/stdin_human_readable_py/.cs50.yaml new file mode 100644 index 00000000..0745d8da --- /dev/null +++ b/tests/checks/stdin_human_readable_py/.cs50.yaml @@ -0,0 +1,2 @@ +check50: + checks: check.py diff --git a/tests/checks/stdin_human_readable_py/check.py b/tests/checks/stdin_human_readable_py/check.py new file mode 100644 index 00000000..0e44e8ca --- /dev/null +++ b/tests/checks/stdin_human_readable_py/check.py @@ -0,0 +1,11 @@ +import check50 + +@check50.check() +def exists(): + """foo.py exists""" + check50.exists("foo.py") + +@check50.check(exists) +def takes_input(): + """takes input""" + check50.run("python3 foo.py").stdin("aaa", prompt=False, str_line="bbb") From 4542ee2c17af4e64adacb89631b0244109d33caa Mon Sep 17 00:00:00 2001 From: jelleas Date: Thu, 30 Apr 2020 15:23:30 +0200 Subject: [PATCH 02/64] optionally display timeouts --- check50/_api.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/check50/_api.py b/check50/_api.py index df612a41..65bac5dd 100644 --- a/check50/_api.py +++ b/check50/_api.py @@ -213,7 +213,7 @@ def stdin(self, line, str_line=None, prompt=True, timeout=3): pass return self - def stdout(self, output=None, str_output=None, regex=True, timeout=3): + def stdout(self, output=None, str_output=None, regex=True, timeout=3, show_timeout=False): """ Retrieve all output from stdout until timeout (3 sec by default). If ``output`` is None, ``stdout`` returns all of the stdout outputted by the process, else @@ -229,6 +229,9 @@ def stdout(self, output=None, str_output=None, regex=True, timeout=3): :type regex: bool :param timeout: maximum number of seconds to wait for ``output`` :type timeout: int / float + :param show_timeout: flag indicating whether the timeout in seconds \ + should be displayed when a timeout occurs + :type show_timeout: bool :raises check50.Mismatch: if ``output`` is specified and nothing that the \ process outputs matches it :raises check50.Failure: if process times out or if it outputs invalid UTF-8 text. @@ -269,7 +272,9 @@ def stdout(self, output=None, str_output=None, regex=True, timeout=3): result += self.process.after raise Mismatch(str_output, result.replace("\r\n", "\n")) except TIMEOUT: - raise Failure(_("did not find {} within {} seconds").format(_raw(str_output), timeout)) + if show_timeout: + raise Failure(_("did not find {} within {} seconds").format(_raw(str_output), timeout)) + raise Failure(_("did not find {}").format(_raw(str_output))) except UnicodeDecodeError: raise Failure(_("output not valid ASCII text")) except Exception: From c65495e0bc4c8738d356d322382806d0beeb9955 Mon Sep 17 00:00:00 2001 From: jelleas Date: Thu, 30 Apr 2020 16:41:27 +0200 Subject: [PATCH 03/64] your => the --- check50/_api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/check50/_api.py b/check50/_api.py index de723253..22425a71 100644 --- a/check50/_api.py +++ b/check50/_api.py @@ -274,7 +274,7 @@ def stdout(self, output=None, str_output=None, regex=True, timeout=3, show_timeo except TIMEOUT: if show_timeout: raise Missing(str_output, self.process.before, - help=_("check50 waited {} seconds for the output of your program").format(timeout)) + help=_("check50 waited {} seconds for the output of the program").format(timeout)) raise Missing(str_output, self.process.before) except UnicodeDecodeError: raise Failure(_("output not valid ASCII text")) From 7f69760c7a46f92de5dca992ed6dcc817f779905 Mon Sep 17 00:00:00 2001 From: jelleas Date: Mon, 11 May 2020 14:57:44 +0200 Subject: [PATCH 04/64] rm --log, hardcore options --- check50/__main__.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/check50/__main__.py b/check50/__main__.py index 20b2a125..bb5b2f2d 100644 --- a/check50/__main__.py +++ b/check50/__main__.py @@ -267,9 +267,6 @@ def main(): parser.add_argument("-l", "--local", action="store_true", help=_("run checks locally instead of uploading to cs50")) - parser.add_argument("--log", - action="store_true", - help=_("display more detailed information about check results")) parser.add_argument("-o", "--output", action="store", nargs="+", @@ -289,7 +286,7 @@ def main(): nargs="?", default="", const="info", - choices=[attr for attr in dir(logging) if attr.isupper() and isinstance(getattr(logging, attr), int)], + choices=["DEBUG", "INFO", "WARNING", "ERROR", "CRITICAL"], type=str.upper, help=_("sets the verbosity level." ' "INFO" displays the full tracebacks of errors and shows all commands run.' @@ -414,7 +411,7 @@ def main(): output_file.write(renderer.to_json(**results)) output_file.write("\n") elif output == "ansi": - output_file.write(renderer.to_ansi(**results, log=args.log)) + output_file.write(renderer.to_ansi(**results, log=False)) output_file.write("\n") elif output == "html": if os.environ.get("CS50_IDE_TYPE") and args.local: From 5cddbd839e1d132b1f23eccd0d198eee05ab62e7 Mon Sep 17 00:00:00 2001 From: jelleas Date: Mon, 11 May 2020 15:31:47 +0200 Subject: [PATCH 05/64] --verbose AND --log-level --- check50/__main__.py | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/check50/__main__.py b/check50/__main__.py index bb5b2f2d..0db8def6 100644 --- a/check50/__main__.py +++ b/check50/__main__.py @@ -259,7 +259,7 @@ def main(): parser.add_argument("slug", help=_("prescribed identifier of work to check")) parser.add_argument("-d", "--dev", action="store_true", - help=_("run check50 in development mode (implies --offline and --verbose info).\n" + help=_("run check50 in development mode (implies --offline, --verbose, and --log-level INFO).\n" "causes SLUG to be interpreted as a literal path to a checks package")) parser.add_argument("--offline", action="store_true", @@ -282,14 +282,16 @@ def main(): metavar="FILE", help=_("file to write output to")) parser.add_argument("-v", "--verbose", + action="store_true", + help=_("shows the full traceback of any errors")) + parser.add_argument("--log-level", action="store", - nargs="?", + nargs=1, default="", - const="info", choices=["DEBUG", "INFO", "WARNING", "ERROR", "CRITICAL"], type=str.upper, - help=_("sets the verbosity level." - ' "INFO" displays the full tracebacks of errors and shows all commands run.' + help=_("sets the log level." + ' "INFO" shows all commands run.' ' "DEBUG" adds the output of all command run.')) parser.add_argument("--no-download-checks", action="store_true", @@ -307,11 +309,12 @@ def main(): global SLUG SLUG = args.slug - # dev implies offline and verbose "info" if not overwritten + # dev implies offline, verbose, and log level "INFO" if not overwritten if args.dev: args.offline = True - if not args.verbose: - args.verbose = "info" + args.verbose = True + if not args.log_level: + args.log_level = "INFO" # offline implies local if args.offline: @@ -319,8 +322,8 @@ def main(): args.no_download_checks = True args.local = True - # Setup logging for lib50 depending on verbosity level - setup_logging(args.verbose) + # Setup logging for lib50 + setup_logging(args.log_level if args.log_level else "CRITICAL") # Warning in case of running remotely with no_download_checks or no_install_dependencies set if not args.local: @@ -339,7 +342,7 @@ def main(): args.output = [output for output in args.output if not (output in seen_output or seen_output.add(output))] # Set excepthook - excepthook.verbose = bool(args.verbose) + excepthook.verbose = args.verbose excepthook.outputs = args.output excepthook.output_file = args.output_file @@ -382,9 +385,10 @@ def main(): # Have lib50 decide which files to include included = lib50.files(config.get("files"))[0] - with open(os.devnull, "w") if args.verbose else nullcontext() as devnull: - # Redirect stdout to devnull if some verbosity level is set - if args.verbose: + # Redirect stdout to devnull if verbose or log level is set + should_redirect_devnull = args.verbose or args.log_level + with open(os.devnull, "w") if should_redirect_devnull else nullcontext() as devnull: + if should_redirect_devnull: stdout = stderr = devnull else: stdout = sys.stdout From e3503c91b4d1d378fdbb0ad73972ff624bb891ec Mon Sep 17 00:00:00 2001 From: jelleas Date: Tue, 12 May 2020 21:03:25 +0200 Subject: [PATCH 06/64] find/replace --log --- check50/c.py | 2 +- check50/flask.py | 2 +- check50/py.py | 3 +-- docs/source/check50_user.rst | 20 ++++++++++---------- 4 files changed, 13 insertions(+), 14 deletions(-) diff --git a/check50/c.py b/check50/c.py index 26b195a5..c63f06b6 100644 --- a/check50/c.py +++ b/check50/c.py @@ -130,4 +130,4 @@ def _check_valgrind(xml_file): # Only raise exception if we encountered errors. if reported: - raise Failure(_("valgrind tests failed; rerun with --log for more information.")) + raise Failure(_("valgrind tests failed; see the web UI for more information.")) diff --git a/check50/flask.py b/check50/flask.py index 5944bfc6..d330758a 100644 --- a/check50/flask.py +++ b/check50/flask.py @@ -142,7 +142,7 @@ def _send(self, method, route, data, params, **kwargs): self.response = getattr(self._client, method.lower())(route, data=data, **kwargs) except BaseException as e: # Catch all exceptions thrown by app log(_("exception raised in application: {}: {}").format(type(e).__name__, e)) - raise Failure(_("application raised an exception (rerun with --log for more details)")) + raise Failure(_("application raised an exception (see the web UI for more details)")) return self def _search_page(self, output, str_output, content, match_fn, **kwargs): diff --git a/check50/py.py b/check50/py.py index 66f6ed89..564d0c28 100644 --- a/check50/py.py +++ b/check50/py.py @@ -65,5 +65,4 @@ def compile(file): for line in e.msg.splitlines(): log(line) - raise Failure(_("{} raised while compiling {} (rerun with --log for more details)").format(e.exc_type_name, file)) - + raise Failure(_("{} raised while compiling {} (see the web UI for more details)").format(e.exc_type_name, file)) diff --git a/docs/source/check50_user.rst b/docs/source/check50_user.rst index 69e6b908..7ed0f6c4 100644 --- a/docs/source/check50_user.rst +++ b/docs/source/check50_user.rst @@ -26,19 +26,19 @@ Operation modes Check50 can run in four mutually exclusive modes of operation. ********************** -local +online ********************** -By default check50 runs locally. That means the checks run locally on the machine you run check50 on. The checks however are fetched remotely from GitHub. +By default check50 runs the checks remotely and then waits for the results to come back. ********************** -offline +local ********************** -Running with :code:`--offline` runs the checks locally and has check50 look for checks locally. check50 will not try to fetch checks remotely in this mode. +To run checks locally, pass the :code:`--local` flag. The checks are still fetched remotely from GitHub. ********************** -online +offline ********************** -Running with :code:`--online` runs the checks remotely and then waits for the results to come back. +Running with :code:`--offline` runs the checks locally and has check50 look for checks locally. check50 will not try to fetch checks remotely in this mode. ********************** dev @@ -53,12 +53,12 @@ By default check50 will try to keep its output concise in its :code:`ansi` outpu ********************** verbose ********************** -Running with :code:`--verbose` lets check50 output both the log and any tracebacks in the :code:`ansi` output mode. +Running with :code:`--verbose` lets check50 output any tracebacks in the :code:`ansi` output mode. ********************** -log +log-level ********************** -Running check50 with :code:`--log` will have check50 print out its logs. +Running check50 with :code:`--log-level INFO` will display any git commands run. :code:`--log-level DEBUG` adds all output of any git commands run. Targeting checks @@ -68,7 +68,7 @@ Check50 lets you target specific checks by name with the :code:`--target` flags. ********************** target ********************** -With :code:`--target` you can target checks from a larger body of checks by name. check50 will only run and show these checks and their dependencies. +With :code:`--target` you can target checks from a larger body of checks by name. check50 will only run and show these checks and their dependencies. Output modes From e1d6a32871aa8afa0ba04391cf0c057abd07278c Mon Sep 17 00:00:00 2001 From: jelleas Date: Wed, 13 May 2020 23:05:07 +0200 Subject: [PATCH 07/64] no default log level, rm nargs=1 --- check50/__main__.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/check50/__main__.py b/check50/__main__.py index 0db8def6..a2620f41 100644 --- a/check50/__main__.py +++ b/check50/__main__.py @@ -286,7 +286,6 @@ def main(): help=_("shows the full traceback of any errors")) parser.add_argument("--log-level", action="store", - nargs=1, default="", choices=["DEBUG", "INFO", "WARNING", "ERROR", "CRITICAL"], type=str.upper, @@ -323,7 +322,7 @@ def main(): args.local = True # Setup logging for lib50 - setup_logging(args.log_level if args.log_level else "CRITICAL") + setup_logging(args.log_level) # Warning in case of running remotely with no_download_checks or no_install_dependencies set if not args.local: From bbf3a6779bdfff36fec3af454c6e08fc5baa619a Mon Sep 17 00:00:00 2001 From: Chad Sharp Date: Wed, 13 May 2020 17:27:53 -0400 Subject: [PATCH 08/64] add check50 logger, colored logging --- check50/__main__.py | 159 +++++++++++++++++++++------------ check50/renderer/_renderers.py | 4 +- 2 files changed, 105 insertions(+), 58 deletions(-) diff --git a/check50/__main__.py b/check50/__main__.py index 0db8def6..07508c52 100644 --- a/check50/__main__.py +++ b/check50/__main__.py @@ -1,5 +1,6 @@ import argparse import contextlib +import enum import gettext import importlib import inspect @@ -25,9 +26,33 @@ from . import internal, renderer, __version__ from .runner import CheckRunner +LOGGER = logging.getLogger("check50") + lib50.set_local_path(os.environ.get("CHECK50_PATH", "~/.local/share/check50")) -SLUG = None + +class LogLevel(enum.IntEnum): + ERROR = enum.auto() + WARNING = enum.auto() + INFO = enum.auto() + DEBUG = enum.auto() + + +class ColoredFormatter(logging.Formatter): + COLORS = { + "ERROR": "red", + "WARNING": "yellow", + "DEBUG": "cyan", + "INFO": "magenta", + } + + def __init__(self, fmt, use_color=True): + logging.Formatter.__init__(self, fmt=fmt) + self.use_color = use_color + + def format(self, record): + msg = super().format(record) + return msg if not self.use_color else termcolor.colored(msg, getattr(record, "color", self.COLORS.get(record.levelname))) class RemoteCheckError(internal.Error): @@ -52,7 +77,7 @@ def excepthook(cls, exc, tb): ctxmanager = open(excepthook.output_file, "w") if excepthook.output_file else nullcontext(sys.stdout) with ctxmanager as output_file: json.dump({ - "slug": SLUG, + "slug": excepthook.slug, "error": { "type": cls.__name__, "value": str(exc), @@ -165,20 +190,20 @@ def setup_logging(level): level 'info' logs all git commands run to stderr level 'debug' logs all git commands and their output to stderr """ - # No verbosity level set, don't log anything - if not level: - return - lib50_logger = logging.getLogger("lib50") + for logger in (logging.getLogger("lib50"), LOGGER): + # Set verbosity level on the lib50 logger + logger.setLevel(getattr(logging, level.name)) - # Set verbosity level on the lib50 logger - lib50_logger.setLevel(getattr(logging, level.upper())) + handler = logging.StreamHandler(sys.stderr) + handler.setFormatter(ColoredFormatter("(%(levelname)s) %(message)s")) - # Direct all logs to sys.stderr - lib50_logger.addHandler(logging.StreamHandler(sys.stderr)) + # Direct all logs to sys.stderr + logger.addHandler(handler) # Don't animate the progressbar - lib50.ProgressBar.DISABLED = True + if level > LogLevel.WARNING: + lib50.ProgressBar.DISABLED = True def await_results(commit_hash, slug, pings=45, sleep=2): @@ -253,6 +278,53 @@ def raise_invalid_slug(slug, offline=False): raise internal.Error(msg) +def validate_args(args): + """Validate arguments and apply defaults that are dependent on other arguments""" + + # dev implies offline, verbose, and log level "INFO" if not overwritten + if args.dev: + args.offline = True + args.verbose = True + args.log_level = max(args.log_level, LogLevel.INFO) + + # offline implies local + if args.offline: + args.no_install_dependencies = True + args.no_download_checks = True + args.local = True + + if not args.log_level: + args.log_level = LogLevel.WARNING + + # Setup logging for lib50 + setup_logging(args.log_level) + + # Warning in case of running remotely with no_download_checks or no_install_dependencies set + if not args.local: + useless_args = [] + if args.no_download_checks: + useless_args.append("--no-downloads-checks") + if args.no_install_dependencies: + useless_args.append("--no-install-dependencies") + + if useless_args: + LOGGER.warning(_("You should always use --local when using: {}").format(", ".join(useless_args))) + + # Filter out any duplicates from args.output + seen_output = [] + for output in args.output: + if output in seen_output: + LOGGER.warning(_("Duplicate output format specified: {}").format(output)) + else: + seen_output.append(output) + + args.output = seen_output + + if args.ansi_log and "ansi" not in seen_output: + LOGGER.warning(_("--ansi-log has no effect when ansi is not among the output formats")) + + + def main(): parser = argparse.ArgumentParser(prog="check50") @@ -286,13 +358,16 @@ def main(): help=_("shows the full traceback of any errors")) parser.add_argument("--log-level", action="store", - nargs=1, - default="", - choices=["DEBUG", "INFO", "WARNING", "ERROR", "CRITICAL"], - type=str.upper, + default="WARNING", + choices=list(LogLevel.__members__), + type=lambda l: LogLevel.__members__[l.upper()], help=_("sets the log level." + ' "WARNING" displays usage warnings' ' "INFO" shows all commands run.' ' "DEBUG" adds the output of all command run.')) + parser.add_argument("--ansi-log", + action="store_true", + help=_("display log in ansi output mode")) parser.add_argument("--no-download-checks", action="store_true", help=_("do not download checks, but use previously downloaded checks instead (only works with --local)")) @@ -306,67 +381,37 @@ def main(): args = parser.parse_args() - global SLUG - SLUG = args.slug + # Validate arguments and apply defaults + validate_args(args) - # dev implies offline, verbose, and log level "INFO" if not overwritten - if args.dev: - args.offline = True - args.verbose = True - if not args.log_level: - args.log_level = "INFO" - - # offline implies local - if args.offline: - args.no_install_dependencies = True - args.no_download_checks = True - args.local = True - - # Setup logging for lib50 - setup_logging(args.log_level if args.log_level else "CRITICAL") - - # Warning in case of running remotely with no_download_checks or no_install_dependencies set - if not args.local: - useless_args = [] - if args.no_download_checks: - useless_args.append("--no-downloads-checks") - if args.no_install_dependencies: - useless_args.append("--no-install-dependencies") - - if useless_args: - termcolor.cprint(_("Warning: you should always use --local when using: {}".format(", ".join(useless_args))), - "yellow", attrs=["bold"]) - - # Filter out any duplicates from args.output - seen_output = set() - args.output = [output for output in args.output if not (output in seen_output or seen_output.add(output))] # Set excepthook excepthook.verbose = args.verbose excepthook.outputs = args.output excepthook.output_file = args.output_file + excepthook.slug = args.slug # If remote, push files to GitHub and await results if not args.local: - commit_hash = lib50.push("check50", SLUG, internal.CONFIG_LOADER, data={"check50": True})[1] + commit_hash = lib50.push("check50", args.slug, internal.CONFIG_LOADER, data={"check50": True})[1] with lib50.ProgressBar("Waiting for results") if "ansi" in args.output else nullcontext(): - tag_hash, results = await_results(commit_hash, SLUG) + tag_hash, results = await_results(commit_hash, args.slug) # Otherwise run checks locally else: with lib50.ProgressBar("Checking") if "ansi" in args.output else nullcontext(): # If developing, assume slug is a path to check_dir if args.dev: - internal.check_dir = Path(SLUG).expanduser().resolve() + internal.check_dir = Path(args.slug).expanduser().resolve() if not internal.check_dir.is_dir(): raise internal.Error(_("{} is not a directory").format(internal.check_dir)) # Otherwise have lib50 create a local copy of slug else: try: - internal.check_dir = lib50.local(SLUG, offline=args.no_download_checks) + internal.check_dir = lib50.local(args.slug, offline=args.no_download_checks) except lib50.ConnectionError: - raise internal.Error(_("check50 could not retrieve checks from GitHub. Try running check50 again with --offline.").format(SLUG)) + raise internal.Error(_("check50 could not retrieve checks from GitHub. Try running check50 again with --offline.").format()) except lib50.InvalidSlugError: - raise_invalid_slug(SLUG, offline=args.no_download_checks) + raise_invalid_slug(args.slug, offline=args.no_download_checks) # Load config config = internal.load_config(internal.check_dir) @@ -401,12 +446,14 @@ def main(): check_results = CheckRunner(checks_file).run(included, working_area, args.target) results = { - "slug": SLUG, + "slug": args.slug, "results": [attr.asdict(result) for result in check_results], "version": __version__ } + LOGGER.debug(results) + # Render output file_manager = open(args.output_file, "w") if args.output_file else nullcontext(sys.stdout) with file_manager as output_file: @@ -415,7 +462,7 @@ def main(): output_file.write(renderer.to_json(**results)) output_file.write("\n") elif output == "ansi": - output_file.write(renderer.to_ansi(**results, log=False)) + output_file.write(renderer.to_ansi(**results, _log=args.ansi_log)) output_file.write("\n") elif output == "html": if os.environ.get("CS50_IDE_TYPE") and args.local: diff --git a/check50/renderer/_renderers.py b/check50/renderer/_renderers.py index 53512dd2..bbbb43ef 100644 --- a/check50/renderer/_renderers.py +++ b/check50/renderer/_renderers.py @@ -23,7 +23,7 @@ def to_json(slug, results, version): return json.dumps({"slug": slug, "results": results, "version": version}, indent=4) -def to_ansi(slug, results, version, log=False): +def to_ansi(slug, results, version, _log=False): lines = [termcolor.colored(_("Results for {} generated by check50 v{}").format(slug, version), "white", attrs=["bold"])] for result in results: if result["passed"]: @@ -41,7 +41,7 @@ def to_ansi(slug, results, version, log=False): if result["cause"].get("help") is not None: lines.append(termcolor.colored(f" {result['cause']['help']}", "red")) - if log: + if _log: lines += (f" {line}" for line in result["log"]) return "\n".join(lines) From eb5e1f3392614d3e03e215f9b0a71c03a457d56d Mon Sep 17 00:00:00 2001 From: jelleas Date: Wed, 13 May 2020 23:29:10 +0200 Subject: [PATCH 09/64] check50 logger --- check50/__main__.py | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/check50/__main__.py b/check50/__main__.py index a2620f41..651800d5 100644 --- a/check50/__main__.py +++ b/check50/__main__.py @@ -161,24 +161,36 @@ def compile_checks(checks, prompt=False): def setup_logging(level): """ - Sets up logging for lib50. + Sets up logging for check50 and lib50. level 'info' logs all git commands run to stderr level 'debug' logs all git commands and their output to stderr """ - # No verbosity level set, don't log anything + # Setup default check50 logger + logger = logging.getLogger('check50') + logger.addHandler(logging.StreamHandler(sys.stderr)) + logger.setLevel("WARNING") + + # If no custom logger level set, stop if not level: return + level = level.upper() + + # Overwrite check50 logger default + logger.setLevel(level) + + # Setup lib50 logger lib50_logger = logging.getLogger("lib50") # Set verbosity level on the lib50 logger - lib50_logger.setLevel(getattr(logging, level.upper())) + lib50_logger.setLevel(level) # Direct all logs to sys.stderr lib50_logger.addHandler(logging.StreamHandler(sys.stderr)) - # Don't animate the progressbar - lib50.ProgressBar.DISABLED = True + # Don't animate the progressbar in case lib50 logs git commands + if getattr(logging, level) <= logging.INFO: + lib50.ProgressBar.DISABLED = True def await_results(commit_hash, slug, pings=45, sleep=2): @@ -333,8 +345,8 @@ def main(): useless_args.append("--no-install-dependencies") if useless_args: - termcolor.cprint(_("Warning: you should always use --local when using: {}".format(", ".join(useless_args))), - "yellow", attrs=["bold"]) + warning_msg = "Warning: you should always use --local when using: {}".format(", ".join(useless_args)) + logging.getLogger("check50").warning(termcolor.colored(warning_msg, "yellow", attrs=["bold"])) # Filter out any duplicates from args.output seen_output = set() From c57b3c811bf42f36108cc61ca6ba5b06c3f3e1cc Mon Sep 17 00:00:00 2001 From: jelleas Date: Wed, 13 May 2020 23:31:15 +0200 Subject: [PATCH 10/64] gettext --- check50/__main__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/check50/__main__.py b/check50/__main__.py index 651800d5..6a07e374 100644 --- a/check50/__main__.py +++ b/check50/__main__.py @@ -345,7 +345,7 @@ def main(): useless_args.append("--no-install-dependencies") if useless_args: - warning_msg = "Warning: you should always use --local when using: {}".format(", ".join(useless_args)) + warning_msg = _("Warning: you should always use --local when using: {}").format(", ".join(useless_args)) logging.getLogger("check50").warning(termcolor.colored(warning_msg, "yellow", attrs=["bold"])) # Filter out any duplicates from args.output From 3a053fc361e6c03a939417792f1b15426e629dd3 Mon Sep 17 00:00:00 2001 From: Chad Sharp Date: Wed, 13 May 2020 17:45:14 -0400 Subject: [PATCH 11/64] log level >= INFO implies --ansi-log --- check50/__main__.py | 12 +++++++----- check50/c.py | 2 +- check50/flask.py | 2 +- check50/py.py | 2 +- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/check50/__main__.py b/check50/__main__.py index eb5cbf13..e713c80d 100644 --- a/check50/__main__.py +++ b/check50/__main__.py @@ -42,8 +42,8 @@ class ColoredFormatter(logging.Formatter): COLORS = { "ERROR": "red", "WARNING": "yellow", - "DEBUG": "cyan", - "INFO": "magenta", + "INFO": "cyan", + "DEBUG": "magenta", } def __init__(self, fmt, use_color=True): @@ -320,11 +320,13 @@ def validate_args(args): args.output = seen_output - if args.ansi_log and "ansi" not in seen_output: + if "ansi" in args.output: + if args.log_level >= LogLevel.INFO: + args.ansi_log = True + elif args.ansi_log: LOGGER.warning(_("--ansi-log has no effect when ansi is not among the output formats")) - def main(): parser = argparse.ArgumentParser(prog="check50") @@ -359,7 +361,7 @@ def main(): parser.add_argument("--log-level", action="store", default="WARNING", - choices=list(LogLevel.__members__), + choices=list(LogLevel.__members__.values()), type=lambda l: LogLevel.__members__[l.upper()], help=_("sets the log level." ' "WARNING" displays usage warnings' diff --git a/check50/c.py b/check50/c.py index c63f06b6..c48a0277 100644 --- a/check50/c.py +++ b/check50/c.py @@ -130,4 +130,4 @@ def _check_valgrind(xml_file): # Only raise exception if we encountered errors. if reported: - raise Failure(_("valgrind tests failed; see the web UI for more information.")) + raise Failure(_("valgrind tests failed; see log for more information.")) diff --git a/check50/flask.py b/check50/flask.py index d330758a..3ee266db 100644 --- a/check50/flask.py +++ b/check50/flask.py @@ -142,7 +142,7 @@ def _send(self, method, route, data, params, **kwargs): self.response = getattr(self._client, method.lower())(route, data=data, **kwargs) except BaseException as e: # Catch all exceptions thrown by app log(_("exception raised in application: {}: {}").format(type(e).__name__, e)) - raise Failure(_("application raised an exception (see the web UI for more details)")) + raise Failure(_("application raised an exception (see the log for more details)")) return self def _search_page(self, output, str_output, content, match_fn, **kwargs): diff --git a/check50/py.py b/check50/py.py index 564d0c28..9269d5f2 100644 --- a/check50/py.py +++ b/check50/py.py @@ -65,4 +65,4 @@ def compile(file): for line in e.msg.splitlines(): log(line) - raise Failure(_("{} raised while compiling {} (see the web UI for more details)").format(e.exc_type_name, file)) + raise Failure(_("{} raised while compiling {} (see the log for more details)").format(e.exc_type_name, file)) From a84176684e1abd33acc06ad0d71afa15994d5303 Mon Sep 17 00:00:00 2001 From: Chad Sharp Date: Wed, 13 May 2020 17:58:30 -0400 Subject: [PATCH 12/64] flip LogLevel ordering --- check50/__main__.py | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/check50/__main__.py b/check50/__main__.py index e713c80d..4f27084e 100644 --- a/check50/__main__.py +++ b/check50/__main__.py @@ -32,10 +32,10 @@ class LogLevel(enum.IntEnum): - ERROR = enum.auto() - WARNING = enum.auto() - INFO = enum.auto() - DEBUG = enum.auto() + DEBUG = logging.DEBUG + INFO = logging.INFO + WARNING = logging.WARNING + ERROR = logging.ERROR class ColoredFormatter(logging.Formatter): @@ -196,14 +196,13 @@ def setup_logging(level): logger.setLevel(getattr(logging, level.name)) handler = logging.StreamHandler(sys.stderr) - handler.setFormatter(ColoredFormatter("(%(levelname)s) %(message)s")) - + handler.setFormatter(ColoredFormatter("(%(levelname)s) %(message)s", use_color=sys.stderr.isatty())) + # Direct all logs to sys.stderr logger.addHandler(handler) # Don't animate the progressbar - if level > LogLevel.WARNING: - lib50.ProgressBar.DISABLED = True + lib50.ProgressBar.DISABLED = level < LogLevel.WARNING def await_results(commit_hash, slug, pings=45, sleep=2): @@ -281,11 +280,13 @@ def raise_invalid_slug(slug, offline=False): def validate_args(args): """Validate arguments and apply defaults that are dependent on other arguments""" + args.log_level = LogLevel.__members__[args.log_level.upper()] + # dev implies offline, verbose, and log level "INFO" if not overwritten if args.dev: args.offline = True args.verbose = True - args.log_level = max(args.log_level, LogLevel.INFO) + args.log_level = min(args.log_level, LogLevel.INFO) # offline implies local if args.offline: @@ -293,9 +294,6 @@ def validate_args(args): args.no_download_checks = True args.local = True - if not args.log_level: - args.log_level = LogLevel.WARNING - # Setup logging for lib50 setup_logging(args.log_level) @@ -321,7 +319,7 @@ def validate_args(args): args.output = seen_output if "ansi" in args.output: - if args.log_level >= LogLevel.INFO: + if args.log_level <= LogLevel.INFO: args.ansi_log = True elif args.ansi_log: LOGGER.warning(_("--ansi-log has no effect when ansi is not among the output formats")) @@ -346,6 +344,7 @@ def main(): nargs="+", default=["ansi", "html"], choices=["ansi", "json", "html"], + type=str.lower, help=_("format of check results")) parser.add_argument("--target", action="store", @@ -361,8 +360,8 @@ def main(): parser.add_argument("--log-level", action="store", default="WARNING", - choices=list(LogLevel.__members__.values()), - type=lambda l: LogLevel.__members__[l.upper()], + choices=list(LogLevel.__members__), + type=str.upper, help=_("sets the log level." ' "WARNING" displays usage warnings' ' "INFO" shows all commands run.' From 6de9961e007599b7c53d4e93b57c3373676b877c Mon Sep 17 00:00:00 2001 From: Chad Sharp Date: Wed, 13 May 2020 18:05:30 -0400 Subject: [PATCH 13/64] don't overwrite log-level if set in dev mode --- check50/__main__.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/check50/__main__.py b/check50/__main__.py index 4f27084e..e494e03e 100644 --- a/check50/__main__.py +++ b/check50/__main__.py @@ -280,13 +280,14 @@ def raise_invalid_slug(slug, offline=False): def validate_args(args): """Validate arguments and apply defaults that are dependent on other arguments""" - args.log_level = LogLevel.__members__[args.log_level.upper()] + # Default log_level + default_level = "WARNING" # dev implies offline, verbose, and log level "INFO" if not overwritten if args.dev: args.offline = True args.verbose = True - args.log_level = min(args.log_level, LogLevel.INFO) + defualt_level = "INFO" # offline implies local if args.offline: @@ -294,6 +295,9 @@ def validate_args(args): args.no_download_checks = True args.local = True + + args.log_level = LogLevel.__members__[args.log_level.upper() or default_level] + # Setup logging for lib50 setup_logging(args.log_level) @@ -359,11 +363,11 @@ def main(): help=_("shows the full traceback of any errors")) parser.add_argument("--log-level", action="store", - default="WARNING", choices=list(LogLevel.__members__), + default="", type=str.upper, help=_("sets the log level." - ' "WARNING" displays usage warnings' + ' "WARNING" (default) displays usage warnings' ' "INFO" shows all commands run.' ' "DEBUG" adds the output of all command run.')) parser.add_argument("--ansi-log", From 657a10d01c60372e0a5dff8a4a3e6a62e4ee3283 Mon Sep 17 00:00:00 2001 From: Chad Sharp Date: Wed, 13 May 2020 18:16:06 -0400 Subject: [PATCH 14/64] dev doens't imply log-level info, just ansi-log --- check50/__main__.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/check50/__main__.py b/check50/__main__.py index e494e03e..a5635936 100644 --- a/check50/__main__.py +++ b/check50/__main__.py @@ -201,7 +201,7 @@ def setup_logging(level): # Direct all logs to sys.stderr logger.addHandler(handler) - # Don't animate the progressbar + # Don't animate the progressbar if loglevel is info or debug lib50.ProgressBar.DISABLED = level < LogLevel.WARNING @@ -280,14 +280,12 @@ def raise_invalid_slug(slug, offline=False): def validate_args(args): """Validate arguments and apply defaults that are dependent on other arguments""" - # Default log_level - default_level = "WARNING" - # dev implies offline, verbose, and log level "INFO" if not overwritten if args.dev: args.offline = True args.verbose = True - defualt_level = "INFO" + if "ansi" in args.output: + args.ansi_output = True # offline implies local if args.offline: @@ -296,7 +294,7 @@ def validate_args(args): args.local = True - args.log_level = LogLevel.__members__[args.log_level.upper() or default_level] + args.log_level = LogLevel.__members__[args.log_level.upper()] # Setup logging for lib50 setup_logging(args.log_level) @@ -364,7 +362,7 @@ def main(): parser.add_argument("--log-level", action="store", choices=list(LogLevel.__members__), - default="", + default="WARNING", type=str.upper, help=_("sets the log level." ' "WARNING" (default) displays usage warnings' From 92a3ab8297062a1a68f37b1755be9ab22574dd0e Mon Sep 17 00:00:00 2001 From: jelleas Date: Thu, 14 May 2020 12:05:42 +0200 Subject: [PATCH 15/64] pretty_print in formatter, redir dev/null if NOT verbose --- check50/__main__.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/check50/__main__.py b/check50/__main__.py index a5635936..1cc87b8f 100644 --- a/check50/__main__.py +++ b/check50/__main__.py @@ -8,6 +8,7 @@ import json import logging import os +import pprint import site from pathlib import Path import shutil @@ -49,8 +50,11 @@ class ColoredFormatter(logging.Formatter): def __init__(self, fmt, use_color=True): logging.Formatter.__init__(self, fmt=fmt) self.use_color = use_color + self.pretty_printer = pprint.PrettyPrinter(indent=1, compact=True) def format(self, record): + if not isinstance(record.msg, str): + record.msg = self.pretty_printer.pformat(record.msg) msg = super().format(record) return msg if not self.use_color else termcolor.colored(msg, getattr(record, "color", self.COLORS.get(record.levelname))) @@ -358,7 +362,7 @@ def main(): help=_("file to write output to")) parser.add_argument("-v", "--verbose", action="store_true", - help=_("shows the full traceback of any errors")) + help=_("shows the full traceback of any errors and shows print statements written in checks")) parser.add_argument("--log-level", action="store", choices=list(LogLevel.__members__), @@ -432,14 +436,13 @@ def main(): # Have lib50 decide which files to include included = lib50.files(config.get("files"))[0] - # Redirect stdout to devnull if verbose or log level is set - should_redirect_devnull = args.verbose or args.log_level - with open(os.devnull, "w") if should_redirect_devnull else nullcontext() as devnull: - if should_redirect_devnull: - stdout = stderr = devnull - else: + # Redirect stdout/stderr to devnull if not verbose + with nullcontext() if args.verbose else open(os.devnull, "w") as devnull: + if args.verbose: stdout = sys.stdout stderr = sys.stderr + else: + stdout = stderr = devnull # Create a working_area (temp dir) named - with all included student files with lib50.working_area(included, name='-') as working_area, \ @@ -453,7 +456,7 @@ def main(): "version": __version__ } - + # Log results in debug LOGGER.debug(results) # Render output From 85a768f99c84843a6cb161e2e2a356d18efd7e54 Mon Sep 17 00:00:00 2001 From: jelleas Date: Thu, 28 May 2020 16:07:36 +0200 Subject: [PATCH 16/64] max 100 lines of logs/check + scrollable logs in webview --- check50/c.py | 3 ++- check50/renderer/templates/results.html | 2 +- check50/runner.py | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/check50/c.py b/check50/c.py index 26b195a5..7925b544 100644 --- a/check50/c.py +++ b/check50/c.py @@ -60,8 +60,9 @@ def compile(*files, exe_name=None, cc=CC, **cflags): # Strip out ANSI codes stdout = re.sub(r"\x1B\[[0-?]*[ -/]*[@-~]", "", process.stdout()) + # Log the last 50 lines of output in case compilation fails if process.exitcode != 0: - for line in stdout.splitlines(): + for line in stdout.splitlines()[-50:]: log(line) raise Failure("code failed to compile") diff --git a/check50/renderer/templates/results.html b/check50/renderer/templates/results.html index 683ef88f..6a51efa8 100644 --- a/check50/renderer/templates/results.html +++ b/check50/renderer/templates/results.html @@ -43,7 +43,7 @@

:| {{ check.description }}

{% if check.log %} Log
-
+
{% for item in check.log %} {{ item }}
diff --git a/check50/runner.py b/check50/runner.py index d0d7f44d..4d286dde 100644 --- a/check50/runner.py +++ b/check50/runner.py @@ -155,7 +155,7 @@ def wrapper(checks_root, dependency_state): else: result.passed = True finally: - result.log = _log + result.log = _log if len(_log) <= 1 else ["..."] + _log[-100:] result.data = _data return result, state return wrapper From 8375fdbc5f9cdb6bd2789c8d71bfb9bf065b4feb Mon Sep 17 00:00:00 2001 From: jelleas Date: Thu, 28 May 2020 16:11:20 +0200 Subject: [PATCH 17/64] 1 => 100 --- check50/runner.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/check50/runner.py b/check50/runner.py index 4d286dde..16e43ba7 100644 --- a/check50/runner.py +++ b/check50/runner.py @@ -155,7 +155,7 @@ def wrapper(checks_root, dependency_state): else: result.passed = True finally: - result.log = _log if len(_log) <= 1 else ["..."] + _log[-100:] + result.log = _log if len(_log) <= 100 else ["..."] + _log[-100:] result.data = _data return result, state return wrapper From 7b5c5b97e7e1e28495aa646996be950d0f34a0d3 Mon Sep 17 00:00:00 2001 From: jelleas Date: Thu, 28 May 2020 16:19:36 +0200 Subject: [PATCH 18/64] configurable max_log_lines --- check50/c.py | 4 ++-- check50/runner.py | 6 ++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/check50/c.py b/check50/c.py index 7925b544..cb6adf3f 100644 --- a/check50/c.py +++ b/check50/c.py @@ -14,7 +14,7 @@ CFLAGS = {"std": "c11", "ggdb": True, "lm": True} -def compile(*files, exe_name=None, cc=CC, **cflags): +def compile(*files, exe_name=None, cc=CC, max_log_lines=50, **cflags): """ Compile C source files. @@ -62,7 +62,7 @@ def compile(*files, exe_name=None, cc=CC, **cflags): # Log the last 50 lines of output in case compilation fails if process.exitcode != 0: - for line in stdout.splitlines()[-50:]: + for line in stdout.splitlines()[-max_log_lines:]: log(line) raise Failure("code failed to compile") diff --git a/check50/runner.py b/check50/runner.py index 16e43ba7..47ef768c 100644 --- a/check50/runner.py +++ b/check50/runner.py @@ -80,13 +80,15 @@ def _handle_timeout(*args): signal.signal(signal.SIGALRM, signal.SIG_DFL) -def check(dependency=None, timeout=60): +def check(dependency=None, timeout=60, max_log_lines=100): """Mark function as a check. :param dependency: the check that this check depends on :type dependency: function :param timeout: maximum number of seconds the check can run :type timeout: int + :param max_log_lines: maximum number of lines that can appear in the log + :type max_log_lines: int When a check depends on another, the former will only run if the latter passes. Additionally, the dependent check will inherit the filesystem of its dependency. @@ -155,7 +157,7 @@ def wrapper(checks_root, dependency_state): else: result.passed = True finally: - result.log = _log if len(_log) <= 100 else ["..."] + _log[-100:] + result.log = _log if len(_log) <= max_log_lines else ["..."] + _log[-max_log_lines:] result.data = _data return result, state return wrapper From ecee05853e1670d4bdd4093a10f98bdfc29cd97d Mon Sep 17 00:00:00 2001 From: jelleas Date: Thu, 28 May 2020 16:22:35 +0200 Subject: [PATCH 19/64] comments --- check50/c.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/check50/c.py b/check50/c.py index cb6adf3f..535afa3c 100644 --- a/check50/c.py +++ b/check50/c.py @@ -60,7 +60,7 @@ def compile(*files, exe_name=None, cc=CC, max_log_lines=50, **cflags): # Strip out ANSI codes stdout = re.sub(r"\x1B\[[0-?]*[ -/]*[@-~]", "", process.stdout()) - # Log the last 50 lines of output in case compilation fails + # Log the last max_log_lines lines of output in case compilation fails if process.exitcode != 0: for line in stdout.splitlines()[-max_log_lines:]: log(line) From 0f16292b78ee1a1ab85b4a15a952f3ba0e6439ef Mon Sep 17 00:00:00 2001 From: L33Tech <36166244+L33Tech@users.noreply.github.com> Date: Wed, 17 Jun 2020 15:10:59 +0000 Subject: [PATCH 20/64] Update README.md --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 9fb73c03..094be984 100644 --- a/README.md +++ b/README.md @@ -2,5 +2,5 @@ check50 is a testing tool for checking student code. As a student you can use check50 to check your CS50 problem sets or any other Problem sets for which check50 checks exist. check50 allows teachers to automatically grade code on correctness and to provide automatic feedback while students are coding. -You can find documentation and instructions for writing your own checks at https://cs50.readthedocs.io/check50/. +You can find documentation and instructions for writing your own checks at https://cs50.readthedocs.io/projects/check50/. From da2d6963b10a1d1bd332de7d6d38aea8155e8e14 Mon Sep 17 00:00:00 2001 From: Joshua Archibald Date: Mon, 22 Jun 2020 16:08:49 -0400 Subject: [PATCH 21/64] Add test for compiling an empty C file. --- tests/c_tests.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/c_tests.py b/tests/c_tests.py index 3cb32471..6a3e02cd 100644 --- a/tests/c_tests.py +++ b/tests/c_tests.py @@ -28,6 +28,12 @@ def tearDown(self): self.working_directory.cleanup() class TestCompile(Base): + def test_compile_incorrect(self): + open("blank.c", "w").close() + + with self.assertRaises(check50.Failure): + check50.c.compile("blank.c") + def test_compile_hello_world(self): with open("hello.c", "w") as f: src = '#include \n'\ From 9c1f07324d72224d39c172c4f419dd12aac5d5eb Mon Sep 17 00:00:00 2001 From: Joshua Archibald Date: Mon, 22 Jun 2020 16:22:37 -0400 Subject: [PATCH 22/64] Add incorrect file test to TestExitPy. --- tests/check50_tests.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/tests/check50_tests.py b/tests/check50_tests.py index c8a8b2e2..16e52b4d 100644 --- a/tests/check50_tests.py +++ b/tests/check50_tests.py @@ -58,7 +58,7 @@ def test_no_file(self): process.expect_exact("can't check until a frown turns upside down") process.close(force=True) - def test_with_file(self): + def test_with_correct_file(self): open("foo.py", "w").close() process = pexpect.spawn(f"check50 --dev {CHECKS_DIRECTORY}/exit_py") process.expect_exact(":)") @@ -67,6 +67,18 @@ def test_with_file(self): process.expect_exact("foo.py exits properly") process.close(force=True) + def test_with_incorrect_file(self): + with open("foo.py", "w") as f: + f.write("from sys import exit\nexit(1)") + + process = pexpect.spawn(f"check50 --dev {CHECKS_DIRECTORY}/exit_py") + process.expect_exact(":)") + process.expect_exact("foo.py exists") + process.expect_exact(":(") + process.expect_exact("foo.py exits properly") + process.expect_exact("expected exit code 0, not 1") + process.close(force=True) + class TestStdoutPy(Base): def test_no_file(self): From d29f27433f63181314fac0d1eec72ac68d1622aa Mon Sep 17 00:00:00 2001 From: jelleas Date: Fri, 26 Jun 2020 21:10:05 +0200 Subject: [PATCH 23/64] compromise, first 50//2 + last 50//2 log lines --- check50/c.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/check50/c.py b/check50/c.py index 535afa3c..e33e0a98 100644 --- a/check50/c.py +++ b/check50/c.py @@ -60,10 +60,16 @@ def compile(*files, exe_name=None, cc=CC, max_log_lines=50, **cflags): # Strip out ANSI codes stdout = re.sub(r"\x1B\[[0-?]*[ -/]*[@-~]", "", process.stdout()) - # Log the last max_log_lines lines of output in case compilation fails + # Log max_log_lines lines of output in case compilation fails if process.exitcode != 0: - for line in stdout.splitlines()[-max_log_lines:]: + lines = stdout.splitlines() + + if len(lines) > max_log_lines: + lines = lines[:max_log_lines // 2] + lines[-(max_log_lines // 2):] + + for line in lines: log(line) + raise Failure("code failed to compile") From fec918497478bfed47e14f71dcf1c197cfa0836f Mon Sep 17 00:00:00 2001 From: jelleas Date: Wed, 1 Jul 2020 20:44:46 +0200 Subject: [PATCH 24/64] mv excepthook => internal --- check50/__main__.py | 56 ++++++--------------------------------------- check50/internal.py | 55 ++++++++++++++++++++++++++++++++++++++++++++ check50/runner.py | 3 ++- 3 files changed, 64 insertions(+), 50 deletions(-) diff --git a/check50/__main__.py b/check50/__main__.py index e0cac2bc..95378a82 100644 --- a/check50/__main__.py +++ b/check50/__main__.py @@ -42,48 +42,6 @@ def nullcontext(entry_result=None): yield entry_result -def excepthook(cls, exc, tb): - # All channels to output to - outputs = excepthook.outputs - - for output in excepthook.outputs: - outputs.remove(output) - if output == "json": - ctxmanager = open(excepthook.output_file, "w") if excepthook.output_file else nullcontext(sys.stdout) - with ctxmanager as output_file: - json.dump({ - "slug": SLUG, - "error": { - "type": cls.__name__, - "value": str(exc), - "traceback": traceback.format_tb(exc.__traceback__), - "data" : exc.payload if hasattr(exc, "payload") else {} - }, - "version": __version__ - }, output_file, indent=4) - output_file.write("\n") - - elif output == "ansi" or output == "html": - if (issubclass(cls, internal.Error) or issubclass(cls, lib50.Error)) and exc.args: - termcolor.cprint(str(exc), "red", file=sys.stderr) - elif issubclass(cls, FileNotFoundError): - termcolor.cprint(_("{} not found").format(exc.filename), "red", file=sys.stderr) - elif issubclass(cls, KeyboardInterrupt): - termcolor.cprint(f"check cancelled", "red") - elif not issubclass(cls, Exception): - # Class is some other BaseException, better just let it go - return - else: - termcolor.cprint(_("Sorry, something's wrong! Let sysadmins@cs50.harvard.edu know!"), "red", file=sys.stderr) - - if excepthook.verbose: - traceback.print_exception(cls, exc, tb) - if hasattr(exc, "payload"): - print("Exception payload:", json.dumps(exc.payload), sep="\n") - - sys.exit(1) - - def yes_no_prompt(prompt): """ Raise a prompt, returns True if yes is entered, False if no is entered. @@ -100,10 +58,10 @@ def yes_no_prompt(prompt): # Assume we should print tracebacks until we get command line arguments -excepthook.verbose = True -excepthook.output = "ansi" -excepthook.output_file = None -sys.excepthook = excepthook +internal.excepthook.verbose = True +internal.excepthook.outputs = ("ansi",) +internal.excepthook.output_file = None +sys.excepthook = internal.excepthook def install_dependencies(dependencies, verbose=False): @@ -342,9 +300,9 @@ def main(): args.output = [output for output in args.output if not (output in seen_output or seen_output.add(output))] # Set excepthook - excepthook.verbose = bool(args.verbose) - excepthook.outputs = args.output - excepthook.output_file = args.output_file + internal.excepthook.verbose = bool(args.verbose) + internal.excepthook.outputs = args.output + internal.excepthook.output_file = args.output_file # If remote, push files to GitHub and await results if not args.local: diff --git a/check50/internal.py b/check50/internal.py index b5996c45..bacd349f 100644 --- a/check50/internal.py +++ b/check50/internal.py @@ -88,6 +88,61 @@ def __exit__(self, exc_type, exc_val, exc_tb): register = Register() +def excepthook(cls, exc, tb): + """ + check50's excepthook with configurable error output. + + :ivar excepthook.verbose: show the full tracebook iff set to True + :vartype excepthook.verbose: bool + :ivar excepthook.outputs: in which format errors should be returned (can be multiple) + :vartype excepthook.outputs: tuple of strings, any of "json", "ansi", "html" + :ivar excepthook.output_file: file to which the output should be redirected + :vartype excepthook.output_file: str or pathlib.Path + + See also: https://docs.python.org/3/library/sys.html#sys.excepthook + """ + + # All channels to output to + outputs = excepthook.outputs + + for output in excepthook.outputs: + outputs.remove(output) + if output == "json": + ctxmanager = open(excepthook.output_file, "w") if excepthook.output_file else nullcontext(sys.stdout) + with ctxmanager as output_file: + json.dump({ + "slug": SLUG, + "error": { + "type": cls.__name__, + "value": str(exc), + "traceback": traceback.format_tb(exc.__traceback__), + "data" : exc.payload if hasattr(exc, "payload") else {} + }, + "version": __version__ + }, output_file, indent=4) + output_file.write("\n") + + elif output == "ansi" or output == "html": + if (issubclass(cls, internal.Error) or issubclass(cls, lib50.Error)) and exc.args: + termcolor.cprint(str(exc), "red", file=sys.stderr) + elif issubclass(cls, FileNotFoundError): + termcolor.cprint(_("{} not found").format(exc.filename), "red", file=sys.stderr) + elif issubclass(cls, KeyboardInterrupt): + termcolor.cprint(f"check cancelled", "red") + elif not issubclass(cls, Exception): + # Class is some other BaseException, better just let it go + return + else: + termcolor.cprint(_("Sorry, something's wrong! Let sysadmins@cs50.harvard.edu know!"), "red", file=sys.stderr) + + if excepthook.verbose: + traceback.print_exception(cls, exc, tb) + if hasattr(exc, "payload"): + print("Exception payload:", json.dumps(exc.payload), sep="\n") + + sys.exit(1) + + def load_config(check_dir): """ Load configuration file from ``check_dir / ".cs50.yaml"``, applying diff --git a/check50/runner.py b/check50/runner.py index d0d7f44d..f4c6dbac 100644 --- a/check50/runner.py +++ b/check50/runner.py @@ -290,7 +290,8 @@ def _skip_children(self, check_name, results): class run_check: """ - Hack to get around the fact that `pickle` can't serialize closures. + Check job that runs in the a child process. + This is only a class to get around the fact that `pickle` can't serialize closures. This class is essentially a function that reimports the check module and runs the check. """ From fdcc93c2efffda1ee0b9ccf14348d6ab2c2d07f3 Mon Sep 17 00:00:00 2001 From: jelleas Date: Thu, 2 Jul 2020 00:35:55 +0200 Subject: [PATCH 25/64] pass module vars to check process --- check50/_api.py | 4 ++++ check50/runner.py | 12 ++++++++++++ 2 files changed, 16 insertions(+) diff --git a/check50/_api.py b/check50/_api.py index abd2d69e..8d523013 100644 --- a/check50/_api.py +++ b/check50/_api.py @@ -401,6 +401,10 @@ class Missing(Failure): def __init__(self, missing_item, collection, help=None): super().__init__(rationale=_("Did not find {} in {}").format(_raw(missing_item), _raw(collection)), help=help) + + if missing_item == EOF: + missing_item = "EOF" + self.payload.update({"missing_item": str(missing_item), "collection": str(collection)}) diff --git a/check50/runner.py b/check50/runner.py index f4c6dbac..f21f7472 100644 --- a/check50/runner.py +++ b/check50/runner.py @@ -301,7 +301,19 @@ def __init__(self, check_name, spec, checks_root, state=None): self.checks_root = checks_root self.state = state + # Carry over relevant module variables to the check process + self.check_dir = internal.check_dir + self.excepthook_outputs = internal.excepthook.outputs + self.excepthook_output_file = internal.excepthook.output_file + self.excepthook_verbose = internal.excepthook.verbose + def __call__(self): + # Init module variables in check process + internal.check_dir = self.check_dir + internal.excepthook.outputs = self.excepthook_outputs + internal.excepthook.output_file = self.excepthook_output_file + internal.excepthook.verbose = self.excepthook_verbose + mod = importlib.util.module_from_spec(self.spec) self.spec.loader.exec_module(mod) internal.check_running = True From 66c07cf2a79afdd534b3d6aeb833446cdf93c926 Mon Sep 17 00:00:00 2001 From: jelleas Date: Thu, 2 Jul 2020 00:53:11 +0200 Subject: [PATCH 26/64] __main__.SLUG => internal.slug --- check50/__main__.py | 22 ++++++++-------------- check50/internal.py | 11 ++++++++--- check50/runner.py | 2 ++ 3 files changed, 18 insertions(+), 17 deletions(-) diff --git a/check50/__main__.py b/check50/__main__.py index 95378a82..679a0932 100644 --- a/check50/__main__.py +++ b/check50/__main__.py @@ -4,7 +4,6 @@ import importlib import inspect import itertools -import json import logging import os import site @@ -14,7 +13,6 @@ import subprocess import sys import tempfile -import traceback import time import attr @@ -27,9 +25,6 @@ lib50.set_local_path(os.environ.get("CHECK50_PATH", "~/.local/share/check50")) -SLUG = None - - class RemoteCheckError(internal.Error): def __init__(self, remote_json): super().__init__("check50 ran into an error while running checks! Please contact sysadmins@cs50.harvard.edu!") @@ -265,8 +260,7 @@ def main(): args = parser.parse_args() - global SLUG - SLUG = args.slug + internal.slug = args.slug # dev implies offline and verbose "info" if not overwritten if args.dev: @@ -306,25 +300,25 @@ def main(): # If remote, push files to GitHub and await results if not args.local: - commit_hash = lib50.push("check50", SLUG, internal.CONFIG_LOADER, data={"check50": True})[1] + commit_hash = lib50.push("check50", internal.slug, internal.CONFIG_LOADER, data={"check50": True})[1] with lib50.ProgressBar("Waiting for results") if "ansi" in args.output else nullcontext(): - tag_hash, results = await_results(commit_hash, SLUG) + tag_hash, results = await_results(commit_hash, internal.slug) # Otherwise run checks locally else: with lib50.ProgressBar("Checking") if "ansi" in args.output else nullcontext(): # If developing, assume slug is a path to check_dir if args.dev: - internal.check_dir = Path(SLUG).expanduser().resolve() + internal.check_dir = Path(internal.slug).expanduser().resolve() if not internal.check_dir.is_dir(): raise internal.Error(_("{} is not a directory").format(internal.check_dir)) # Otherwise have lib50 create a local copy of slug else: try: - internal.check_dir = lib50.local(SLUG, offline=args.no_download_checks) + internal.check_dir = lib50.local(internal.slug, offline=args.no_download_checks) except lib50.ConnectionError: - raise internal.Error(_("check50 could not retrieve checks from GitHub. Try running check50 again with --offline.").format(SLUG)) + raise internal.Error(_("check50 could not retrieve checks from GitHub. Try running check50 again with --offline.").format(internal.slug)) except lib50.InvalidSlugError: - raise_invalid_slug(SLUG, offline=args.no_download_checks) + raise_invalid_slug(internal.slug, offline=args.no_download_checks) # Load config config = internal.load_config(internal.check_dir) @@ -358,7 +352,7 @@ def main(): check_results = CheckRunner(checks_file).run(included, working_area, args.target) results = { - "slug": SLUG, + "slug": internal.slug, "results": [attr.asdict(result) for result in check_results], "version": __version__ } diff --git a/check50/internal.py b/check50/internal.py index bacd349f..66a67e67 100644 --- a/check50/internal.py +++ b/check50/internal.py @@ -2,13 +2,15 @@ Additional check50 internals exposed to extension writers in addition to the standard API """ -import importlib from pathlib import Path +import importlib +import json import sys +import traceback import lib50 -from . import _simple +from . import _simple, __version__ #: Directory containing the check and its associated files check_dir = None @@ -19,6 +21,9 @@ #: Boolean that indicates if a check is currently running check_running = False +#: The user specified slug used to indentifies the set of checks +slug = None + #: ``lib50`` config loader CONFIG_LOADER = lib50.config.Loader("check50") CONFIG_LOADER.scope("files", "include", "exclude", "require") @@ -111,7 +116,7 @@ def excepthook(cls, exc, tb): ctxmanager = open(excepthook.output_file, "w") if excepthook.output_file else nullcontext(sys.stdout) with ctxmanager as output_file: json.dump({ - "slug": SLUG, + "slug": slug, "error": { "type": cls.__name__, "value": str(exc), diff --git a/check50/runner.py b/check50/runner.py index f21f7472..54350ddd 100644 --- a/check50/runner.py +++ b/check50/runner.py @@ -303,6 +303,7 @@ def __init__(self, check_name, spec, checks_root, state=None): # Carry over relevant module variables to the check process self.check_dir = internal.check_dir + self.slug = internal.slug self.excepthook_outputs = internal.excepthook.outputs self.excepthook_output_file = internal.excepthook.output_file self.excepthook_verbose = internal.excepthook.verbose @@ -310,6 +311,7 @@ def __init__(self, check_name, spec, checks_root, state=None): def __call__(self): # Init module variables in check process internal.check_dir = self.check_dir + internal.slug = self.slug internal.excepthook.outputs = self.excepthook_outputs internal.excepthook.output_file = self.excepthook_output_file internal.excepthook.verbose = self.excepthook_verbose From 6a19f21c346db0e30af810d1034f8935009684c2 Mon Sep 17 00:00:00 2001 From: jelleas Date: Thu, 2 Jul 2020 14:51:39 +0200 Subject: [PATCH 27/64] DRY names --- check50/runner.py | 35 ++++++++++++++++++++++------------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/check50/runner.py b/check50/runner.py index 54350ddd..22a4977a 100644 --- a/check50/runner.py +++ b/check50/runner.py @@ -10,6 +10,7 @@ from pathlib import Path import shutil import signal +import sys import tempfile import traceback @@ -300,21 +301,29 @@ def __init__(self, check_name, spec, checks_root, state=None): self.spec = spec self.checks_root = checks_root self.state = state - - # Carry over relevant module variables to the check process - self.check_dir = internal.check_dir - self.slug = internal.slug - self.excepthook_outputs = internal.excepthook.outputs - self.excepthook_output_file = internal.excepthook.output_file - self.excepthook_verbose = internal.excepthook.verbose + self.attribute_names = ( + "internal.check_dir", + "internal.slug", + "internal.excepthook.outputs", + "internal.excepthook.output_file", + "internal.excepthook.verbose" + ) + self.attribute_values = tuple(eval(name) for name in self.attribute_names) + + @staticmethod + def _set_attribute(name, value): + """Get an attribute from a name in global scope and set its value.""" + parts = name.split(".") + + obj = sys.modules[__name__] + for part in parts[:-1]: + obj = getattr(obj, part) + + setattr(obj, parts[-1], value) def __call__(self): - # Init module variables in check process - internal.check_dir = self.check_dir - internal.slug = self.slug - internal.excepthook.outputs = self.excepthook_outputs - internal.excepthook.output_file = self.excepthook_output_file - internal.excepthook.verbose = self.excepthook_verbose + for name, val in zip(self.attribute_names, self.attribute_values): + self._set_attribute(name, val) mod = importlib.util.module_from_spec(self.spec) self.spec.loader.exec_module(mod) From d4153e7d06a6a30a394af99fd108445b2d293a53 Mon Sep 17 00:00:00 2001 From: jelleas Date: Thu, 2 Jul 2020 15:04:44 +0200 Subject: [PATCH 28/64] typo --- check50/runner.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/check50/runner.py b/check50/runner.py index 22a4977a..e6431cab 100644 --- a/check50/runner.py +++ b/check50/runner.py @@ -291,7 +291,7 @@ def _skip_children(self, check_name, results): class run_check: """ - Check job that runs in the a child process. + Check job that runs in a seperate process. This is only a class to get around the fact that `pickle` can't serialize closures. This class is essentially a function that reimports the check module and runs the check. """ From 93537f6cdcc3608da665b5f2466b741a3d321765 Mon Sep 17 00:00:00 2001 From: jelleas Date: Thu, 2 Jul 2020 15:05:18 +0200 Subject: [PATCH 29/64] typo --- check50/runner.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/check50/runner.py b/check50/runner.py index e6431cab..25ded6cd 100644 --- a/check50/runner.py +++ b/check50/runner.py @@ -291,7 +291,7 @@ def _skip_children(self, check_name, results): class run_check: """ - Check job that runs in a seperate process. + Check job that runs in a separate process. This is only a class to get around the fact that `pickle` can't serialize closures. This class is essentially a function that reimports the check module and runs the check. """ From 806528a5325d1725ec1a9859af11669bf39613e5 Mon Sep 17 00:00:00 2001 From: jelleas Date: Sat, 4 Jul 2020 22:35:45 +0200 Subject: [PATCH 30/64] internal.Error => Error --- check50/internal.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/check50/internal.py b/check50/internal.py index 66a67e67..13c583ce 100644 --- a/check50/internal.py +++ b/check50/internal.py @@ -128,7 +128,7 @@ def excepthook(cls, exc, tb): output_file.write("\n") elif output == "ansi" or output == "html": - if (issubclass(cls, internal.Error) or issubclass(cls, lib50.Error)) and exc.args: + if (issubclass(cls, Error) or issubclass(cls, lib50.Error)) and exc.args: termcolor.cprint(str(exc), "red", file=sys.stderr) elif issubclass(cls, FileNotFoundError): termcolor.cprint(_("{} not found").format(exc.filename), "red", file=sys.stderr) From 16afb48acdf51f51eb8e6939675ea26daf795857 Mon Sep 17 00:00:00 2001 From: jelleas Date: Sat, 4 Jul 2020 22:36:38 +0200 Subject: [PATCH 31/64] import termcolor --- check50/internal.py | 1 + 1 file changed, 1 insertion(+) diff --git a/check50/internal.py b/check50/internal.py index 13c583ce..fa30ceb6 100644 --- a/check50/internal.py +++ b/check50/internal.py @@ -6,6 +6,7 @@ import importlib import json import sys +import termcolor import traceback import lib50 From ae30d6855f7f3b0e1467d2f3e7b5a37b57e6da08 Mon Sep 17 00:00:00 2001 From: jelleas Date: Mon, 6 Jul 2020 16:04:03 +0200 Subject: [PATCH 32/64] internal.run_root_dir --- check50/__main__.py | 4 ++-- check50/internal.py | 5 ++++- check50/runner.py | 25 ++++++++++++++----------- 3 files changed, 20 insertions(+), 14 deletions(-) diff --git a/check50/__main__.py b/check50/__main__.py index e0cac2bc..67c03c7e 100644 --- a/check50/__main__.py +++ b/check50/__main__.py @@ -387,7 +387,7 @@ def main(): with open(os.devnull, "w") if args.verbose else nullcontext() as devnull: # Redirect stdout to devnull if some verbosity level is set - if args.verbose: + if not args.verbose: stdout = stderr = devnull else: stdout = sys.stdout @@ -398,7 +398,7 @@ def main(): contextlib.redirect_stdout(stdout), \ contextlib.redirect_stderr(stderr): - check_results = CheckRunner(checks_file).run(included, working_area, args.target) + check_results = CheckRunner(checks_file, working_area.parent).run(included, args.target) results = { "slug": SLUG, "results": [attr.asdict(result) for result in check_results], diff --git a/check50/internal.py b/check50/internal.py index b5996c45..fc4ba6bc 100644 --- a/check50/internal.py +++ b/check50/internal.py @@ -13,9 +13,12 @@ #: Directory containing the check and its associated files check_dir = None -#: Temporary directory in which check is being run +#: Temporary directory in which the current check is being run run_dir = None +#: Temporary directory that is the root (parent) of all run_dir(s) +run_root_dir = None + #: Boolean that indicates if a check is currently running check_running = False diff --git a/check50/runner.py b/check50/runner.py index d0d7f44d..161f96fe 100644 --- a/check50/runner.py +++ b/check50/runner.py @@ -123,7 +123,7 @@ def decorator(check): check._check_dependency = dependency @functools.wraps(check) - def wrapper(checks_root, dependency_state): + def wrapper(run_root_dir, dependency_state): # Result template result = CheckResult.from_check(check) # Any shared (returned) state @@ -131,8 +131,8 @@ def wrapper(checks_root, dependency_state): try: # Setup check environment, copying disk state from dependency - internal.run_dir = checks_root / check.__name__ - src_dir = checks_root / (dependency.__name__ if dependency else "-") + internal.run_dir = run_root_dir / check.__name__ + src_dir = run_root_dir / (dependency.__name__ if dependency else "-") shutil.copytree(src_dir, internal.run_dir) os.chdir(internal.run_dir) @@ -163,7 +163,10 @@ def wrapper(checks_root, dependency_state): class CheckRunner: - def __init__(self, checks_path): + def __init__(self, checks_path, run_root_dir=Path(".")): + internal.run_root_dir = run_root_dir + os.chdir(internal.run_root_dir) + # TODO: Naming the module "checks" is arbitray. Better name? self.checks_spec = importlib.util.spec_from_file_location("checks", checks_path) @@ -189,7 +192,7 @@ def __init__(self, checks_path): self.check_descriptions = {name: check.__doc__ for name, check in checks} - def run(self, files, working_area, targets=None): + def run(self, files, targets=None): """ Run checks concurrently. Returns a list of CheckResults ordered by declaration order of the checks in the imported module @@ -200,7 +203,6 @@ def run(self, files, working_area, targets=None): # Ensure that dictionary is ordered by check declaration order (via self.check_names) # NOTE: Requires CPython 3.6. If we need to support older versions of Python, replace with OrderedDict. results = {name: None for name in self.check_names} - checks_root = working_area.parent try: max_workers = int(os.environ.get("CHECK50_WORKERS")) @@ -209,7 +211,7 @@ def run(self, files, working_area, targets=None): with futures.ProcessPoolExecutor(max_workers=max_workers) as executor: # Start all checks that have no dependencies - not_done = set(executor.submit(run_check(name, self.checks_spec, checks_root)) + not_done = set(executor.submit(run_check(name, self.checks_spec, internal.run_root_dir)) for name in graph[None]) not_passed = [] @@ -223,7 +225,7 @@ def run(self, files, working_area, targets=None): # Dispatch dependent checks for child_name in graph[result.name]: not_done.add(executor.submit( - run_check(child_name, self.checks_spec, checks_root, state))) + run_check(child_name, self.checks_spec, internal.run_root_dir, state))) else: not_passed.append(result.name) @@ -294,17 +296,18 @@ class run_check: This class is essentially a function that reimports the check module and runs the check. """ - def __init__(self, check_name, spec, checks_root, state=None): + def __init__(self, check_name, spec, run_root_dir, state=None): self.check_name = check_name self.spec = spec - self.checks_root = checks_root + self.run_root_dir = run_root_dir self.state = state def __call__(self): + internal.run_root_dir = self.run_root_dir mod = importlib.util.module_from_spec(self.spec) self.spec.loader.exec_module(mod) internal.check_running = True try: - return getattr(mod, self.check_name)(self.checks_root, self.state) + return getattr(mod, self.check_name)(self.run_root_dir, self.state) finally: internal.check_running = False From bf38b35e73ce3a5a4987c2cdc0222227f41c002d Mon Sep 17 00:00:00 2001 From: jelleas Date: Mon, 6 Jul 2020 18:32:35 +0200 Subject: [PATCH 33/64] CheckRunner as contextmanager --- check50/__main__.py | 23 ++++++------- check50/runner.py | 78 ++++++++++++++++++++++++++++----------------- 2 files changed, 61 insertions(+), 40 deletions(-) diff --git a/check50/__main__.py b/check50/__main__.py index 67c03c7e..64a7f765 100644 --- a/check50/__main__.py +++ b/check50/__main__.py @@ -383,7 +383,7 @@ def main(): checks_file = (internal.check_dir / config["checks"]).resolve() # Have lib50 decide which files to include - included = lib50.files(config.get("files"))[0] + included_files = lib50.files(config.get("files"))[0] with open(os.devnull, "w") if args.verbose else nullcontext() as devnull: # Redirect stdout to devnull if some verbosity level is set @@ -394,16 +394,17 @@ def main(): stderr = sys.stderr # Create a working_area (temp dir) named - with all included student files - with lib50.working_area(included, name='-') as working_area, \ - contextlib.redirect_stdout(stdout), \ - contextlib.redirect_stderr(stderr): - - check_results = CheckRunner(checks_file, working_area.parent).run(included, args.target) - results = { - "slug": SLUG, - "results": [attr.asdict(result) for result in check_results], - "version": __version__ - } + with contextlib.redirect_stdout(stdout), \ + contextlib.redirect_stderr(stderr): + + with CheckRunner(checks_file, included_files) as check_runner: + check_results = check_runner.run(args.target) + + results = { + "slug": SLUG, + "results": [attr.asdict(result) for result in check_results], + "version": __version__ + } # Render output diff --git a/check50/runner.py b/check50/runner.py index 161f96fe..91b62bd3 100644 --- a/check50/runner.py +++ b/check50/runner.py @@ -14,6 +14,7 @@ import traceback import attr +import lib50 from . import internal from ._api import log, Failure, _copy, _log, _data @@ -163,36 +164,11 @@ def wrapper(run_root_dir, dependency_state): class CheckRunner: - def __init__(self, checks_path, run_root_dir=Path(".")): - internal.run_root_dir = run_root_dir - os.chdir(internal.run_root_dir) + def __init__(self, checks_path, included_files): + self.checks_path = checks_path + self.included_files = included_files - # TODO: Naming the module "checks" is arbitray. Better name? - self.checks_spec = importlib.util.spec_from_file_location("checks", checks_path) - - # Clear check_names, import module, then save check_names. Not thread safe. - # Ideally, there'd be a better way to extract declaration order than @check mutating global state, - # but there are a lot of subtleties with using `inspect` or similar here - _check_names.clear() - check_module = importlib.util.module_from_spec(self.checks_spec) - self.checks_spec.loader.exec_module(check_module) - self.check_names = _check_names.copy() - _check_names.clear() - - # Grab all checks from the module - checks = inspect.getmembers(check_module, lambda f: hasattr(f, "_check_dependency")) - - # Map each check to tuples containing the names of the checks that depend on it - self.dependency_graph = collections.defaultdict(set) - for name, check in checks: - dependency = None if check._check_dependency is None else check._check_dependency.__name__ - self.dependency_graph[dependency].add(name) - - # Map each check name to its description - self.check_descriptions = {name: check.__doc__ for name, check in checks} - - - def run(self, files, targets=None): + def run(self, targets=None): """ Run checks concurrently. Returns a list of CheckResults ordered by declaration order of the checks in the imported module @@ -290,6 +266,50 @@ def _skip_children(self, check_name, results): self._skip_children(name, results) + def __enter__(self): + # Set up a temp dir for the checks + self._working_area_manager = lib50.working_area(self.included_files, name='-') + internal.run_root_dir = self._working_area_manager.__enter__().parent + + # Change current working dir to the temp dir + self._cd_manager = lib50.cd(internal.run_root_dir) + self._cd_manager.__enter__() + + # TODO: Naming the module "checks" is arbitray. Better name? + self.checks_spec = importlib.util.spec_from_file_location("checks", self.checks_path) + + # Clear check_names, import module, then save check_names. Not thread safe. + # Ideally, there'd be a better way to extract declaration order than @check mutating global state, + # but there are a lot of subtleties with using `inspect` or similar here + _check_names.clear() + check_module = importlib.util.module_from_spec(self.checks_spec) + self.checks_spec.loader.exec_module(check_module) + self.check_names = _check_names.copy() + _check_names.clear() + + # Grab all checks from the module + checks = inspect.getmembers(check_module, lambda f: hasattr(f, "_check_dependency")) + + # Map each check to tuples containing the names of the checks that depend on it + self.dependency_graph = collections.defaultdict(set) + for name, check in checks: + dependency = None if check._check_dependency is None else check._check_dependency.__name__ + self.dependency_graph[dependency].add(name) + + # Map each check name to its description + self.check_descriptions = {name: check.__doc__ for name, check in checks} + + return self + + + def __exit__(self, type, value, tb): + # Destroy the temporary directory for the checks + self._working_area_manager.__exit__(type, value, tb) + + # cd back to the directory check50 was called from + self._cd_manager.__exit__(type, value, tb) + + class run_check: """ Hack to get around the fact that `pickle` can't serialize closures. From de6e50eb99efca2b9694724564fa49abc2931d23 Mon Sep 17 00:00:00 2001 From: jelleas Date: Mon, 6 Jul 2020 19:29:25 +0200 Subject: [PATCH 34/64] internal.student_dir --- check50/internal.py | 4 ++++ check50/runner.py | 12 +++++++----- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/check50/internal.py b/check50/internal.py index fc4ba6bc..2bde54a3 100644 --- a/check50/internal.py +++ b/check50/internal.py @@ -19,9 +19,13 @@ #: Temporary directory that is the root (parent) of all run_dir(s) run_root_dir = None +#: Directory check50 was run from +student_dir = None + #: Boolean that indicates if a check is currently running check_running = False + #: ``lib50`` config loader CONFIG_LOADER = lib50.config.Loader("check50") CONFIG_LOADER.scope("files", "include", "exclude", "require") diff --git a/check50/runner.py b/check50/runner.py index 91b62bd3..c61d8fcc 100644 --- a/check50/runner.py +++ b/check50/runner.py @@ -187,7 +187,7 @@ def run(self, targets=None): with futures.ProcessPoolExecutor(max_workers=max_workers) as executor: # Start all checks that have no dependencies - not_done = set(executor.submit(run_check(name, self.checks_spec, internal.run_root_dir)) + not_done = set(executor.submit(run_check(name, self.checks_spec)) for name in graph[None]) not_passed = [] @@ -201,7 +201,7 @@ def run(self, targets=None): # Dispatch dependent checks for child_name in graph[result.name]: not_done.add(executor.submit( - run_check(child_name, self.checks_spec, internal.run_root_dir, state))) + run_check(child_name, self.checks_spec, state))) else: not_passed.append(result.name) @@ -267,6 +267,9 @@ def _skip_children(self, check_name, results): def __enter__(self): + # Remember the student's directory + internal.student_dir = Path.cwd() + # Set up a temp dir for the checks self._working_area_manager = lib50.working_area(self.included_files, name='-') internal.run_root_dir = self._working_area_manager.__enter__().parent @@ -316,14 +319,13 @@ class run_check: This class is essentially a function that reimports the check module and runs the check. """ - def __init__(self, check_name, spec, run_root_dir, state=None): + def __init__(self, check_name, spec, state=None): self.check_name = check_name self.spec = spec - self.run_root_dir = run_root_dir + self.run_root_dir = internal.run_root_dir self.state = state def __call__(self): - internal.run_root_dir = self.run_root_dir mod = importlib.util.module_from_spec(self.spec) self.spec.loader.exec_module(mod) internal.check_running = True From 12987b393ac52e16db93bd6bb53b9f87c93116f0 Mon Sep 17 00:00:00 2001 From: jelleas Date: Mon, 6 Jul 2020 19:31:31 +0200 Subject: [PATCH 35/64] cleanup --- check50/runner.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/check50/runner.py b/check50/runner.py index c61d8fcc..d1ebc1f3 100644 --- a/check50/runner.py +++ b/check50/runner.py @@ -322,7 +322,6 @@ class run_check: def __init__(self, check_name, spec, state=None): self.check_name = check_name self.spec = spec - self.run_root_dir = internal.run_root_dir self.state = state def __call__(self): @@ -330,6 +329,6 @@ def __call__(self): self.spec.loader.exec_module(mod) internal.check_running = True try: - return getattr(mod, self.check_name)(self.run_root_dir, self.state) + return getattr(mod, self.check_name)(internal.run_root_dir, self.state) finally: internal.check_running = False From ef086d98e7112784399a32f711e9a9d19b6f138b Mon Sep 17 00:00:00 2001 From: jelleas Date: Mon, 6 Jul 2020 21:54:36 +0200 Subject: [PATCH 36/64] CROSS_PROCESS_ATTRIBUTES --- check50/runner.py | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/check50/runner.py b/check50/runner.py index 25ded6cd..cc932415 100644 --- a/check50/runner.py +++ b/check50/runner.py @@ -296,19 +296,22 @@ class run_check: This class is essentially a function that reimports the check module and runs the check. """ + # All attributes shared between check50's main process and each checks' process + # Required for "spawn": https://docs.python.org/3/library/multiprocessing.html#contexts-and-start-methods + CROSS_PROCESS_ATTRIBUTES = ( + "internal.check_dir", + "internal.slug", + "internal.excepthook.outputs", + "internal.excepthook.output_file", + "internal.excepthook.verbose" + ) + def __init__(self, check_name, spec, checks_root, state=None): self.check_name = check_name self.spec = spec self.checks_root = checks_root self.state = state - self.attribute_names = ( - "internal.check_dir", - "internal.slug", - "internal.excepthook.outputs", - "internal.excepthook.output_file", - "internal.excepthook.verbose" - ) - self.attribute_values = tuple(eval(name) for name in self.attribute_names) + self.attribute_values = tuple(eval(name) for name in self.CROSS_PROCESS_ATTRIBUTES) @staticmethod def _set_attribute(name, value): @@ -322,7 +325,7 @@ def _set_attribute(name, value): setattr(obj, parts[-1], value) def __call__(self): - for name, val in zip(self.attribute_names, self.attribute_values): + for name, val in zip(self.CROSS_PROCESS_ATTRIBUTES, self.attribute_values): self._set_attribute(name, val) mod = importlib.util.module_from_spec(self.spec) From bfe71adfbf28c9f2b6e41430269e1ee78fd36ce6 Mon Sep 17 00:00:00 2001 From: jelleas Date: Mon, 6 Jul 2020 22:07:31 +0200 Subject: [PATCH 37/64] _excepthook --- check50/__main__.py | 14 +++--- check50/internal.py | 110 ++++++++++++++++++++++---------------------- check50/runner.py | 6 +-- 3 files changed, 65 insertions(+), 65 deletions(-) diff --git a/check50/__main__.py b/check50/__main__.py index 679a0932..8c1890ec 100644 --- a/check50/__main__.py +++ b/check50/__main__.py @@ -53,10 +53,10 @@ def yes_no_prompt(prompt): # Assume we should print tracebacks until we get command line arguments -internal.excepthook.verbose = True -internal.excepthook.outputs = ("ansi",) -internal.excepthook.output_file = None -sys.excepthook = internal.excepthook +internal._excepthook.verbose = True +internal._excepthook.outputs = ("ansi",) +internal._excepthook.output_file = None +sys.excepthook = internal._excepthook def install_dependencies(dependencies, verbose=False): @@ -294,9 +294,9 @@ def main(): args.output = [output for output in args.output if not (output in seen_output or seen_output.add(output))] # Set excepthook - internal.excepthook.verbose = bool(args.verbose) - internal.excepthook.outputs = args.output - internal.excepthook.output_file = args.output_file + internal._excepthook.verbose = bool(args.verbose) + internal._excepthook.outputs = args.output + internal._excepthook.output_file = args.output_file # If remote, push files to GitHub and await results if not args.local: diff --git a/check50/internal.py b/check50/internal.py index fa30ceb6..e1fba9a7 100644 --- a/check50/internal.py +++ b/check50/internal.py @@ -94,61 +94,6 @@ def __exit__(self, exc_type, exc_val, exc_tb): register = Register() -def excepthook(cls, exc, tb): - """ - check50's excepthook with configurable error output. - - :ivar excepthook.verbose: show the full tracebook iff set to True - :vartype excepthook.verbose: bool - :ivar excepthook.outputs: in which format errors should be returned (can be multiple) - :vartype excepthook.outputs: tuple of strings, any of "json", "ansi", "html" - :ivar excepthook.output_file: file to which the output should be redirected - :vartype excepthook.output_file: str or pathlib.Path - - See also: https://docs.python.org/3/library/sys.html#sys.excepthook - """ - - # All channels to output to - outputs = excepthook.outputs - - for output in excepthook.outputs: - outputs.remove(output) - if output == "json": - ctxmanager = open(excepthook.output_file, "w") if excepthook.output_file else nullcontext(sys.stdout) - with ctxmanager as output_file: - json.dump({ - "slug": slug, - "error": { - "type": cls.__name__, - "value": str(exc), - "traceback": traceback.format_tb(exc.__traceback__), - "data" : exc.payload if hasattr(exc, "payload") else {} - }, - "version": __version__ - }, output_file, indent=4) - output_file.write("\n") - - elif output == "ansi" or output == "html": - if (issubclass(cls, Error) or issubclass(cls, lib50.Error)) and exc.args: - termcolor.cprint(str(exc), "red", file=sys.stderr) - elif issubclass(cls, FileNotFoundError): - termcolor.cprint(_("{} not found").format(exc.filename), "red", file=sys.stderr) - elif issubclass(cls, KeyboardInterrupt): - termcolor.cprint(f"check cancelled", "red") - elif not issubclass(cls, Exception): - # Class is some other BaseException, better just let it go - return - else: - termcolor.cprint(_("Sorry, something's wrong! Let sysadmins@cs50.harvard.edu know!"), "red", file=sys.stderr) - - if excepthook.verbose: - traceback.print_exception(cls, exc, tb) - if hasattr(exc, "payload"): - print("Exception payload:", json.dumps(exc.payload), sep="\n") - - sys.exit(1) - - def load_config(check_dir): """ Load configuration file from ``check_dir / ".cs50.yaml"``, applying @@ -240,6 +185,61 @@ def import_file(name, path): return mod +def _excepthook(cls, exc, tb): + """ + check50's excepthook with configurable error output. + + :ivar _excepthook.verbose: show the full tracebook iff set to True + :vartype _excepthook.verbose: bool + :ivar _excepthook.outputs: in which format errors should be returned (can be multiple) + :vartype _excepthook.outputs: tuple of strings, any of "json", "ansi", "html" + :ivar _excepthook.output_file: file to which the output should be redirected + :vartype _excepthook.output_file: str or pathlib.Path + + See also: https://docs.python.org/3/library/sys.html#sys.excepthook + """ + + # All channels to output to + outputs = _excepthook.outputs + + for output in _excepthook.outputs: + outputs.remove(output) + if output == "json": + ctxmanager = open(_excepthook.output_file, "w") if _excepthook.output_file else nullcontext(sys.stdout) + with ctxmanager as output_file: + json.dump({ + "slug": slug, + "error": { + "type": cls.__name__, + "value": str(exc), + "traceback": traceback.format_tb(exc.__traceback__), + "data" : exc.payload if hasattr(exc, "payload") else {} + }, + "version": __version__ + }, output_file, indent=4) + output_file.write("\n") + + elif output == "ansi" or output == "html": + if (issubclass(cls, Error) or issubclass(cls, lib50.Error)) and exc.args: + termcolor.cprint(str(exc), "red", file=sys.stderr) + elif issubclass(cls, FileNotFoundError): + termcolor.cprint(_("{} not found").format(exc.filename), "red", file=sys.stderr) + elif issubclass(cls, KeyboardInterrupt): + termcolor.cprint(f"check cancelled", "red") + elif not issubclass(cls, Exception): + # Class is some other BaseException, better just let it go + return + else: + termcolor.cprint(_("Sorry, something's wrong! Let sysadmins@cs50.harvard.edu know!"), "red", file=sys.stderr) + + if _excepthook.verbose: + traceback.print_exception(cls, exc, tb) + if hasattr(exc, "payload"): + print("Exception payload:", json.dumps(exc.payload), sep="\n") + + sys.exit(1) + + def _yes_no_prompt(prompt): """ Raise a prompt, returns True if yes is entered, False if no is entered. diff --git a/check50/runner.py b/check50/runner.py index cc932415..ee4f5bd3 100644 --- a/check50/runner.py +++ b/check50/runner.py @@ -301,9 +301,9 @@ class run_check: CROSS_PROCESS_ATTRIBUTES = ( "internal.check_dir", "internal.slug", - "internal.excepthook.outputs", - "internal.excepthook.output_file", - "internal.excepthook.verbose" + "internal._excepthook.outputs", + "internal._excepthook.output_file", + "internal._excepthook.verbose" ) def __init__(self, check_name, spec, checks_root, state=None): From d443296e7b64b8fce1bc5dd12631dea18dc2deba Mon Sep 17 00:00:00 2001 From: jelleas Date: Wed, 8 Jul 2020 18:54:25 +0200 Subject: [PATCH 38/64] [error][actions] --- check50/__main__.py | 108 ++++++++++-------- tests/check50_tests.py | 16 +++ .../remote_exception_no_traceback/.cs50.yaml | 2 + .../remote_exception_no_traceback/check.py | 22 ++++ .../remote_exception_traceback/.cs50.yaml | 2 + .../remote_exception_traceback/check.py | 22 ++++ 6 files changed, 124 insertions(+), 48 deletions(-) create mode 100644 tests/checks/remote_exception_no_traceback/.cs50.yaml create mode 100644 tests/checks/remote_exception_no_traceback/check.py create mode 100644 tests/checks/remote_exception_traceback/.cs50.yaml create mode 100644 tests/checks/remote_exception_traceback/check.py diff --git a/check50/__main__.py b/check50/__main__.py index 1cc87b8f..a9b7b7d0 100644 --- a/check50/__main__.py +++ b/check50/__main__.py @@ -48,7 +48,7 @@ class ColoredFormatter(logging.Formatter): } def __init__(self, fmt, use_color=True): - logging.Formatter.__init__(self, fmt=fmt) + super().__init__(fmt=fmt) self.use_color = use_color self.pretty_printer = pprint.PrettyPrinter(indent=1, compact=True) @@ -72,43 +72,56 @@ def nullcontext(entry_result=None): def excepthook(cls, exc, tb): - # All channels to output to - outputs = excepthook.outputs - - for output in excepthook.outputs: - outputs.remove(output) - if output == "json": - ctxmanager = open(excepthook.output_file, "w") if excepthook.output_file else nullcontext(sys.stdout) - with ctxmanager as output_file: - json.dump({ - "slug": excepthook.slug, - "error": { - "type": cls.__name__, - "value": str(exc), - "traceback": traceback.format_tb(exc.__traceback__), - "data" : exc.payload if hasattr(exc, "payload") else {} + # If an error happened remotely, grab its traceback and message + if issubclass(cls, RemoteCheckError) and "error" in exc.payload["remote_json"]: + formatted_traceback = exc.payload["remote_json"]["error"]["traceback"] + show_traceback = exc.payload["remote_json"]["error"]["actions"]["show_traceback"] + message = exc.payload["remote_json"]["error"]["actions"]["message"] + # Otherwise, create the traceback and message from this error + else: + formatted_traceback = traceback.format_exception(cls, exc, tb) + show_traceback = False + + if (issubclass(cls, internal.Error) or issubclass(cls, lib50.Error)) and exc.args: + message = str(exc) + elif issubclass(cls, FileNotFoundError): + message = _("{} not found").format(exc.filename) + elif issubclass(cls, KeyboardInterrupt): + message = _("check cancelled") + elif not issubclass(cls, Exception): + # Class is some other BaseException, better just let it go + return + else: + show_traceback = True + message = _("Sorry, something is wrong! check50 ran into an error.\n" \ + "Please let CS50 know by emailing the error above to sysadmins@cs50.harvard.edu.") + + # Output exception as json + if "json" in excepthook.outputs: + ctxmanager = open(excepthook.output_file, "w") if excepthook.output_file else nullcontext(sys.stdout) + with ctxmanager as output_file: + json.dump({ + "slug": excepthook.slug, + "error": { + "type": cls.__name__, + "value": str(exc), + "traceback": formatted_traceback, + "actions": { + "show_traceback": show_traceback, + "message": message }, - "version": __version__ - }, output_file, indent=4) - output_file.write("\n") - - elif output == "ansi" or output == "html": - if (issubclass(cls, internal.Error) or issubclass(cls, lib50.Error)) and exc.args: - termcolor.cprint(str(exc), "red", file=sys.stderr) - elif issubclass(cls, FileNotFoundError): - termcolor.cprint(_("{} not found").format(exc.filename), "red", file=sys.stderr) - elif issubclass(cls, KeyboardInterrupt): - termcolor.cprint(f"check cancelled", "red") - elif not issubclass(cls, Exception): - # Class is some other BaseException, better just let it go - return - else: - termcolor.cprint(_("Sorry, something's wrong! Let sysadmins@cs50.harvard.edu know!"), "red", file=sys.stderr) - - if excepthook.verbose: - traceback.print_exception(cls, exc, tb) - if hasattr(exc, "payload"): - print("Exception payload:", json.dumps(exc.payload), sep="\n") + "data" : exc.payload if hasattr(exc, "payload") else {} + }, + "version": __version__ + }, output_file, indent=4) + output_file.write("\n") + + # Output exception to stderr + if "ansi" in excepthook.outputs or "html" in excepthook.outputs: + if show_traceback: + for line in formatted_traceback: + termcolor.cprint(line, end="", file=sys.stderr) + termcolor.cprint(message, "red", file=sys.stderr) sys.exit(1) @@ -281,15 +294,15 @@ def raise_invalid_slug(slug, offline=False): raise internal.Error(msg) -def validate_args(args): +def process_args(args): """Validate arguments and apply defaults that are dependent on other arguments""" - # dev implies offline, verbose, and log level "INFO" if not overwritten + # dev implies offline, verbose and ansi_log if args.dev: args.offline = True args.verbose = True if "ansi" in args.output: - args.ansi_output = True + args.ansi_log = True # offline implies local if args.offline: @@ -297,8 +310,7 @@ def validate_args(args): args.no_download_checks = True args.local = True - - args.log_level = LogLevel.__members__[args.log_level.upper()] + args.log_level = LogLevel[args.log_level.upper()] # Setup logging for lib50 setup_logging(args.log_level) @@ -365,13 +377,13 @@ def main(): help=_("shows the full traceback of any errors and shows print statements written in checks")) parser.add_argument("--log-level", action="store", - choices=list(LogLevel.__members__), + choices=[level.name.lower() for level in LogLevel], default="WARNING", - type=str.upper, + type=str.lower, help=_("sets the log level." - ' "WARNING" (default) displays usage warnings' - ' "INFO" shows all commands run.' - ' "DEBUG" adds the output of all command run.')) + ' "warning" (default) displays usage warnings' + ' "info" shows all commands run.' + ' "debug" adds the output of all command run.')) parser.add_argument("--ansi-log", action="store_true", help=_("display log in ansi output mode")) @@ -389,7 +401,7 @@ def main(): args = parser.parse_args() # Validate arguments and apply defaults - validate_args(args) + process_args(args) # Set excepthook excepthook.verbose = args.verbose diff --git a/tests/check50_tests.py b/tests/check50_tests.py index c8a8b2e2..08ed40d9 100644 --- a/tests/check50_tests.py +++ b/tests/check50_tests.py @@ -384,5 +384,21 @@ def test_target_failing_dependency(self): self.assertEqual(output["results"][1]["name"], "exists5") +class TestRemoteException(Base): + def test_no_traceback(self): + # Check that bar (part of traceback) is not shown + process = pexpect.spawn(f"check50 --dev {CHECKS_DIRECTORY}/remote_exception_no_traceback") + self.assertRaises(pexpect.exceptions.EOF, lambda: process.expect("bar")) + + # Check that foo (the message) is shown + process = pexpect.spawn(f"check50 --dev {CHECKS_DIRECTORY}/remote_exception_no_traceback") + process.expect("foo") + + def test_traceback(self): + process = pexpect.spawn(f"check50 --dev {CHECKS_DIRECTORY}/remote_exception_traceback") + process.expect("bar") + process.expect("foo") + + if __name__ == "__main__": unittest.main() diff --git a/tests/checks/remote_exception_no_traceback/.cs50.yaml b/tests/checks/remote_exception_no_traceback/.cs50.yaml new file mode 100644 index 00000000..0745d8da --- /dev/null +++ b/tests/checks/remote_exception_no_traceback/.cs50.yaml @@ -0,0 +1,2 @@ +check50: + checks: check.py diff --git a/tests/checks/remote_exception_no_traceback/check.py b/tests/checks/remote_exception_no_traceback/check.py new file mode 100644 index 00000000..a96c9e1e --- /dev/null +++ b/tests/checks/remote_exception_no_traceback/check.py @@ -0,0 +1,22 @@ +import check50 +from check50.__main__ import RemoteCheckError + +json = { + "slug": "jelleas/foo/master", + "error": { + "type": "InvalidSlugError", + "value": "foo", + "traceback": [ + "Traceback (most recent call last):\n", + "bar\n" + ], + "actions": { + "show_traceback": False, + "message": "foo" + }, + "data": {} + }, + "version": "3.1.1" +} + +raise RemoteCheckError(json) diff --git a/tests/checks/remote_exception_traceback/.cs50.yaml b/tests/checks/remote_exception_traceback/.cs50.yaml new file mode 100644 index 00000000..0745d8da --- /dev/null +++ b/tests/checks/remote_exception_traceback/.cs50.yaml @@ -0,0 +1,2 @@ +check50: + checks: check.py diff --git a/tests/checks/remote_exception_traceback/check.py b/tests/checks/remote_exception_traceback/check.py new file mode 100644 index 00000000..4cde673a --- /dev/null +++ b/tests/checks/remote_exception_traceback/check.py @@ -0,0 +1,22 @@ +import check50 +from check50.__main__ import RemoteCheckError + +json = { + "slug": "jelleas/foo/master", + "error": { + "type": "InvalidSlugError", + "value": "foo", + "traceback": [ + "Traceback (most recent call last):\n", + "bar\n" + ], + "actions": { + "show_traceback": True, + "message": "foo" + }, + "data": {} + }, + "version": "3.1.1" +} + +raise RemoteCheckError(json) From 3724b526286339aa5d00d9202a147f3d2c2ad55f Mon Sep 17 00:00:00 2001 From: jelleas Date: Wed, 8 Jul 2020 18:58:17 +0200 Subject: [PATCH 39/64] clean up verbose --- check50/__main__.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/check50/__main__.py b/check50/__main__.py index a9b7b7d0..2c3f665b 100644 --- a/check50/__main__.py +++ b/check50/__main__.py @@ -142,7 +142,6 @@ def yes_no_prompt(prompt): # Assume we should print tracebacks until we get command line arguments -excepthook.verbose = True excepthook.output = "ansi" excepthook.output_file = None sys.excepthook = excepthook @@ -349,7 +348,7 @@ def main(): parser.add_argument("slug", help=_("prescribed identifier of work to check")) parser.add_argument("-d", "--dev", action="store_true", - help=_("run check50 in development mode (implies --offline, --verbose, and --log-level INFO).\n" + help=_("run check50 in development mode (implies --offline, --verbose, and --ansi-log).\n" "causes SLUG to be interpreted as a literal path to a checks package")) parser.add_argument("--offline", action="store_true", @@ -374,7 +373,7 @@ def main(): help=_("file to write output to")) parser.add_argument("-v", "--verbose", action="store_true", - help=_("shows the full traceback of any errors and shows print statements written in checks")) + help=_("shows any installed dependencies and shows print statements written in checks")) parser.add_argument("--log-level", action="store", choices=[level.name.lower() for level in LogLevel], @@ -404,7 +403,6 @@ def main(): process_args(args) # Set excepthook - excepthook.verbose = args.verbose excepthook.outputs = args.output excepthook.output_file = args.output_file excepthook.slug = args.slug From a1d0b16214c1b8a453f006bc4dd8f0cf8668566a Mon Sep 17 00:00:00 2001 From: jelleas Date: Thu, 9 Jul 2020 16:51:34 +0200 Subject: [PATCH 40/64] fix logging --- check50/__main__.py | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/check50/__main__.py b/check50/__main__.py index 5e8671bd..b411878e 100644 --- a/check50/__main__.py +++ b/check50/__main__.py @@ -45,7 +45,7 @@ class ColoredFormatter(logging.Formatter): } def __init__(self, fmt, use_color=True): - logging.Formatter.__init__(self, fmt=fmt) + super().__init__(fmt=fmt) self.use_color = use_color def format(self, record): @@ -142,7 +142,7 @@ def setup_logging(level): for logger in (logging.getLogger("lib50"), LOGGER): # Set verbosity level on the lib50 logger - logger.setLevel(getattr(logging, level.name)) + logger.setLevel(level.upper()) handler = logging.StreamHandler(sys.stderr) handler.setFormatter(ColoredFormatter("(%(levelname)s) %(message)s")) @@ -150,9 +150,8 @@ def setup_logging(level): # Direct all logs to sys.stderr logger.addHandler(handler) - # Don't animate the progressbar - if level > LogLevel.WARNING: - lib50.ProgressBar.DISABLED = True + # Don't animate the progressbar if LogLevel is either info or debug + lib50.ProgressBar.DISABLED = logger.level < LogLevel.WARNING def await_results(commit_hash, slug, pings=45, sleep=2): @@ -227,14 +226,15 @@ def raise_invalid_slug(slug, offline=False): raise internal.Error(msg) -def validate_args(args): +def process_args(args): """Validate arguments and apply defaults that are dependent on other arguments""" # dev implies offline, verbose, and log level "INFO" if not overwritten if args.dev: args.offline = True args.verbose = True - args.log_level = max(args.log_level, LogLevel.INFO) + if "ansi" in args.output: + args.ansi_log = True # offline implies local if args.offline: @@ -307,13 +307,13 @@ def main(): help=_("shows the full traceback of any errors")) parser.add_argument("--log-level", action="store", - default="WARNING", - choices=list(LogLevel.__members__), - type=lambda l: LogLevel.__members__[l.upper()], + default="warning", + choices=[level.name.lower() for level in LogLevel], + type=str.lower, help=_("sets the log level." - ' "WARNING" displays usage warnings' - ' "INFO" shows all commands run.' - ' "DEBUG" adds the output of all command run.')) + ' "warning" displays usage warnings' + ' "info" shows all commands run.' + ' "debug" adds the output of all command run.')) parser.add_argument("--ansi-log", action="store_true", help=_("display log in ansi output mode")) @@ -333,7 +333,7 @@ def main(): internal.slug = args.slug # Validate arguments and apply defaults - validate_args(args) + process_args(args) # Set excepthook internal._excepthook.outputs = args.output From 04f0500317c209a9c10b4fb2aa9c4253cf132563 Mon Sep 17 00:00:00 2001 From: jelleas Date: Fri, 10 Jul 2020 18:59:08 +0200 Subject: [PATCH 41/64] add support for int/float output to run.stdout --- check50/_api.py | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/check50/_api.py b/check50/_api.py index 8d523013..93d19c93 100644 --- a/check50/_api.py +++ b/check50/_api.py @@ -1,6 +1,7 @@ import hashlib import functools import os +import re import shlex import shutil import signal @@ -214,8 +215,11 @@ def stdout(self, output=None, str_output=None, regex=True, timeout=3): it returns ``self``. :param output: optional output to be expected from stdout, raises \ - :class:`check50.Failure` if no match - :type output: str + :class:`check50.Failure` if no match \ + In case output is a float or int, the following regex \ + is used to match just that number: r"(? Date: Fri, 10 Jul 2020 20:13:25 +0200 Subject: [PATCH 42/64] test .stdout(int/float) --- tests/api_tests.py | 49 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/tests/api_tests.py b/tests/api_tests.py index 4a3ca803..4308cad2 100644 --- a/tests/api_tests.py +++ b/tests/api_tests.py @@ -163,6 +163,55 @@ def test_out_no_regex(self): self.process.stdout(".o.", regex=False) self.assertFalse(self.process.process.isalive()) + def test_int(self): + self.write("print(123)") + self.runpy() + with self.assertRaises(check50.Failure): + self.process.stdout(1) + + self.write("print(21)") + self.runpy() + with self.assertRaises(check50.Failure): + self.process.stdout(1) + + self.write("print(1.0)") + self.runpy() + with self.assertRaises(check50.Failure): + self.process.stdout(1) + + self.write("print('a1b')") + self.runpy() + self.process.stdout(1) + + self.write("print(1)") + self.runpy() + self.process.stdout(1) + + def test_float(self): + self.write("print(1.01)") + self.runpy() + with self.assertRaises(check50.Failure): + self.process.stdout(1.0) + + self.write("print(21.0)") + self.runpy() + with self.assertRaises(check50.Failure): + self.process.stdout(1.0) + + self.write("print(1)") + self.runpy() + with self.assertRaises(check50.Failure): + self.process.stdout(1.0) + + self.write("print('a1.0b')") + self.runpy() + self.process.stdout(1.0) + + self.write("print(1.0)") + self.runpy() + self.process.stdout(1.0) + + class TestProcessStdoutFile(Base): def setUp(self): super().setUp() From 1bb90593120b3b618645f404df087aba15169e59 Mon Sep 17 00:00:00 2001 From: Chad Sharp Date: Fri, 10 Jul 2020 14:32:30 -0400 Subject: [PATCH 43/64] remove unnecessary import --- check50/py.py | 1 - 1 file changed, 1 deletion(-) diff --git a/check50/py.py b/check50/py.py index 9269d5f2..167de3f8 100644 --- a/check50/py.py +++ b/check50/py.py @@ -1,7 +1,6 @@ import importlib from pathlib import Path import py_compile -import traceback from . import internal from ._api import Failure, exists, log From e394f55cf1198d4b0b79a505f18e98700b480d30 Mon Sep 17 00:00:00 2001 From: jelleas Date: Fri, 10 Jul 2020 20:50:21 +0200 Subject: [PATCH 44/64] clarify verbose --- check50/__main__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/check50/__main__.py b/check50/__main__.py index b411878e..92d6b048 100644 --- a/check50/__main__.py +++ b/check50/__main__.py @@ -280,7 +280,7 @@ def main(): parser.add_argument("slug", help=_("prescribed identifier of work to check")) parser.add_argument("-d", "--dev", action="store_true", - help=_("run check50 in development mode (implies --offline, --verbose, and --log-level INFO).\n" + help=_("run check50 in development mode (implies --offline, --verbose, and --ansi-log).\n" "causes SLUG to be interpreted as a literal path to a checks package")) parser.add_argument("--offline", action="store_true", @@ -304,7 +304,7 @@ def main(): help=_("file to write output to")) parser.add_argument("-v", "--verbose", action="store_true", - help=_("shows the full traceback of any errors")) + help=_("shows any installed dependencies and shows print statements written in checks")) parser.add_argument("--log-level", action="store", default="warning", From d35f1dadc96dc22d4b3579188b9fba54a705038b Mon Sep 17 00:00:00 2001 From: jelleas Date: Fri, 10 Jul 2020 20:52:12 +0200 Subject: [PATCH 45/64] wording --- check50/__main__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/check50/__main__.py b/check50/__main__.py index 92d6b048..9dc37ba5 100644 --- a/check50/__main__.py +++ b/check50/__main__.py @@ -304,7 +304,7 @@ def main(): help=_("file to write output to")) parser.add_argument("-v", "--verbose", action="store_true", - help=_("shows any installed dependencies and shows print statements written in checks")) + help=_("shows which dependencies get installed (if any) and shows print statements in checks")) parser.add_argument("--log-level", action="store", default="warning", From 26dcd20f9e27615ae49587a7f3587dc8d60c2704 Mon Sep 17 00:00:00 2001 From: jelleas Date: Fri, 10 Jul 2020 20:54:50 +0200 Subject: [PATCH 46/64] wording v2 --- check50/__main__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/check50/__main__.py b/check50/__main__.py index 9dc37ba5..069eb249 100644 --- a/check50/__main__.py +++ b/check50/__main__.py @@ -304,7 +304,7 @@ def main(): help=_("file to write output to")) parser.add_argument("-v", "--verbose", action="store_true", - help=_("shows which dependencies get installed (if any) and shows print statements in checks")) + help=_("shows any print statements in checks if running locally, and shows which dependencies get installed")) parser.add_argument("--log-level", action="store", default="warning", From 57931480219a7d38823231edab818a55ff0fd2ac Mon Sep 17 00:00:00 2001 From: jelleas Date: Fri, 10 Jul 2020 22:03:10 +0200 Subject: [PATCH 47/64] check50.api_.number_regex --- check50/__init__.py | 3 ++- check50/_api.py | 32 ++++++++++++++++++++++++++++---- 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/check50/__init__.py b/check50/__init__.py index 2249d6d5..840825ef 100644 --- a/check50/__init__.py +++ b/check50/__init__.py @@ -38,6 +38,7 @@ def _setup_translation(): exists, hash, include, + number_regex, run, log, _log, hidden, @@ -48,5 +49,5 @@ def _setup_translation(): from .runner import check from pexpect import EOF -__all__ = ["import_checks", "data", "exists", "hash", "include", +__all__ = ["import_checks", "data", "exists", "hash", "include", "number_regex", "run", "log", "Failure", "Mismatch", "check", "EOF"] diff --git a/check50/_api.py b/check50/_api.py index 93d19c93..dfeb365f 100644 --- a/check50/_api.py +++ b/check50/_api.py @@ -1,5 +1,6 @@ import hashlib import functools +import numbers import os import re import shlex @@ -136,6 +137,29 @@ def import_checks(path): return mod +def number_regex(number): + """ + Create a regular expression to match the number exactly: + (? Date: Fri, 10 Jul 2020 22:10:16 +0200 Subject: [PATCH 48/64] simpler regex --- check50/_api.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/check50/_api.py b/check50/_api.py index dfeb365f..12e486d6 100644 --- a/check50/_api.py +++ b/check50/_api.py @@ -140,11 +140,11 @@ def import_checks(path): def number_regex(number): """ Create a regular expression to match the number exactly: - (? Date: Sat, 11 Jul 2020 20:12:27 -0400 Subject: [PATCH 49/64] add exceptions.py, make excepthook a class --- check50/__main__.py | 44 +++++++---------------- check50/exceptions.py | 83 +++++++++++++++++++++++++++++++++++++++++++ check50/internal.py | 81 ++--------------------------------------- check50/runner.py | 6 ++-- 4 files changed, 101 insertions(+), 113 deletions(-) create mode 100644 check50/exceptions.py diff --git a/check50/__main__.py b/check50/__main__.py index 069eb249..2b2b01dd 100644 --- a/check50/__main__.py +++ b/check50/__main__.py @@ -21,7 +21,7 @@ import requests import termcolor -from . import internal, renderer, __version__ +from . import exceptions, internal, renderer, __version__ from .runner import CheckRunner LOGGER = logging.getLogger("check50") @@ -59,25 +59,8 @@ def nullcontext(entry_result=None): yield entry_result -def yes_no_prompt(prompt): - """ - Raise a prompt, returns True if yes is entered, False if no is entered. - Will reraise prompt in case of any other reply. - """ - yes = {"yes", "ye", "y", ""} - no = {"no", "n"} - - reply = None - while reply not in yes and reply not in no: - reply = input(f"{prompt} [Y/n] ").lower() - - return reply in yes - -# Assume we should print tracebacks until we get command line arguments -internal._excepthook.outputs = ("ansi",) -internal._excepthook.output_file = None -sys.excepthook = internal._excepthook +exceptions.ExceptHook.initialize() def install_dependencies(dependencies, verbose=False): @@ -101,7 +84,7 @@ def install_dependencies(dependencies, verbose=False): try: subprocess.check_call(pip, stdout=stdout, stderr=stderr) except subprocess.CalledProcessError: - raise internal.Error(_("failed to install dependencies")) + raise exceptions.Error(_("failed to install dependencies")) # Reload sys.path, to find recently installed packages importlib.reload(site) @@ -123,7 +106,7 @@ def install_translations(config): def compile_checks(checks, prompt=False): # Prompt to replace __init__.py (compile destination) if prompt and os.path.exists(internal.check_dir / "__init__.py"): - if not yes_no_prompt("check50 will compile the YAML checks to __init__.py, are you sure you want to overwrite its contents?"): + if not internal._yes_no_prompt("check50 will compile the YAML checks to __init__.py, are you sure you want to overwrite its contents?"): raise Error("Aborting: could not overwrite to __init__.py") # Compile simple checks @@ -166,22 +149,22 @@ def await_results(commit_hash, slug, pings=45, sleep=2): results = res.json() if res.status_code not in [404, 200]: - raise internal.RemoteCheckError(results) + raise exceptions.RemoteCheckError(results) if res.status_code == 200 and results["received_at"] is not None: break time.sleep(sleep) else: # Terminate if no response - raise internal.Error( + raise exceptions.Error( _("check50 is taking longer than normal!\n" "See https://submit.cs50.io/check50/{} for more detail").format(commit_hash)) if not results["check50"]: - raise internal.RemoteCheckError(results) + raise exceptions.RemoteCheckError(results) if "error" in results["check50"]: - raise internal.RemoteCheckError(results["check50"]) + raise exceptions.RemoteCheckError(results["check50"]) # TODO: Should probably check payload["version"] here to make sure major version is same as __version__ # (otherwise we may not be able to parse results) @@ -202,7 +185,7 @@ def __call__(self, parser, namespace, values, option_string=None): try: lib50.logout() except lib50.Error: - raise internal.Error(_("failed to logout")) + raise exceptions.Error(_("failed to logout")) else: termcolor.cprint(_("logged out successfully"), "green") parser.exit() @@ -223,7 +206,7 @@ def raise_invalid_slug(slug, offline=False): msg += _("\nIf you are confident the slug is correct and you have an internet connection," \ " try running without --offline.") - raise internal.Error(msg) + raise exceptions.Error(msg) def process_args(args): @@ -336,8 +319,7 @@ def main(): process_args(args) # Set excepthook - internal._excepthook.outputs = args.output - internal._excepthook.output_file = args.output_file + exceptions.ExceptHook.initialize(args.output, args.output_file) # If remote, push files to GitHub and await results if not args.local: @@ -352,13 +334,13 @@ def main(): if args.dev: internal.check_dir = Path(internal.slug).expanduser().resolve() if not internal.check_dir.is_dir(): - raise internal.Error(_("{} is not a directory").format(internal.check_dir)) + raise exceptions.Error(_("{} is not a directory").format(internal.check_dir)) # Otherwise have lib50 create a local copy of slug else: try: internal.check_dir = lib50.local(internal.slug, offline=args.no_download_checks) except lib50.ConnectionError: - raise internal.Error(_("check50 could not retrieve checks from GitHub. Try running check50 again with --offline.").format(internal.slug)) + raise exceptions.Error(_("check50 could not retrieve checks from GitHub. Try running check50 again with --offline.").format(internal.slug)) except lib50.InvalidSlugError: raise_invalid_slug(internal.slug, offline=args.no_download_checks) diff --git a/check50/exceptions.py b/check50/exceptions.py new file mode 100644 index 00000000..2191e134 --- /dev/null +++ b/check50/exceptions.py @@ -0,0 +1,83 @@ +import json +import sys +import traceback + +import lib50 +import termcolor + +from . import internal, __version__ + +class Error(Exception): + """Exception for internal check50 errors.""" + pass + + +class RemoteCheckError(Error): + """An exception for errors that happen in check50's remote operation.""" + def __init__(self, remote_json): + super().__init__("check50 ran into an error while running checks! Please contact sysadmins@cs50.harvard.edu!") + self.payload = {"remote_json": remote_json} + + +class ExceptHook: + def __init__(self, outputs=("ansi",), output_file=None): + self.outputs = outputs + self.output_file = output_file + + def __call__(self, cls, exc, tb): + # If an error happened remotely, grab its traceback and message + if issubclass(cls, RemoteCheckError) and "error" in exc.payload["remote_json"]: + formatted_traceback = exc.payload["remote_json"]["error"]["traceback"] + show_traceback = exc.payload["remote_json"]["error"]["actions"]["show_traceback"] + message = exc.payload["remote_json"]["error"]["actions"]["message"] + # Otherwise, create the traceback and message from this error + else: + formatted_traceback = traceback.format_exception(cls, exc, tb) + show_traceback = False + + if (issubclass(cls, Error) or issubclass(cls, lib50.Error)) and exc.args: + message = str(exc) + elif issubclass(cls, FileNotFoundError): + message = _("{} not found").format(exc.filename) + elif issubclass(cls, KeyboardInterrupt): + message = _("check cancelled") + elif not issubclass(cls, Exception): + # Class is some other BaseException, better just let it go + return + else: + show_traceback = True + message = _("Sorry, something is wrong! check50 ran into an error.\n" \ + "Please let CS50 know by emailing the error above to sysadmins@cs50.harvard.edu.") + + # Output exception as json + if "json" in self.outputs: + ctxmanager = open(self.output_file, "w") if self.output_file else nullcontext(sys.stdout) + with ctxmanager as output_file: + json.dump({ + "slug": internal.slug, + "error": { + "type": cls.__name__, + "value": str(exc), + "traceback": formatted_traceback, + "actions": { + "show_traceback": show_traceback, + "message": message + }, + "data" : exc.payload if hasattr(exc, "payload") else {} + }, + "version": __version__ + }, output_file, indent=4) + output_file.write("\n") + + # Output exception to stderr + if "ansi" in self.outputs or "html" in self.outputs: + if show_traceback: + for line in formatted_traceback: + termcolor.cprint(line, end="", file=sys.stderr) + termcolor.cprint(message, "red", file=sys.stderr) + + sys.exit(1) + + @classmethod + def initialize(cls, *args, **kwargs): + sys.excepthook = cls(*args, **kwargs) diff --git a/check50/internal.py b/check50/internal.py index 34f3fb50..59aa2ab0 100644 --- a/check50/internal.py +++ b/check50/internal.py @@ -11,7 +11,8 @@ import lib50 -from . import _simple, __version__ +from . import _simple +from .exceptions import Error #: Directory containing the check and its associated files check_dir = None @@ -30,18 +31,6 @@ CONFIG_LOADER.scope("files", "include", "exclude", "require") -class Error(Exception): - """Exception for internal check50 errors.""" - pass - - -class RemoteCheckError(Error): - """An exception for errors that happen in check50's remote operation.""" - def __init__(self, remote_json): - super().__init__("check50 ran into an error while running checks! Please contact sysadmins@cs50.harvard.edu!") - self.payload = {"remote_json": remote_json} - - class Register: """ Class with which functions can be registered to run before / after checks. @@ -192,72 +181,6 @@ def import_file(name, path): return mod -def _excepthook(cls, exc, tb): - """ - check50's excepthook with configurable error output. - - :ivar _excepthook.outputs: in which format errors should be returned (can be multiple) - :vartype _excepthook.outputs: tuple of strings, any of "json", "ansi", "html" - :ivar _excepthook.output_file: file to which the output should be redirected - :vartype _excepthook.output_file: str or pathlib.Path - - See also: https://docs.python.org/3/library/sys.html#sys.excepthook - """ - - # If an error happened remotely, grab its traceback and message - if issubclass(cls, RemoteCheckError) and "error" in exc.payload["remote_json"]: - formatted_traceback = exc.payload["remote_json"]["error"]["traceback"] - show_traceback = exc.payload["remote_json"]["error"]["actions"]["show_traceback"] - message = exc.payload["remote_json"]["error"]["actions"]["message"] - # Otherwise, create the traceback and message from this error - else: - formatted_traceback = traceback.format_exception(cls, exc, tb) - show_traceback = False - - if (issubclass(cls, Error) or issubclass(cls, lib50.Error)) and exc.args: - message = str(exc) - elif issubclass(cls, FileNotFoundError): - message = _("{} not found").format(exc.filename) - elif issubclass(cls, KeyboardInterrupt): - message = _("check cancelled") - elif not issubclass(cls, Exception): - # Class is some other BaseException, better just let it go - return - else: - show_traceback = True - message = _("Sorry, something is wrong! check50 ran into an error.\n" \ - "Please let CS50 know by emailing the error above to sysadmins@cs50.harvard.edu.") - - # Output exception as json - if "json" in _excepthook.outputs: - ctxmanager = open(_excepthook.output_file, "w") if _excepthook.output_file else nullcontext(sys.stdout) - with ctxmanager as output_file: - json.dump({ - "slug": slug, - "error": { - "type": cls.__name__, - "value": str(exc), - "traceback": formatted_traceback, - "actions": { - "show_traceback": show_traceback, - "message": message - }, - "data" : exc.payload if hasattr(exc, "payload") else {} - }, - "version": __version__ - }, output_file, indent=4) - output_file.write("\n") - - # Output exception to stderr - if "ansi" in _excepthook.outputs or "html" in _excepthook.outputs: - if show_traceback: - for line in formatted_traceback: - termcolor.cprint(line, end="", file=sys.stderr) - termcolor.cprint(message, "red", file=sys.stderr) - - sys.exit(1) - - def _yes_no_prompt(prompt): """ Raise a prompt, returns True if yes is entered, False if no is entered. diff --git a/check50/runner.py b/check50/runner.py index 014cddf0..a378fe38 100644 --- a/check50/runner.py +++ b/check50/runner.py @@ -16,7 +16,7 @@ import attr -from . import internal +from . import exceptions, internal, __version__ from ._api import log, Failure, _copy, _log, _data _check_names = [] @@ -303,8 +303,8 @@ class run_check: CROSS_PROCESS_ATTRIBUTES = ( "internal.check_dir", "internal.slug", - "internal._excepthook.outputs", - "internal._excepthook.output_file" + "sys.excepthook", + "__version__" ) def __init__(self, check_name, spec, checks_root, state=None): From e58f815184cedf505782a3253eed333f4a60629d Mon Sep 17 00:00:00 2001 From: Chad Sharp Date: Sat, 11 Jul 2020 20:17:24 -0400 Subject: [PATCH 50/64] fix tests --- tests/checks/remote_exception_no_traceback/check.py | 2 +- tests/checks/remote_exception_traceback/check.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/checks/remote_exception_no_traceback/check.py b/tests/checks/remote_exception_no_traceback/check.py index c94ed443..c24a1f0e 100644 --- a/tests/checks/remote_exception_no_traceback/check.py +++ b/tests/checks/remote_exception_no_traceback/check.py @@ -1,5 +1,5 @@ import check50 -from check50.internal import RemoteCheckError +from check50.exceptions import RemoteCheckError json = { "slug": "jelleas/foo/master", diff --git a/tests/checks/remote_exception_traceback/check.py b/tests/checks/remote_exception_traceback/check.py index 84763c3d..9ae4281b 100644 --- a/tests/checks/remote_exception_traceback/check.py +++ b/tests/checks/remote_exception_traceback/check.py @@ -1,5 +1,5 @@ import check50 -from check50.internal import RemoteCheckError +from check50.exceptions import RemoteCheckError json = { "slug": "jelleas/foo/master", From 6aabc9a8f891a01edcafe61712ca2c52d629c7df Mon Sep 17 00:00:00 2001 From: Chad Sharp Date: Sat, 11 Jul 2020 20:22:46 -0400 Subject: [PATCH 51/64] s/exceptions/_exceptions/g --- check50/__main__.py | 24 +++--- check50/exceptions.py | 83 ------------------- check50/internal.py | 15 ++-- check50/runner.py | 2 +- .../remote_exception_no_traceback/check.py | 2 +- .../remote_exception_traceback/check.py | 2 +- 6 files changed, 22 insertions(+), 106 deletions(-) delete mode 100644 check50/exceptions.py diff --git a/check50/__main__.py b/check50/__main__.py index 2b2b01dd..561b7c70 100644 --- a/check50/__main__.py +++ b/check50/__main__.py @@ -21,7 +21,7 @@ import requests import termcolor -from . import exceptions, internal, renderer, __version__ +from . import _exceptions, internal, renderer, __version__ from .runner import CheckRunner LOGGER = logging.getLogger("check50") @@ -60,7 +60,7 @@ def nullcontext(entry_result=None): -exceptions.ExceptHook.initialize() +_exceptions.ExceptHook.initialize() def install_dependencies(dependencies, verbose=False): @@ -84,7 +84,7 @@ def install_dependencies(dependencies, verbose=False): try: subprocess.check_call(pip, stdout=stdout, stderr=stderr) except subprocess.CalledProcessError: - raise exceptions.Error(_("failed to install dependencies")) + raise _exceptions.Error(_("failed to install dependencies")) # Reload sys.path, to find recently installed packages importlib.reload(site) @@ -149,22 +149,22 @@ def await_results(commit_hash, slug, pings=45, sleep=2): results = res.json() if res.status_code not in [404, 200]: - raise exceptions.RemoteCheckError(results) + raise _exceptions.RemoteCheckError(results) if res.status_code == 200 and results["received_at"] is not None: break time.sleep(sleep) else: # Terminate if no response - raise exceptions.Error( + raise _exceptions.Error( _("check50 is taking longer than normal!\n" "See https://submit.cs50.io/check50/{} for more detail").format(commit_hash)) if not results["check50"]: - raise exceptions.RemoteCheckError(results) + raise _exceptions.RemoteCheckError(results) if "error" in results["check50"]: - raise exceptions.RemoteCheckError(results["check50"]) + raise _exceptions.RemoteCheckError(results["check50"]) # TODO: Should probably check payload["version"] here to make sure major version is same as __version__ # (otherwise we may not be able to parse results) @@ -185,7 +185,7 @@ def __call__(self, parser, namespace, values, option_string=None): try: lib50.logout() except lib50.Error: - raise exceptions.Error(_("failed to logout")) + raise _exceptions.Error(_("failed to logout")) else: termcolor.cprint(_("logged out successfully"), "green") parser.exit() @@ -206,7 +206,7 @@ def raise_invalid_slug(slug, offline=False): msg += _("\nIf you are confident the slug is correct and you have an internet connection," \ " try running without --offline.") - raise exceptions.Error(msg) + raise _exceptions.Error(msg) def process_args(args): @@ -319,7 +319,7 @@ def main(): process_args(args) # Set excepthook - exceptions.ExceptHook.initialize(args.output, args.output_file) + _exceptions.ExceptHook.initialize(args.output, args.output_file) # If remote, push files to GitHub and await results if not args.local: @@ -334,13 +334,13 @@ def main(): if args.dev: internal.check_dir = Path(internal.slug).expanduser().resolve() if not internal.check_dir.is_dir(): - raise exceptions.Error(_("{} is not a directory").format(internal.check_dir)) + raise _exceptions.Error(_("{} is not a directory").format(internal.check_dir)) # Otherwise have lib50 create a local copy of slug else: try: internal.check_dir = lib50.local(internal.slug, offline=args.no_download_checks) except lib50.ConnectionError: - raise exceptions.Error(_("check50 could not retrieve checks from GitHub. Try running check50 again with --offline.").format(internal.slug)) + raise _exceptions.Error(_("check50 could not retrieve checks from GitHub. Try running check50 again with --offline.").format(internal.slug)) except lib50.InvalidSlugError: raise_invalid_slug(internal.slug, offline=args.no_download_checks) diff --git a/check50/exceptions.py b/check50/exceptions.py deleted file mode 100644 index 2191e134..00000000 --- a/check50/exceptions.py +++ /dev/null @@ -1,83 +0,0 @@ -import json -import sys -import traceback - -import lib50 -import termcolor - -from . import internal, __version__ - -class Error(Exception): - """Exception for internal check50 errors.""" - pass - - -class RemoteCheckError(Error): - """An exception for errors that happen in check50's remote operation.""" - def __init__(self, remote_json): - super().__init__("check50 ran into an error while running checks! Please contact sysadmins@cs50.harvard.edu!") - self.payload = {"remote_json": remote_json} - - -class ExceptHook: - def __init__(self, outputs=("ansi",), output_file=None): - self.outputs = outputs - self.output_file = output_file - - def __call__(self, cls, exc, tb): - # If an error happened remotely, grab its traceback and message - if issubclass(cls, RemoteCheckError) and "error" in exc.payload["remote_json"]: - formatted_traceback = exc.payload["remote_json"]["error"]["traceback"] - show_traceback = exc.payload["remote_json"]["error"]["actions"]["show_traceback"] - message = exc.payload["remote_json"]["error"]["actions"]["message"] - # Otherwise, create the traceback and message from this error - else: - formatted_traceback = traceback.format_exception(cls, exc, tb) - show_traceback = False - - if (issubclass(cls, Error) or issubclass(cls, lib50.Error)) and exc.args: - message = str(exc) - elif issubclass(cls, FileNotFoundError): - message = _("{} not found").format(exc.filename) - elif issubclass(cls, KeyboardInterrupt): - message = _("check cancelled") - elif not issubclass(cls, Exception): - # Class is some other BaseException, better just let it go - return - else: - show_traceback = True - message = _("Sorry, something is wrong! check50 ran into an error.\n" \ - "Please let CS50 know by emailing the error above to sysadmins@cs50.harvard.edu.") - - # Output exception as json - if "json" in self.outputs: - ctxmanager = open(self.output_file, "w") if self.output_file else nullcontext(sys.stdout) - with ctxmanager as output_file: - json.dump({ - "slug": internal.slug, - "error": { - "type": cls.__name__, - "value": str(exc), - "traceback": formatted_traceback, - "actions": { - "show_traceback": show_traceback, - "message": message - }, - "data" : exc.payload if hasattr(exc, "payload") else {} - }, - "version": __version__ - }, output_file, indent=4) - output_file.write("\n") - - # Output exception to stderr - if "ansi" in self.outputs or "html" in self.outputs: - if show_traceback: - for line in formatted_traceback: - termcolor.cprint(line, end="", file=sys.stderr) - termcolor.cprint(message, "red", file=sys.stderr) - - sys.exit(1) - - @classmethod - def initialize(cls, *args, **kwargs): - sys.excepthook = cls(*args, **kwargs) diff --git a/check50/internal.py b/check50/internal.py index 59aa2ab0..0bd9c97f 100644 --- a/check50/internal.py +++ b/check50/internal.py @@ -11,8 +11,7 @@ import lib50 -from . import _simple -from .exceptions import Error +from . import _simple, _exceptions #: Directory containing the check and its associated files check_dir = None @@ -47,7 +46,7 @@ def after_check(self, func): :param func: callback to run after check :raises check50.internal.Error: if called when no check is being run""" if not check_running: - raise Error("cannot register callback to run after check when no check is running") + raise _exceptions.Error("cannot register callback to run after check when no check is running") self._after_checks.append(func) def after_every(self, func): @@ -56,7 +55,7 @@ def after_every(self, func): :param func: callback to be run after every check :raises check50.internal.Error: if called when a check is being run""" if check_running: - raise Error("cannot register callback to run after every check when check is running") + raise _exceptions.Error("cannot register callback to run after every check when check is running") self._after_everies.append(func) def before_every(self, func): @@ -66,7 +65,7 @@ def before_every(self, func): :raises check50.internal.Error: if called when a check is being run""" if check_running: - raise Error("cannot register callback to run before every check when check is running") + raise _exceptions.Error("cannot register callback to run before every check when check is running") self._before_everies.append(func) def __enter__(self): @@ -117,14 +116,14 @@ def load_config(check_dir): try: config_file = lib50.config.get_config_filepath(check_dir) except lib50.Error: - raise Error(_("Invalid slug for check50. Did you mean something else?")) + raise _exceptions.Error(_("Invalid slug for check50. Did you mean something else?")) # Load config with open(config_file) as f: try: config = CONFIG_LOADER.load(f.read()) except lib50.InvalidConfigError: - raise Error(_("Invalid slug for check50. Did you mean something else?")) + raise _exceptions.Error(_("Invalid slug for check50. Did you mean something else?")) # Update the config with defaults if isinstance(config, dict): @@ -157,7 +156,7 @@ def compile_checks(checks, prompt=False, out_file="__init__.py"): # Prompt to replace __init__.py (compile destination) if prompt and file_path.exists(): if not _yes_no_prompt("check50 will compile the YAML checks to __init__.py, are you sure you want to overwrite its contents?"): - raise Error("Aborting: could not overwrite to __init__.py") + raise _exceptions.Error("Aborting: could not overwrite to __init__.py") # Compile simple checks with open(check_dir / out_file, "w") as f: diff --git a/check50/runner.py b/check50/runner.py index a378fe38..4b23dc31 100644 --- a/check50/runner.py +++ b/check50/runner.py @@ -16,7 +16,7 @@ import attr -from . import exceptions, internal, __version__ +from . import internal, __version__ from ._api import log, Failure, _copy, _log, _data _check_names = [] diff --git a/tests/checks/remote_exception_no_traceback/check.py b/tests/checks/remote_exception_no_traceback/check.py index c24a1f0e..9ce01487 100644 --- a/tests/checks/remote_exception_no_traceback/check.py +++ b/tests/checks/remote_exception_no_traceback/check.py @@ -1,5 +1,5 @@ import check50 -from check50.exceptions import RemoteCheckError +from check50._exceptions import RemoteCheckError json = { "slug": "jelleas/foo/master", diff --git a/tests/checks/remote_exception_traceback/check.py b/tests/checks/remote_exception_traceback/check.py index 9ae4281b..85c5d8f9 100644 --- a/tests/checks/remote_exception_traceback/check.py +++ b/tests/checks/remote_exception_traceback/check.py @@ -1,5 +1,5 @@ import check50 -from check50.exceptions import RemoteCheckError +from check50._exceptions import RemoteCheckError json = { "slug": "jelleas/foo/master", From f067db391463122c52bd40845c80b25f043a1c29 Mon Sep 17 00:00:00 2001 From: jelleas Date: Mon, 13 Jul 2020 16:32:08 +0200 Subject: [PATCH 52/64] don't enum.auto() --- check50/__main__.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/check50/__main__.py b/check50/__main__.py index 561b7c70..b1380d08 100644 --- a/check50/__main__.py +++ b/check50/__main__.py @@ -30,10 +30,10 @@ class LogLevel(enum.IntEnum): - ERROR = enum.auto() - WARNING = enum.auto() - INFO = enum.auto() - DEBUG = enum.auto() + DEBUG = logging.DEBUG + INFO = logging.INFO + WARNING = logging.WARNING + ERROR = logging.ERROR class ColoredFormatter(logging.Formatter): @@ -128,7 +128,7 @@ def setup_logging(level): logger.setLevel(level.upper()) handler = logging.StreamHandler(sys.stderr) - handler.setFormatter(ColoredFormatter("(%(levelname)s) %(message)s")) + handler.setFormatter(ColoredFormatter("(%(levelname)s) %(message)s", use_color=sys.stderr.isatty())) # Direct all logs to sys.stderr logger.addHandler(handler) From ba0890cfe8d81ddc76a7d0f0c181e2684112b531 Mon Sep 17 00:00:00 2001 From: jelleas Date: Mon, 13 Jul 2020 16:37:22 +0200 Subject: [PATCH 53/64] reintroduce _exceptions.py --- check50/_exceptions.py | 83 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) create mode 100644 check50/_exceptions.py diff --git a/check50/_exceptions.py b/check50/_exceptions.py new file mode 100644 index 00000000..2191e134 --- /dev/null +++ b/check50/_exceptions.py @@ -0,0 +1,83 @@ +import json +import sys +import traceback + +import lib50 +import termcolor + +from . import internal, __version__ + +class Error(Exception): + """Exception for internal check50 errors.""" + pass + + +class RemoteCheckError(Error): + """An exception for errors that happen in check50's remote operation.""" + def __init__(self, remote_json): + super().__init__("check50 ran into an error while running checks! Please contact sysadmins@cs50.harvard.edu!") + self.payload = {"remote_json": remote_json} + + +class ExceptHook: + def __init__(self, outputs=("ansi",), output_file=None): + self.outputs = outputs + self.output_file = output_file + + def __call__(self, cls, exc, tb): + # If an error happened remotely, grab its traceback and message + if issubclass(cls, RemoteCheckError) and "error" in exc.payload["remote_json"]: + formatted_traceback = exc.payload["remote_json"]["error"]["traceback"] + show_traceback = exc.payload["remote_json"]["error"]["actions"]["show_traceback"] + message = exc.payload["remote_json"]["error"]["actions"]["message"] + # Otherwise, create the traceback and message from this error + else: + formatted_traceback = traceback.format_exception(cls, exc, tb) + show_traceback = False + + if (issubclass(cls, Error) or issubclass(cls, lib50.Error)) and exc.args: + message = str(exc) + elif issubclass(cls, FileNotFoundError): + message = _("{} not found").format(exc.filename) + elif issubclass(cls, KeyboardInterrupt): + message = _("check cancelled") + elif not issubclass(cls, Exception): + # Class is some other BaseException, better just let it go + return + else: + show_traceback = True + message = _("Sorry, something is wrong! check50 ran into an error.\n" \ + "Please let CS50 know by emailing the error above to sysadmins@cs50.harvard.edu.") + + # Output exception as json + if "json" in self.outputs: + ctxmanager = open(self.output_file, "w") if self.output_file else nullcontext(sys.stdout) + with ctxmanager as output_file: + json.dump({ + "slug": internal.slug, + "error": { + "type": cls.__name__, + "value": str(exc), + "traceback": formatted_traceback, + "actions": { + "show_traceback": show_traceback, + "message": message + }, + "data" : exc.payload if hasattr(exc, "payload") else {} + }, + "version": __version__ + }, output_file, indent=4) + output_file.write("\n") + + # Output exception to stderr + if "ansi" in self.outputs or "html" in self.outputs: + if show_traceback: + for line in formatted_traceback: + termcolor.cprint(line, end="", file=sys.stderr) + termcolor.cprint(message, "red", file=sys.stderr) + + sys.exit(1) + + @classmethod + def initialize(cls, *args, **kwargs): + sys.excepthook = cls(*args, **kwargs) From f3a6dff1cdf1650b0534418e17566dfb5e424c2e Mon Sep 17 00:00:00 2001 From: Chad Sharp Date: Mon, 13 Jul 2020 17:37:40 -0400 Subject: [PATCH 54/64] remove --verbose --- check50/__main__.py | 65 ++++++++++++++++++++++++--------------------- 1 file changed, 34 insertions(+), 31 deletions(-) diff --git a/check50/__main__.py b/check50/__main__.py index b1380d08..4e5190bd 100644 --- a/check50/__main__.py +++ b/check50/__main__.py @@ -63,12 +63,11 @@ def nullcontext(entry_result=None): _exceptions.ExceptHook.initialize() -def install_dependencies(dependencies, verbose=False): +def install_dependencies(dependencies): """Install all packages in dependency list via pip.""" if not dependencies: return - stdout = stderr = None if verbose else subprocess.DEVNULL with tempfile.TemporaryDirectory() as req_dir: req_file = Path(req_dir) / "requirements.txt" @@ -82,10 +81,12 @@ def install_dependencies(dependencies, verbose=False): pip.append("--user") try: - subprocess.check_call(pip, stdout=stdout, stderr=stderr) + output = subprocess.check_output(pip, stderr=subprocess.STDOUT) except subprocess.CalledProcessError: raise _exceptions.Error(_("failed to install dependencies")) + LOGGER.info(output) + # Reload sys.path, to find recently installed packages importlib.reload(site) @@ -215,10 +216,12 @@ def process_args(args): # dev implies offline, verbose, and log level "INFO" if not overwritten if args.dev: args.offline = True - args.verbose = True if "ansi" in args.output: args.ansi_log = True + if not args.log_level: + args.log_level = "info" + # offline implies local if args.offline: args.no_install_dependencies = True @@ -226,7 +229,7 @@ def process_args(args): args.local = True if not args.log_level: - args.log_level = LogLevel.WARNING + args.log_level = "warning" # Setup logging for lib50 setup_logging(args.log_level) @@ -256,6 +259,18 @@ def process_args(args): LOGGER.warning(_("--ansi-log has no effect when ansi is not among the output formats")) +class LoggerWriter: + def __init__(self, logger, level): + self.logger = logger + self.level = level + + def write(self, message): + if message != "\n": + self.logger.log(self.level, message) + + def flush(self): + pass + def main(): parser = argparse.ArgumentParser(prog="check50") @@ -263,7 +278,7 @@ def main(): parser.add_argument("slug", help=_("prescribed identifier of work to check")) parser.add_argument("-d", "--dev", action="store_true", - help=_("run check50 in development mode (implies --offline, --verbose, and --ansi-log).\n" + help=_("run check50 in development mode (implies --offline, and --log-level info).\n" "causes SLUG to be interpreted as a literal path to a checks package")) parser.add_argument("--offline", action="store_true", @@ -285,12 +300,8 @@ def main(): action="store", metavar="FILE", help=_("file to write output to")) - parser.add_argument("-v", "--verbose", - action="store_true", - help=_("shows any print statements in checks if running locally, and shows which dependencies get installed")) parser.add_argument("--log-level", action="store", - default="warning", choices=[level.name.lower() for level in LogLevel], type=str.lower, help=_("sets the log level." @@ -354,33 +365,25 @@ def main(): install_translations(config["translations"]) if not args.no_install_dependencies: - install_dependencies(config["dependencies"], verbose=args.verbose) + install_dependencies(config["dependencies"]) checks_file = (internal.check_dir / config["checks"]).resolve() # Have lib50 decide which files to include included = lib50.files(config.get("files"))[0] - # Redirect stdout to devnull if verbose or log level is set - should_redirect_devnull = args.verbose or args.log_level - with open(os.devnull, "w") if should_redirect_devnull else nullcontext() as devnull: - if should_redirect_devnull: - stdout = stderr = devnull - else: - stdout = sys.stdout - stderr = sys.stderr - - # Create a working_area (temp dir) named - with all included student files - with lib50.working_area(included, name='-') as working_area, \ - contextlib.redirect_stdout(stdout), \ - contextlib.redirect_stderr(stderr): - - check_results = CheckRunner(checks_file).run(included, working_area, args.target) - results = { - "slug": internal.slug, - "results": [attr.asdict(result) for result in check_results], - "version": __version__ - } + + # Create a working_area (temp dir) named - with all included student files + with lib50.working_area(included, name='-') as working_area, \ + contextlib.redirect_stdout(LoggerWriter(LOGGER, logging.INFO)), \ + contextlib.redirect_stderr(LoggerWriter(LOGGER, logging.INFO)): + + check_results = CheckRunner(checks_file).run(included, working_area, args.target) + results = { + "slug": internal.slug, + "results": [attr.asdict(result) for result in check_results], + "version": __version__ + } LOGGER.debug(results) From 01e06c3d65e387e05bc26c967742f90560fe3d7b Mon Sep 17 00:00:00 2001 From: jelleas Date: Tue, 14 Jul 2020 16:03:00 +0200 Subject: [PATCH 55/64] -h message --- check50/__main__.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/check50/__main__.py b/check50/__main__.py index 4e5190bd..4881757e 100644 --- a/check50/__main__.py +++ b/check50/__main__.py @@ -273,13 +273,13 @@ def flush(self): def main(): - parser = argparse.ArgumentParser(prog="check50") + parser = argparse.ArgumentParser(prog="check50", formatter_class=argparse.RawTextHelpFormatter) parser.add_argument("slug", help=_("prescribed identifier of work to check")) parser.add_argument("-d", "--dev", action="store_true", help=_("run check50 in development mode (implies --offline, and --log-level info).\n" - "causes SLUG to be interpreted as a literal path to a checks package")) + "causes slug to be interpreted as a literal path to a checks package.")) parser.add_argument("--offline", action="store_true", help=_("run checks completely offline (implies --local, --no-download-checks and --no-install-dependencies)")) @@ -304,10 +304,9 @@ def main(): action="store", choices=[level.name.lower() for level in LogLevel], type=str.lower, - help=_("sets the log level." - ' "warning" displays usage warnings' - ' "info" shows all commands run.' - ' "debug" adds the output of all command run.')) + help=_('warning; displays usage warnings.' + '\ninfo: adds all commands run, any locally installed dependencies and print messages.' + '\ndebug: adds the output of all commands run.')) parser.add_argument("--ansi-log", action="store_true", help=_("display log in ansi output mode")) From a9f5a1d3522777790419feb4ad51a60f3803daff Mon Sep 17 00:00:00 2001 From: jelleas Date: Tue, 14 Jul 2020 17:31:43 +0200 Subject: [PATCH 56/64] .stdout negative numbers --- check50/_api.py | 16 +++++++++++----- tests/api_tests.py | 19 +++++++++++++++++++ 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/check50/_api.py b/check50/_api.py index 12e486d6..a205cd66 100644 --- a/check50/_api.py +++ b/check50/_api.py @@ -140,12 +140,15 @@ def import_checks(path): def number_regex(number): """ Create a regular expression to match the number exactly: - (? Date: Tue, 14 Jul 2020 17:43:02 +0200 Subject: [PATCH 57/64] check50.regex.decimal --- check50/__init__.py | 4 ++-- check50/_api.py | 48 ++++++++++++++++++++++----------------------- 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/check50/__init__.py b/check50/__init__.py index 840825ef..9d51ddc6 100644 --- a/check50/__init__.py +++ b/check50/__init__.py @@ -38,7 +38,7 @@ def _setup_translation(): exists, hash, include, - number_regex, + regex, run, log, _log, hidden, @@ -49,5 +49,5 @@ def _setup_translation(): from .runner import check from pexpect import EOF -__all__ = ["import_checks", "data", "exists", "hash", "include", "number_regex", +__all__ = ["import_checks", "data", "exists", "hash", "include", "regex", "run", "log", "Failure", "Mismatch", "check", "EOF"] diff --git a/check50/_api.py b/check50/_api.py index a205cd66..5282f5f1 100644 --- a/check50/_api.py +++ b/check50/_api.py @@ -137,33 +137,33 @@ def import_checks(path): return mod -def number_regex(number): - """ - Create a regular expression to match the number exactly: - In case of a positive number: - (?= 0 else "" + return fr"{negative_lookbehind}{re.escape(str(number))}(?!(\.?\d))" class run: @@ -285,7 +285,7 @@ def stdout(self, output=None, str_output=None, regex=True, timeout=3): # In case output is an int/float, use a regex to match exactly that int/float if isinstance(output, numbers.Number): regex = True - output = number_regex(output) + output = globals()["regex"].decimal(output) expect = self.process.expect if regex else self.process.expect_exact From 5eadcb3f0d36990436c84175d0c75cc5aceda08e Mon Sep 17 00:00:00 2001 From: Chad Sharp Date: Tue, 14 Jul 2020 13:50:27 -0400 Subject: [PATCH 58/64] add regex module, Missing to import --- check50/__init__.py | 6 +++--- check50/_api.py | 30 +----------------------------- check50/regex.py | 26 ++++++++++++++++++++++++++ 3 files changed, 30 insertions(+), 32 deletions(-) create mode 100644 check50/regex.py diff --git a/check50/__init__.py b/check50/__init__.py index 9d51ddc6..558597b0 100644 --- a/check50/__init__.py +++ b/check50/__init__.py @@ -38,16 +38,16 @@ def _setup_translation(): exists, hash, include, - regex, run, log, _log, hidden, - Failure, Mismatch + Failure, Mismatch, Missing ) +from . import regex from .runner import check from pexpect import EOF __all__ = ["import_checks", "data", "exists", "hash", "include", "regex", - "run", "log", "Failure", "Mismatch", "check", "EOF"] + "run", "log", "Failure", "Mismatch", "Missing", "check", "EOF"] diff --git a/check50/_api.py b/check50/_api.py index 8aef602e..efd45bee 100644 --- a/check50/_api.py +++ b/check50/_api.py @@ -12,7 +12,7 @@ import pexpect from pexpect.exceptions import EOF, TIMEOUT -from . import internal +from . import internal, regex _log = [] internal.register.before_every(_log.clear) @@ -137,34 +137,6 @@ def import_checks(path): return mod -class regex: - @staticmethod - def decimal(number): - """ - Create a regular expression to match the number exactly: - In case of a positive number: - (?= 0 else "" - return fr"{negative_lookbehind}{re.escape(str(number))}(?!(\.?\d))" - class run: """ diff --git a/check50/regex.py b/check50/regex.py new file mode 100644 index 00000000..8c948d38 --- /dev/null +++ b/check50/regex.py @@ -0,0 +1,26 @@ +def decimal(number): + """ + Create a regular expression to match the number exactly: + In case of a positive number: + (?= 0 else "" + return fr"{negative_lookbehind}{re.escape(str(number))}(?!(\.?\d))" + From ab93474bd0d79a810e6e964e3b49b6e7bcee2254 Mon Sep 17 00:00:00 2001 From: Chad Sharp Date: Tue, 14 Jul 2020 14:03:53 -0400 Subject: [PATCH 59/64] fix missing import --- check50/regex.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/check50/regex.py b/check50/regex.py index 8c948d38..b914dcf0 100644 --- a/check50/regex.py +++ b/check50/regex.py @@ -1,3 +1,6 @@ +import re + + def decimal(number): """ Create a regular expression to match the number exactly: From edba0af1881bf52951c2151ec762fb1c5b14b641 Mon Sep 17 00:00:00 2001 From: Jelle van Assema Date: Tue, 14 Jul 2020 20:58:23 +0200 Subject: [PATCH 60/64] ; => : --- check50/__main__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/check50/__main__.py b/check50/__main__.py index 11f09e8f..2c888927 100644 --- a/check50/__main__.py +++ b/check50/__main__.py @@ -304,7 +304,7 @@ def main(): action="store", choices=[level.name.lower() for level in LogLevel], type=str.lower, - help=_('warning; displays usage warnings.' + help=_('warning: displays usage warnings.' '\ninfo: adds all commands run, any locally installed dependencies and print messages.' '\ndebug: adds the output of all commands run.')) parser.add_argument("--ansi-log", From fd73d91bf70248bc57bd0f4b5edb04afdee50628 Mon Sep 17 00:00:00 2001 From: jelleas Date: Tue, 14 Jul 2020 22:43:34 +0200 Subject: [PATCH 61/64] docs for check50.regex --- check50/regex.py | 18 +++++++++++------- docs/source/api.rst | 9 +++++++++ 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/check50/regex.py b/check50/regex.py index b914dcf0..215c65a8 100644 --- a/check50/regex.py +++ b/check50/regex.py @@ -4,14 +4,19 @@ def decimal(number): """ Create a regular expression to match the number exactly: - In case of a positive number: - (?= 0 else "" return fr"{negative_lookbehind}{re.escape(str(number))}(?!(\.?\d))" - diff --git a/docs/source/api.rst b/docs/source/api.rst index 6a778ba8..dc76c331 100644 --- a/docs/source/api.rst +++ b/docs/source/api.rst @@ -5,18 +5,21 @@ API docs .. _check50: + check50 ******* .. automodule:: check50 :members: + check50.c ********** .. automodule:: check50.c :members: + check50.flask **************** @@ -30,6 +33,12 @@ check50.py :members: +check50.regex +************** +.. automodule:: check50.regex + :members: + + check50.internal ***************** From fb49a1cb1e5e4e2a4e3874e2d612792d01145ffc Mon Sep 17 00:00:00 2001 From: jelleas Date: Tue, 14 Jul 2020 23:58:25 +0200 Subject: [PATCH 62/64] fix tests --- tests/check50_tests.py | 2 +- tests/checks/stdout_py/check.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/check50_tests.py b/tests/check50_tests.py index 352ef2ab..3c5e7adf 100644 --- a/tests/check50_tests.py +++ b/tests/check50_tests.py @@ -111,7 +111,7 @@ def test_stdout_timeout(self): process.expect_exact(":)") process.expect_exact("foo.py exists") process.expect_exact(":(") - process.expect_exact("did not find \"hello\" within 3 seconds") + process.expect_exact("check50 waited 1 seconds for the output of the program") process.close(force=True) class TestStdinPy(Base): diff --git a/tests/checks/stdout_py/check.py b/tests/checks/stdout_py/check.py index 9a47dea2..bd9666a6 100644 --- a/tests/checks/stdout_py/check.py +++ b/tests/checks/stdout_py/check.py @@ -8,4 +8,4 @@ def exists(): @check50.check(exists) def prints_hello(): """prints hello""" - check50.run("python3 foo.py").stdout("hello") + check50.run("python3 foo.py").stdout("hello", show_timeout=True, timeout=1) From 1899096bee05f7643228dff00ad0afb2712217f3 Mon Sep 17 00:00:00 2001 From: Julius Marozas Date: Mon, 5 Oct 2020 13:22:10 +0300 Subject: [PATCH 63/64] Replace bs4 dependency with beautifulsoup4 --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 6feb63a9..c6547179 100644 --- a/setup.py +++ b/setup.py @@ -17,7 +17,7 @@ message_extractors = { 'check50': [('**.py', 'python', None),], }, - install_requires=["attrs>=18", "bs4>=0", "pexpect>=4.6", "lib50>=2,<4", "pyyaml>=3.10", "requests>=2.19", "termcolor>=1.1", "jinja2>=2.10"], + install_requires=["attrs>=18", "beautifulsoup4>=0", "pexpect>=4.6", "lib50>=2,<4", "pyyaml>=3.10", "requests>=2.19", "termcolor>=1.1", "jinja2>=2.10"], extras_require = { "develop": ["sphinx", "sphinx-autobuild", "sphinx_rtd_theme"] }, From 03d5bb037c9323b8ab9d45cdd4d54613a2f8d924 Mon Sep 17 00:00:00 2001 From: Kareem Zidane Date: Sun, 11 Oct 2020 08:55:10 -0400 Subject: [PATCH 64/64] Update setup.py --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index c6547179..a9fe0e31 100644 --- a/setup.py +++ b/setup.py @@ -29,6 +29,6 @@ "console_scripts": ["check50=check50.__main__:main"] }, url="https://github.com/cs50/check50", - version="3.1.2", + version="3.2.0", include_package_data=True )