Skip to content

Commit

Permalink
Merge pull request #206 from cs50/develop
Browse files Browse the repository at this point in the history 
V3.1.0: verbosity levels, logging, --no-install-dependencies
  • Loading branch information
Kareem Zidane authored May 8, 2020
2 parents b01fc00 + 7b167c6 commit 42ab12e
Show file tree
Hide file tree
Showing 7 changed files with 206 additions and 119 deletions.
104 changes: 72 additions & 32 deletions check50/__main__.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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):
"""
Expand All @@ -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"], {
Expand Down Expand Up @@ -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"))
Expand All @@ -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__}")
Expand All @@ -278,87 +310,95 @@ 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()

# 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],
Expand Down
27 changes: 26 additions & 1 deletion check50/_api.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions check50/internal.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
50 changes: 44 additions & 6 deletions check50/renderer/templates/results.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ <h3 style="color:orange">:| {{ check.description }}</h3>
<div class="well well-sm">
{# Reason why the check received its status #}
{% if check.cause %}
<p>
<b>Cause</b><br/>
{% autoescape false %}
{{ check.cause.rationale | e | replace(" ", "&nbsp;") }}
{% endautoescape %}
Expand All @@ -34,16 +36,20 @@ <h3 style="color:orange">:| {{ check.description }}</h3>
<br/>
{% endif %}
{% endautoescape %}
</p>
{% endif %}

{# Log information #}
{% if check.log %}
<b>Log</b>
<br/>
{% for item in check.log %}
{{ item }}
<br/>
{% endfor %}
<samp>
<b>Log</b><br/>
<div style="border:2px solid; padding:2px;">
{% for item in check.log %}
{{ item }}
<br/>
{% endfor %}
</div>
</samp>
{% endif %}

{% if check.cause and check.cause.error %}
Expand Down Expand Up @@ -104,6 +110,38 @@ <h3 style="color:orange">:| {{ check.description }}</h3>
</div>
{% endif %}

{# Missing if there was one #}
{% if check.cause and "missing_item" in check.cause and "collection" in check.cause %}
<br/>

<div class="row">
<div class="col-md-6">
<b>Could not find the following in the output:</b>
<br/>
<div style="background-color:black; color:#90ee90;">
<samp>
{% autoescape false %}
{% set item = check.cause.missing_item | e %}
{{ item | replace(" ", "&nbsp;") | replace("\n", "<br/>") }}
{% endautoescape %}
</samp>
</div>
</div>
<div class="col-md-6">
<b>Actual Output:</b>
<br/>
<div style="background-color:black; color:white;">
<samp>
{% autoescape false %}
{% set collection = check.cause.collection | e %}
{{ collection | replace(" ", "&nbsp;") | replace("\n", "<br/>") }}
{% endautoescape %}
</samp>
</div>
</div>
</div>
{% endif %}

</div>
{% endif %}
{% endfor %}
Expand Down
Loading

0 comments on commit 42ab12e

Please sign in to comment.