diff --git a/check50/__main__.py b/check50/__main__.py
index 552394d5..0b321db1 100644
--- a/check50/__main__.py
+++ b/check50/__main__.py
@@ -159,6 +159,27 @@ def compile_checks(checks, prompt=False):
return "__init__.py"
+def setup_logging(level):
+ """
+ Sets up logging for 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
+ if not level:
+ return
+
+ lib50_logger = logging.getLogger("lib50")
+
+ # Set verbosity level on the lib50 logger
+ lib50_logger.setLevel(getattr(logging, level.upper()))
+
+ # Direct all logs to sys.stderr
+ lib50_logger.addHandler(logging.StreamHandler(sys.stderr))
+
+ # Don't animate the progressbar
+ lib50.ProgressBar.DISABLED = True
+
def await_results(commit_hash, slug, pings=45, sleep=2):
"""
@@ -183,14 +204,12 @@ def await_results(commit_hash, slug, pings=45, sleep=2):
_("check50 is taking longer than normal!\n"
"See https://submit.cs50.io/check50/{} for more detail").format(commit_hash))
-
if not results["check50"]:
raise RemoteCheckError(results)
if "error" in results["check50"]:
raise 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)
return results["tag_hash"], {
@@ -240,11 +259,11 @@ 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).\n"
+ help=_("run check50 in development mode (implies --offline and --verbose info).\n"
"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)"))
+ help=_("run checks completely offline (implies --local, --no-download-checks and --no-install-dependencies)"))
parser.add_argument("-l", "--local",
action="store_true",
help=_("run checks locally instead of uploading to cs50"))
@@ -266,8 +285,21 @@ def main():
metavar="FILE",
help=_("file to write output to"))
parser.add_argument("-v", "--verbose",
+ action="store",
+ nargs="?",
+ default="",
+ const="info",
+ choices=[attr for attr in dir(logging) if attr.isupper() and isinstance(getattr(logging, attr), int)],
+ type=str.upper,
+ help=_("sets the verbosity level."
+ ' "INFO" displays the full tracebacks of errors and shows all commands run.'
+ ' "DEBUG" adds the output of all command run.'))
+ parser.add_argument("--no-download-checks",
+ action="store_true",
+ help=_("do not download checks, but use previously downloaded checks instead (only works with --local)"))
+ parser.add_argument("--no-install-dependencies",
action="store_true",
- help=_("display the full tracebacks of any errors (also implies --log)"))
+ help=_("do not install dependencies (only works with --local)"))
parser.add_argument("-V", "--version",
action="version",
version=f"%(prog)s {__version__}")
@@ -278,58 +310,74 @@ def main():
global SLUG
SLUG = args.slug
-
+ # dev implies offline and verbose "info" if not overwritten
if args.dev:
args.offline = True
- args.verbose = True
+ if not args.verbose:
+ args.verbose = "info"
+ # offline implies local
if args.offline:
+ args.no_install_dependencies = True
+ args.no_download_checks = True
args.local = True
- if args.verbose:
- # Show lib50 commands being run in verbose mode
- logging.basicConfig(level=os.environ.get("CHECK50_LOGLEVEL", "INFO"))
- lib50.ProgressBar.DISABLED = True
- args.log = True
+ # Setup logging for lib50 depending on verbosity level
+ setup_logging(args.verbose)
+
+ # Warning in case of running remotely with no_download_checks or no_install_dependencies set
+ if 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.verbose = bool(args.verbose)
excepthook.outputs = args.output
excepthook.output_file = args.output_file
+ # 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]
with lib50.ProgressBar("Waiting for results") if "ansi" in args.output else nullcontext():
tag_hash, results = await_results(commit_hash, SLUG)
+ # Otherwise run checks locally
else:
- with lib50.ProgressBar("Checking") if not args.verbose and "ansi" in args.output else nullcontext():
+ 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()
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:
- # Otherwise have lib50 create a local copy of slug
try:
- internal.check_dir = lib50.local(SLUG, offline=args.offline)
+ internal.check_dir = lib50.local(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))
except lib50.InvalidSlugError:
- raise_invalid_slug(SLUG, offline=args.offline)
+ raise_invalid_slug(SLUG, offline=args.no_download_checks)
# Load config
config = internal.load_config(internal.check_dir)
+
# Compile local checks if necessary
if isinstance(config["checks"], dict):
config["checks"] = internal.compile_checks(config["checks"], prompt=args.dev)
install_translations(config["translations"])
- if not args.offline:
+ if not args.no_install_dependencies:
install_dependencies(config["dependencies"], verbose=args.verbose)
checks_file = (internal.check_dir / config["checks"]).resolve()
@@ -337,28 +385,20 @@ def main():
# Have lib50 decide which files to include
included = lib50.files(config.get("files"))[0]
- # Only open devnull conditionally
- ctxmanager = open(os.devnull, "w") if not args.verbose else nullcontext()
- with ctxmanager as devnull:
+ 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:
+ stdout = stderr = devnull
+ else:
stdout = sys.stdout
stderr = sys.stderr
- else:
- stdout = stderr = devnull
- # Create a working_area (temp dir) with all included student files named -
+ # 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):
- runner = CheckRunner(checks_file)
-
- # Run checks
- if args.target:
- check_results = runner.run(args.target, included, working_area)
- else:
- check_results = runner.run_all(included, working_area)
-
+ check_results = CheckRunner(checks_file).run(included, working_area, args.target)
results = {
"slug": SLUG,
"results": [attr.asdict(result) for result in check_results],
diff --git a/check50/_api.py b/check50/_api.py
index 8cceaf8b..abd2d69e 100644
--- a/check50/_api.py
+++ b/check50/_api.py
@@ -263,7 +263,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 Missing(str_output, self.process.before)
except UnicodeDecodeError:
raise Failure(_("output not valid ASCII text"))
except Exception:
@@ -379,6 +379,31 @@ def __str__(self):
return self.payload["rationale"]
+class Missing(Failure):
+ """
+ Exception signifying check failure due to an item missing from a collection.
+ This is typically a specific substring in a longer string, for instance the contents of stdout.
+
+ :param item: the expected item / substring
+ :param collection: the collection / string
+ :param help: optional help message to be displayed
+ :type help: str
+
+ Example usage::
+
+ actual = check50.run("./fibonacci 5").stdout()
+
+ if "5" not in actual and "3" in actual:
+ help = "Be sure to start the sequence at 1"
+ raise check50.Missing("5", actual, help=help)
+
+ """
+
+ def __init__(self, missing_item, collection, help=None):
+ super().__init__(rationale=_("Did not find {} in {}").format(_raw(missing_item), _raw(collection)), help=help)
+ self.payload.update({"missing_item": str(missing_item), "collection": str(collection)})
+
+
class Mismatch(Failure):
"""
Exception signifying check failure due to a mismatch in expected and actual outputs.
diff --git a/check50/internal.py b/check50/internal.py
index fcc722b4..b5996c45 100644
--- a/check50/internal.py
+++ b/check50/internal.py
@@ -178,6 +178,7 @@ def import_file(name, path):
spec.loader.exec_module(mod)
return mod
+
def _yes_no_prompt(prompt):
"""
Raise a prompt, returns True if yes is entered, False if no is entered.
diff --git a/check50/renderer/templates/results.html b/check50/renderer/templates/results.html
index 9d66ff39..683ef88f 100644
--- a/check50/renderer/templates/results.html
+++ b/check50/renderer/templates/results.html
@@ -24,6 +24,8 @@
:| {{ check.description }}
{# Reason why the check received its status #}
{% if check.cause %}
+
+ {% endif %}
+
{% endif %}
{% endfor %}
diff --git a/check50/runner.py b/check50/runner.py
index 176e5e98..d0d7f44d 100644
--- a/check50/runner.py
+++ b/check50/runner.py
@@ -164,7 +164,6 @@ def wrapper(checks_root, dependency_state):
class CheckRunner:
def __init__(self, checks_path):
-
# TODO: Naming the module "checks" is arbitray. Better name?
self.checks_spec = importlib.util.spec_from_file_location("checks", checks_path)
@@ -177,29 +176,41 @@ def __init__(self, checks_path):
self.check_names = _check_names.copy()
_check_names.clear()
- # Map each check to tuples containing the names and descriptions of the checks that depend on it
- self.dependency_map = collections.defaultdict(set)
- for name, check in inspect.getmembers(check_module, lambda f: hasattr(f, "_check_dependency")):
- dependency = check._check_dependency.__name__ if check._check_dependency is not None else None
- self.dependency_map[dependency].add((name, check.__doc__))
+ # 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_all(self, files, working_area):
+
+ def run(self, files, working_area, targets=None):
"""
Run checks concurrently.
Returns a list of CheckResults ordered by declaration order of the checks in the imported module
+ targets allows you to limit which checks run. If targets is false-y, all checks are run.
"""
+ graph = self.build_subgraph(targets) if targets else self.dependency_graph
# 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
- max_workers = os.environ.get("CHECK50_WORKERS")
- with futures.ProcessPoolExecutor(max_workers=max_workers if max_workers is None else int(max_workers)) as executor:
+ try:
+ max_workers = int(os.environ.get("CHECK50_WORKERS"))
+ except (ValueError, TypeError):
+ max_workers = 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))
- for name, _ in self.dependency_map[None])
+ for name in graph[None])
not_passed = []
while not_done:
@@ -210,7 +221,7 @@ def run_all(self, files, working_area):
results[result.name] = result
if result.passed:
# Dispatch dependent checks
- for child_name, _ in self.dependency_map[result.name]:
+ for child_name in graph[result.name]:
not_done.add(executor.submit(
run_check(child_name, self.checks_spec, checks_root, state)))
else:
@@ -219,74 +230,48 @@ def run_all(self, files, working_area):
for name in not_passed:
self._skip_children(name, results)
- return list(results.values())
+ # Don't include checks we don't have results for (i.e. in the case that targets != None) in the list.
+ return list(filter(None, results.values()))
- def run(self, check_names, files, working_area):
+ def build_subgraph(self, targets):
"""
- Run just the targeted checks, and the checks they depends on.
- Returns just the result of the targetted checks.
+ Build minimal subgraph of self.dependency_graph that contains each check in targets
"""
- if len(set(check_names)) < len(check_names):
- raise internal.Error(_("Duplicate checks targetted: {}".format(check_names)))
-
- # Find the docs for every check in the dependency map
- check_docs = {}
- for dependents in self.dependency_map.values():
- for dependent, doc in dependents:
- check_docs[dependent] = doc
-
- # For every targetted check, validate that it exists
- for check_name in check_names:
- if check_name not in check_docs:
- raise internal.Error(_("Unknown check {}").format(check_name))
-
- # Build an inverse dependency map, from a check to its dependency
- inverse_dependency_map = self._create_inverse_dependency_map()
-
- # Reconstruct a new dependency_map, consisting of the targetted checks and their dependencies
- new_dependency_map = collections.defaultdict(set)
- for check_name in check_names:
- cur_check_name = check_name
- while cur_check_name != None:
- dependency_name = inverse_dependency_map[cur_check_name]
- new_dependency_map[dependency_name].add((cur_check_name, check_docs[cur_check_name]))
- cur_check_name = dependency_name
-
- # Temporarily replace dependency_map and run
- try:
- old_dependency_map = self.dependency_map
- self.dependency_map = new_dependency_map
- results = self.run_all(files, working_area)
- finally:
- self.dependency_map = old_dependency_map
-
- # Filter out all occurances of None in results (results of non targetted checks)
- return [result for result in results if result != None]
-
-
- def dependencies_of(self, check_names):
- """Get the names of all direct and indirect dependencies of check_names."""
- # Build an inverse dependency map, from a check to its dependency
- inverse_dependency_map = self._create_inverse_dependency_map()
-
- # Gather all dependencies from inverse_dependency_map
- dependencies = set()
- for check_name in check_names:
- cur_check_name = inverse_dependency_map[check_name]
- while cur_check_name != None:
- dependencies.add(cur_check_name)
- cur_check_name = inverse_dependency_map[cur_check_name]
-
- return dependencies
-
- def _create_inverse_dependency_map(self):
+ checks = self.dependencies_of(targets)
+ subgraph = collections.defaultdict(set)
+ for dep, children in self.dependency_graph.items():
+ # If dep is not a dependency of any target,
+ # none of its children will be either, may as well skip.
+ if dep is not None and dep not in checks:
+ continue
+ for child in children:
+ if child in checks:
+ subgraph[dep].add(child)
+ return subgraph
+
+
+ def dependencies_of(self, targets):
+ """Get all unique dependencies of the targetted checks (tartgets)."""
+ inverse_graph = self._create_inverse_dependency_graph()
+ deps = set()
+ for target in targets:
+ if target not in inverse_graph:
+ raise internal.Error(_("Unknown check: {}").format(e.args[0]))
+ curr_check = target
+ while curr_check is not None and curr_check not in deps:
+ deps.add(curr_check)
+ curr_check = inverse_graph[curr_check]
+ return deps
+
+
+ def _create_inverse_dependency_graph(self):
"""Build an inverse dependency map, from a check to its dependency."""
- inverse_dependency_map = {}
- for check_name, dependents in self.dependency_map.items():
- for dependent_name, _ in dependents:
- inverse_dependency_map[dependent_name] = check_name
- return inverse_dependency_map
+ inverse_dependency_graph = {}
+ for check_name, dependents in self.dependency_graph.items():
+ for dependent_name in dependents:
+ inverse_dependency_graph[dependent_name] = check_name
+ return inverse_dependency_graph
def _skip_children(self, check_name, results):
@@ -294,9 +279,9 @@ def _skip_children(self, check_name, results):
Recursively skip the children of check_name (presumably because check_name
did not pass).
"""
- for name, description in self.dependency_map[check_name]:
+ for name in self.dependency_graph[check_name]:
if results[name] is None:
- results[name] = CheckResult(name=name, description=_(description),
+ results[name] = CheckResult(name=name, description=self.check_descriptions[name],
passed=None,
dependency=check_name,
cause={"rationale": _("can't check until a frown turns upside down")})
diff --git a/setup.py b/setup.py
index 954377e4..eb0bba08 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,<3", "pyyaml>=3.10", "requests>=2.19", "termcolor>=1.1", "jinja2>=2.10"],
+ install_requires=["attrs>=18", "bs4>=0", "pexpect>=4.6", "lib50>=2,<4", "pyyaml>=3.10", "requests>=2.19", "termcolor>=1.1", "jinja2>=2.10"],
extras_require = {
"develop": ["sphinx", "sphinx_rtd_theme"]
},
@@ -29,6 +29,6 @@
"console_scripts": ["check50=check50.__main__:main"]
},
url="https://github.com/cs50/check50",
- version="3.0.10",
+ version="3.1.0",
include_package_data=True
)
diff --git a/tests/api_tests.py b/tests/api_tests.py
index 465503a5..4a3ca803 100644
--- a/tests/api_tests.py
+++ b/tests/api_tests.py
@@ -149,14 +149,12 @@ def test_outs(self):
self.process.stdout("foo\n")
self.process.stdout("bar")
self.process.stdout("\n")
- self.assertTrue(self.process.process.isalive())
def test_out_regex(self):
self.write("print('foo')")
self.runpy()
self.process.stdout(".o.")
self.process.stdout("\n")
- self.assertTrue(self.process.process.isalive())
def test_out_no_regex(self):
self.write("print('foo')")