From 35103846ee45850028aa6ba0a3796497cdc77e84 Mon Sep 17 00:00:00 2001 From: Chad Sharp Date: Mon, 31 Jul 2017 10:20:19 -0400 Subject: [PATCH 01/21] Fix nargs --- check50.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/check50.py b/check50.py index a3ddc3b7..1231b177 100755 --- a/check50.py +++ b/check50.py @@ -54,7 +54,7 @@ def main(): help="run checks completely offline (implies --local)") parser.add_argument("--checkdir", action="store", - nargs="?", + nargs=1, default="~/.local/share/check50", help="specify directory containing the checks " "(~/.local/share/check50 by default)") From b71c7ef8e2c1832aa9da34cb777001ada1c4782d Mon Sep 17 00:00:00 2001 From: Brian Yu Date: Mon, 31 Jul 2017 15:13:58 -0400 Subject: [PATCH 02/21] Shorten output URL --- check50.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/check50.py b/check50.py index 1231b177..18eb1dc3 100755 --- a/check50.py +++ b/check50.py @@ -94,7 +94,7 @@ def main(): # Terminate if no response. if pings > 45: cprint("check50 is taking longer than normal!", "red", file=sys.stderr) - cprint("more info at: https://cs50.me/check50/results/{}/{}".format(username, commit_hash), "red", file=sys.stderr) + cprint("more info at: https://cs50.me/checks/{}".format(commit_hash), "red", file=sys.stderr) sys.exit(1) pings += 1 @@ -112,7 +112,7 @@ def main(): # Print results from payload. print_results(payload["checks"], config.args.log) - print("detailed results: https://cs50.me/check50/results/{}/{}".format(username, commit_hash)) + print("detailed results: https://cs50.me/checks/{}".format(commit_hash)) sys.exit(0) # copy all files to temporary directory From ac72d21b615b3edf25347f2625a47a38bbc92505 Mon Sep 17 00:00:00 2001 From: Brian Yu Date: Mon, 31 Jul 2017 16:45:29 -0400 Subject: [PATCH 03/21] Update link to detailed view --- check50.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/check50.py b/check50.py index 18eb1dc3..130569db 100755 --- a/check50.py +++ b/check50.py @@ -94,7 +94,7 @@ def main(): # Terminate if no response. if pings > 45: cprint("check50 is taking longer than normal!", "red", file=sys.stderr) - cprint("more info at: https://cs50.me/checks/{}".format(commit_hash), "red", file=sys.stderr) + cprint("See https://cs50.me/checks/{} for more detail.".format(commit_hash), "red", file=sys.stderr) sys.exit(1) pings += 1 @@ -112,7 +112,7 @@ def main(): # Print results from payload. print_results(payload["checks"], config.args.log) - print("detailed results: https://cs50.me/checks/{}".format(commit_hash)) + print("See https://cs50.me/checks/{} for more detail.".format(commit_hash)) sys.exit(0) # copy all files to temporary directory From 7f73e43bc3a38897f84ad810f469fa29921b96d0 Mon Sep 17 00:00:00 2001 From: Brian Yu Date: Mon, 31 Jul 2017 16:53:06 -0400 Subject: [PATCH 04/21] Bump check50 version --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 10a312e7..a874d37d 100644 --- a/setup.py +++ b/setup.py @@ -20,5 +20,5 @@ "console_scripts": ["check50=check50:main"] }, url="https://github.com/cs50/check50", - version="2.0.0" + version="2.0.1" ) From fd38467f56e3973f8b1733b593cb804718caa579 Mon Sep 17 00:00:00 2001 From: Brian Yu Date: Mon, 31 Jul 2017 17:10:32 -0400 Subject: [PATCH 05/21] Fix comments and spacing --- check50.py | 49 ++++++++++++++++++++++++------------------------- 1 file changed, 24 insertions(+), 25 deletions(-) diff --git a/check50.py b/check50.py index 130569db..6bec618f 100755 --- a/check50.py +++ b/check50.py @@ -39,7 +39,7 @@ def main(): - # parse command line arguments + # Parse command line arguments. parser = argparse.ArgumentParser() parser.add_argument("identifier", nargs=1) parser.add_argument("files", nargs="*") @@ -73,7 +73,6 @@ def main(): if config.args.offline: config.args.local = True - if not config.args.local: try: @@ -115,7 +114,7 @@ def main(): print("See https://cs50.me/checks/{} for more detail.".format(commit_hash)) sys.exit(0) - # copy all files to temporary directory + # Copy all files to temporary directory. config.tempdir = tempfile.mkdtemp() src_dir = os.path.join(config.tempdir, "_") os.mkdir(src_dir) @@ -126,7 +125,7 @@ def main(): checks = import_checks(identifier) - # create and run the test suite + # Create and run the test suite. suite = unittest.TestSuite() for case in config.test_cases: suite.addTest(checks(case)) @@ -134,10 +133,10 @@ def main(): suite.run(result) cleanup() - # Get list of results from TestResult class + # Get list of results from TestResult class. results = result.results - # print the results + # Print the results. if config.args.debug: print_json(results) else: @@ -175,7 +174,7 @@ def copy(src, dst): def excepthook(cls, exc, tb): cleanup() - # Class is a BaseException, better just quit + # Class is a BaseException, better just quit. if not issubclass(cls, Exception): print() return @@ -265,17 +264,17 @@ def import_checks(identifier): else: command = ["git", "clone", "https://github.com/{}/{}".format(org, repo), checks_root] - # Can't use subprocess.DEVNULL because it requires python 3.3 + # Can't use subprocess.DEVNULL because it requires python 3.3. stdout = stderr = None if config.args.verbose else open(os.devnull, "wb") - # Update checks via git + # Update checks via git. try: subprocess.check_call(command, stdout=stdout, stderr=stderr) except subprocess.CalledProcessError: raise InternalError("failed to clone checks") - # Install any dependencies from requirements.txt either in the root of the repository or in the directory of the specific check + # Install any dependencies from requirements.txt either in the root of the repository or in the directory of the specific check. for dir in [checks_root, os.path.dirname(config.check_dir)]: requirements = os.path.join(dir, "requirements.txt") if os.path.exists(requirements): @@ -296,9 +295,9 @@ def import_checks(identifier): raise InternalError("failed to install dependencies in ({})".format(requirements[len(config.args.checkdir)+1:])) try: - # Import module from file path directly + # Import module from file path directly. module = imp.load_source(slug, os.path.join(config.check_dir, "__init__.py")) - # Ensure that there is exactly one class decending from Checks defined in this package + # Ensure that there is exactly one class decending from Checks defined in this package. checks, = (cls for _, cls in inspect.getmembers(module, inspect.isclass) if hasattr(cls, "_Checks__sentinel") and cls.__module__.startswith(slug)) @@ -356,7 +355,7 @@ def valgrind(func): if config.test_cases[-1] == func.__name__: frame = traceback.extract_stack(limit=2)[0] raise InternalError("invalid check in {} on line {} of {}:\n" - "@valgrind must be placed below @check"\ + "@valgrind must be placed below @check" .format(frame.name, frame.lineno, frame.filename)) @wraps(func) def wrapper(self): @@ -372,7 +371,7 @@ def wrapper(self): return wrapper -# decorator for checks +# Decorator for checks def check(dependency=None): def decorator(func): @@ -381,19 +380,19 @@ def decorator(func): @wraps(func) def wrapper(self): - # check if dependency failed + # Check if dependency failed. if dependency and config.test_results.get(dependency) != Checks.PASS: self.result = config.test_results[func.__name__] = Checks.SKIP self.rationale = "can't check until a frown turns upside down" return - # move files into this check's directory + # Move files into this check's directory. self.dir = dst_dir = os.path.join(config.tempdir, self._testMethodName) src_dir = os.path.join(config.tempdir, dependency or "_") shutil.copytree(src_dir, dst_dir) os.chdir(self.dir) - # run the test, catch failures + # Run the test, catch failures. try: func(self) except Error as e: @@ -426,7 +425,7 @@ def _open(file, mode="r"): return open(file, mode, newline="\n") -# wrapper class for pexpect child +# Wrapper class for pexpect child class Child(object): def __init__(self, test, child): self.test = test @@ -456,7 +455,7 @@ def stdout(self, output=None, str_output=None, timeout=3): if output is None: return self.wait(timeout).output - # Files should be interpreted literally, anything else shouldn't be + # Files should be interpreted literally, anything else shouldn't be. try: output = output.read() except AttributeError: @@ -484,7 +483,7 @@ def stdout(self, output=None, str_output=None, timeout=3): except TIMEOUT: raise Error("timed out while waiting for {}".format(Mismatch.raw(str_output))) - # If we expected EOF and we still got output, report an error + # If we expected EOF and we still got output, report an error. if output == EOF and self.child.before: raise Error(Mismatch(EOF, self.child.before.replace("\r\n", "\n"))) @@ -528,7 +527,7 @@ def wait(self, timeout=3): else: raise Error("timed out while waiting for program to exit") - # Read any remaining data in pipe + # Read any remaining data in pipe. while True: try: bytes = self.child.read_nonblocking(size=1024, timeout=0) @@ -554,7 +553,7 @@ class Checks(unittest.TestCase): _valgrind_log = "valgrind.xml" _valgrind = False - # Here so we can properly check subclasses even when child is imported from another module + # Here so we can properly check subclasses even when child is imported from another module. __sentinel = None def tearDown(self): @@ -652,7 +651,7 @@ def _check_valgrind(self): self.log.append("checking for valgrind errors... ") - # Ensure that we don't get duplicate error messages + # Ensure that we don't get duplicate error messages. reported = set() for error in xml.iterfind("error"): # Type of error valgrind encountered @@ -664,7 +663,7 @@ def _check_valgrind(self): # Error message that we will report msg = ["\t", what] - # Find first stack frame within student's code + # Find first stack frame within student's code. for frame in error.iterfind("stack/frame"): obj = frame.find("obj") if obj is not None and os.path.dirname(obj.text) == self.dir: @@ -678,7 +677,7 @@ def _check_valgrind(self): self.log.append(msg) reported.add(msg) - # Only raise exception if we encountered errors + # Only raise exception if we encountered errors. if reported: raise Error("valgrind tests failed; rerun with --log for more information.") From eb2a9ee5cce067894601574c98ca75b66deefa17 Mon Sep 17 00:00:00 2001 From: Brian Yu Date: Mon, 31 Jul 2017 18:39:05 -0400 Subject: [PATCH 06/21] Catch UnicodeDecodeErrors --- check50.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/check50.py b/check50.py index 6bec618f..d32c9b76 100755 --- a/check50.py +++ b/check50.py @@ -482,6 +482,8 @@ def stdout(self, output=None, str_output=None, timeout=3): raise Error(Mismatch(str_output, result.replace("\r\n", "\n"))) except TIMEOUT: raise Error("timed out while waiting for {}".format(Mismatch.raw(str_output))) + except UnicodeDecodeError: + raise Error("output not valid ASCII text") # If we expected EOF and we still got output, report an error. if output == EOF and self.child.before: From 5e3e7c0bd5b4772c5e45eed5006fc778b00e5a18 Mon Sep 17 00:00:00 2001 From: Brian Yu Date: Mon, 31 Jul 2017 18:40:39 -0400 Subject: [PATCH 07/21] Handle catching all stdout exceptions --- check50.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/check50.py b/check50.py index d32c9b76..f4a75a38 100755 --- a/check50.py +++ b/check50.py @@ -484,6 +484,8 @@ def stdout(self, output=None, str_output=None, timeout=3): raise Error("timed out while waiting for {}".format(Mismatch.raw(str_output))) except UnicodeDecodeError: raise Error("output not valid ASCII text") + except Exception: + raise Error("check50 could not verify output") # If we expected EOF and we still got output, report an error. if output == EOF and self.child.before: From f0e9fbf8cb961839385b437f27ac9fb21b7112e4 Mon Sep 17 00:00:00 2001 From: Brian Yu Date: Mon, 31 Jul 2017 20:58:35 -0400 Subject: [PATCH 08/21] Add newline before error message --- check50.py | 1 + 1 file changed, 1 insertion(+) diff --git a/check50.py b/check50.py index f4a75a38..87735368 100755 --- a/check50.py +++ b/check50.py @@ -92,6 +92,7 @@ def main(): # Terminate if no response. if pings > 45: + print() cprint("check50 is taking longer than normal!", "red", file=sys.stderr) cprint("See https://cs50.me/checks/{} for more detail.".format(commit_hash), "red", file=sys.stderr) sys.exit(1) From 4afcee2ba40ec42dbd57f65c963bb430b199c825 Mon Sep 17 00:00:00 2001 From: Brian Yu Date: Tue, 1 Aug 2017 00:41:59 -0400 Subject: [PATCH 09/21] Remove readme --- README.md | 179 +----------------------------------------------------- 1 file changed, 1 insertion(+), 178 deletions(-) diff --git a/README.md b/README.md index b49d0b14..4f71ddc4 100644 --- a/README.md +++ b/README.md @@ -1,180 +1,3 @@ # check50 -## Running `check50` - -Run `check50` via: - -``` -python check50.py identifier [files to check] -``` - -For example, to check `greedy`, assuming you have a file called `greedy.c` in the same directory as `check50.py`, run: - -``` -python check50.py 2017/x/greedy greedy.c -``` - -Identifier names are given by the filenames in the `checks` directory, with subdirectories specified by the `/` symbol. -To check multiple files, separate each filename with a space. Alternatively, not specifying any files will by default use all files -in the current directory (TODO: change this to use the `exclude` files). - -## Contributing to `check50` - -To contribute to `check50`: - -* Fork the `cs50/check50` repository (button in the upper-right corner of the window) -* Clone the forked repository, committing and pushing new changes. -* Submit a pull request to `cs50/check50` from the forked repository. - -### Check File Structure - -All checks are located in a subdirectory of the `checks` directory. -Each problem which requires checks has its own `checks.py` file. -`checks.py` should contain a single class, named after the problem, which is a sublcass of `check50.Checks`. -Individual checks are methods defined within the class. -Check functions must be decorated with the `@check` decorator, and must have a one-line docstring which -describes what the method is checking. The `@check` decorator may optionally take a single parameter: -the name of another check method which must have passed before the decorated check can run. Before a -check runs, all files from the dependency check's working directory are copied into the decorated check's -working directory. - -### Check Successes and Failures - -To indicate that a check has failed, the exception `check50.Error` must be raised. -This can be raised directly by the check method, or indirectly via a method of `check50.Checks` -which is called by the check method. - -`check50.Error` takes a single parameter. If the parameter is a string, it represents the -reason for the check failure. If the parameter is a tuple of `(observed, desired)`, then it -represents that `observed` was the observed output, whereas `desired` was the desired output. - -If a check method finishes without raising `check50.Error`, then the test is assumed to have passed. - -### `rationale`, `helpers`, and `log` - -In addition to keeping track of the success and failure of tests, -`check50` maintains three other per-test-case values that store information -about what took place during a given check. - -`self.rationale` is a string, representing the reason that the check failed, and is set by `check50.Error`. - -`self.helpers` is a list of strings, representing zero or more lines of suggestions as to why a check -may have failed. Check functions may optionally choose to add to `self.helpers` if they can guess why -a particular `check50.Error` was raised. - -`self.log` is a list of strings, representing what steps took place during the check function. -Check functions may optionally add to `self.log`, and methods in `check50.Checks` will also -add to `self.log`. - -### Sample Check and Explanation - -``` -@check("compiles") -def test5(self): - """5 minutes equals 60 bottles.""" - self.spawn("./water").stdin("5").stdout("^.*60.*$", 60).exit(0) -``` - -The `@check` decorator indicates that this is a check method. The parameter `"compiles"` -means that unless the `compiles` check passed, this check (`test5`) will not run. It also -means that before this test is run, the state of the working directory after the `"compiles"` -check ran will be copied into `test5`'s working directory (including the compiled `water` -binary). - -The docstring `"5 minutes equal 60 bottles."` is a description of what this check -method is testing. - -First, `self.spawn("./water")` is called, which executes `./water` as a child process. -Then, the string `"5"` is passed into the program via the `.stdin()` method. - -The `.stdout()` method takes two parameters: the first is a regular expression indicating -the desired output of the program. The second parameter is a human-friendly representation -of the desired output, which will be displayed to students if the check fails. Here, -we assert that the output of `./water` should match the regular expression `"^.*60.*$"`. -If this is not the case, a `check50.Error` will be raised. - -Finally, we assert that the exit status of the program is `0`. - -### `check50.Checks` - -#### `checkfile(self, filename)` - -Takes a relative path to a file located in the directory containing `check.py` -and returns its contents as a string. - -#### `diff(self, f1, f2)` - -Takes two files, `f1` and `f2`, which can be strings or `check50.File` types. -If the filemaes are strings, they are interpreted as relative paths in the -temporary check directory. If the filenames are of type `check50.File`, -then type are considered relative paths in the directory containing -`check.py`. - -Compares the two files, and outputs `0` if the two files are the same, -and nonzero otherwise. - -#### `exists(self, filename)` - -Asserts that `filename` exists, and raises a `check50.Error` otherwise. - -#### `hash(self, filename)` - -Hashes `filename` using SHA-256, and returns the hash of the file. - -#### `spawn(self, cmd, env)` - -Spawns the child process running `cmd` with the environment `env`. -Returns a `check50.Child`, which is a wrapper on a `pexpect` child process. - -#### `include(self, path)` - -Includes `path`, a relative path in the directory containing `check.py` -into the temporary check directory. - -#### `append_code(self, filename, codefile)` - -Appends the contents of `codefile` to the end of `filename` in the -temporary directory. Useful for replacing a Python function during -testing. - -#### `fail(self, rationale)` - -Fails the test with the given `rationale`. - -### `check50.Child` - -#### `stdin(self, line, prompt=True)` - -Passes `line` as input to the child process. -The `prompt` variable, default `True`, determines whether -`check50` should wait for a nonzero-character prompt to appear -before passing in `line`. - -#### `stdout(self, output=None, str_output=None)` - -Asserts that, at program's termination, the -child process produces output that matches the regex `output`, -and raises a `check50.Error` otherwise. If the error is raised, -the `rationale` includes that the expected output was `str_output`. - -If `output` is `None` (i.e. by default, if no parameters are passed -into the `stdout` function), then the function returns a string -representing the program's output. - -#### `reject(self)` - -Asserts that the child process should have rejected an input -and prompted for another input. Typically called after a call to `.stdin()`. -If the child process did not prompt for another input, then a -`check50.Error` will be raised. - -#### `exit(self, code=None)` - -If `code` is not `None`, then `exit` asserts that the program terminated -with exit code `code`, and raises a `check50.Error` otherwise. - -If `code` is `None`, then `exit` returns the exit code of the program. - -#### `kill(self)` - -Terminates the child process. +Coming soon! From ed4a03048850c9494265adafde9bc4ea92ba8e96 Mon Sep 17 00:00:00 2001 From: Brian Yu Date: Tue, 1 Aug 2017 10:19:07 -0400 Subject: [PATCH 10/21] Catch EOF while waiting for stdin --- check50.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/check50.py b/check50.py index 87735368..69a66897 100755 --- a/check50.py +++ b/check50.py @@ -443,7 +443,7 @@ def stdin(self, line, prompt=True, timeout=3): if prompt: try: self.child.expect(".+", timeout=timeout) - except TIMEOUT: + except (TIMEOUT, EOF): raise Error("expected prompt for input, found none") if line == EOF: From 0bb10ffc85a7aa049ebb67c515f2fde171c8f052 Mon Sep 17 00:00:00 2001 From: Brian Yu Date: Tue, 1 Aug 2017 10:30:22 -0400 Subject: [PATCH 11/21] Skip check on check50 error --- check50.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/check50.py b/check50.py index 69a66897..9c05e870 100755 --- a/check50.py +++ b/check50.py @@ -338,18 +338,17 @@ def addSuccess(self, test): }) def addError(self, test, err): + test.log.append(err[1]) + test.log += traceback.format_tb(err[2]) + test.log.append("Contact sysadmins@cs50.harvard.edu with the URL of this check!") self.results.append({ "description": test.shortDescription(), "helpers": test.helpers, "log": test.log, - "rationale": err[1], - "status": Checks.FAIL, + "rationale": "check50 ran into an error while running checks!", + "status": Checks.SKIP, "test": test }) - cprint("check50 ran into an error while running checks.", "red", file=sys.stderr) - print(err[1], file=sys.stderr) - traceback.print_tb(err[2]) - sys.exit(1) def valgrind(func): From dcf5ad66d40def2a12250fcbe2a0839844b56c04 Mon Sep 17 00:00:00 2001 From: Brian Yu Date: Tue, 1 Aug 2017 12:35:29 -0400 Subject: [PATCH 12/21] Fix hanging git pull --- check50.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/check50.py b/check50.py index 9c05e870..8e5e7289 100755 --- a/check50.py +++ b/check50.py @@ -269,10 +269,15 @@ def import_checks(identifier): stdout = stderr = None if config.args.verbose else open(os.devnull, "wb") # Update checks via git. + spawn = pexpect.spawn if sys.version_info < (3, 0) else pexpect.spawnu try: - subprocess.check_call(command, stdout=stdout, stderr=stderr) - except subprocess.CalledProcessError: - raise InternalError("failed to clone checks") + child = spawn(" ".join(command), timeout=5) + child.expect(pexpect.EOF) + except pexpect.TIMEOUT: + raise InternalError("timed out while trying to clone checks") + + if config.args.verbose: + print(child.before + child.buffer) # Install any dependencies from requirements.txt either in the root of the repository or in the directory of the specific check. From 7fa57bb825cd496763aee20a6c023a6178d85f53 Mon Sep 17 00:00:00 2001 From: Brian Yu Date: Tue, 1 Aug 2017 12:36:24 -0400 Subject: [PATCH 13/21] Remove stdout and stderr redirection --- check50.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/check50.py b/check50.py index 8e5e7289..0ad45590 100755 --- a/check50.py +++ b/check50.py @@ -265,9 +265,6 @@ def import_checks(identifier): else: command = ["git", "clone", "https://github.com/{}/{}".format(org, repo), checks_root] - # Can't use subprocess.DEVNULL because it requires python 3.3. - stdout = stderr = None if config.args.verbose else open(os.devnull, "wb") - # Update checks via git. spawn = pexpect.spawn if sys.version_info < (3, 0) else pexpect.spawnu try: From 7d389b906a5933bd03b021afa686e24bbc428c6b Mon Sep 17 00:00:00 2001 From: Brian Yu Date: Tue, 1 Aug 2017 19:56:47 -0400 Subject: [PATCH 14/21] Change timeout to 1s --- check50.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/check50.py b/check50.py index 0ad45590..d4920ca3 100755 --- a/check50.py +++ b/check50.py @@ -435,7 +435,7 @@ def __init__(self, test, child): self.output = [] self.exitstatus = None - def stdin(self, line, prompt=True, timeout=3): + def stdin(self, line, prompt=True, timeout=1): if line == EOF: self.test.log.append("sending EOF...") else: @@ -453,7 +453,7 @@ def stdin(self, line, prompt=True, timeout=3): self.child.sendline(line) return self - def stdout(self, output=None, str_output=None, timeout=3): + def stdout(self, output=None, str_output=None, timeout=1): if output is None: return self.wait(timeout).output @@ -495,7 +495,7 @@ def stdout(self, output=None, str_output=None, timeout=3): return self - def reject(self, timeout=3): + def reject(self, timeout=1): self.test.log.append("checking that input was rejected...") try: self.child.expect(".+", timeout=timeout) @@ -506,7 +506,7 @@ def reject(self, timeout=3): raise Error("timed out while waiting for input to be rejected") return self - def exit(self, code=None, timeout=3): + def exit(self, code=None, timeout=1): self.wait(timeout) if code is None: @@ -517,7 +517,7 @@ def exit(self, code=None, timeout=3): raise Error("expected exit code {}, not {}".format(code, self.exitstatus)) return self - def wait(self, timeout=3): + def wait(self, timeout=1): end = time.time() + timeout while time.time() <= end: if not self.child.isalive(): From 9b2b0d01ed852ff584104fb0fc67ff5edb20e580 Mon Sep 17 00:00:00 2001 From: Brian Yu Date: Tue, 1 Aug 2017 20:09:56 -0400 Subject: [PATCH 15/21] Revert to subprocess for pulling --- check50.py | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/check50.py b/check50.py index d4920ca3..68b6e69e 100755 --- a/check50.py +++ b/check50.py @@ -265,17 +265,14 @@ def import_checks(identifier): else: command = ["git", "clone", "https://github.com/{}/{}".format(org, repo), checks_root] + # Can't use subprocess.DEVNULL because it requires python 3.3. + stdout = stderr = None if config.args.verbose else open(os.devnull, "wb") + # Update checks via git. - spawn = pexpect.spawn if sys.version_info < (3, 0) else pexpect.spawnu try: - child = spawn(" ".join(command), timeout=5) - child.expect(pexpect.EOF) - except pexpect.TIMEOUT: - raise InternalError("timed out while trying to clone checks") - - if config.args.verbose: - print(child.before + child.buffer) - + subprocess.check_call(command, stdout=stdout, stderr=stderr) + except subprocess.CalledProcessError: + raise InternalError("failed to clone checks") # Install any dependencies from requirements.txt either in the root of the repository or in the directory of the specific check. for dir in [checks_root, os.path.dirname(config.check_dir)]: From f99a24777b6abdf3c0e148124570c56fd85091bc Mon Sep 17 00:00:00 2001 From: Chad Sharp Date: Wed, 2 Aug 2017 15:34:31 -0400 Subject: [PATCH 16/21] Make append_code API consistent --- check50.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/check50.py b/check50.py index 68b6e69e..53950e05 100755 --- a/check50.py +++ b/check50.py @@ -637,11 +637,16 @@ def add(self, *paths): for path in paths: copy(path, cwd) - def append_code(self, filename, codefile): - with open(codefile.filename, "r") as code, \ - open(os.path.join(self.dir, filename), "a") as f: - f.write("\n") - f.write(code.read()) + def append_code(self, original, codefile): + if isinstance(original, File): + original = original.filename + + if isinstance(codefile, File): + codefile = codefile.filename + + with open(codefile) as code, open(original, "a") as o: + o.write("\n") + o.write(code.read()) def replace_fn(self, old_fn, new_fn, filename): self.spawn("sed -i='' -e 's/callq\t_{}/callq\t_{}/g' {}".format(old_fn, new_fn, filename)) From 07a42fcdacea4191c892975f978b4972b153faa7 Mon Sep 17 00:00:00 2001 From: Brian Yu Date: Fri, 4 Aug 2017 00:02:47 -0400 Subject: [PATCH 17/21] Clarify stdout timeout error message --- check50.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/check50.py b/check50.py index 53950e05..ba57dd2a 100755 --- a/check50.py +++ b/check50.py @@ -480,7 +480,7 @@ def stdout(self, output=None, str_output=None, timeout=1): result += self.child.after raise Error(Mismatch(str_output, result.replace("\r\n", "\n"))) except TIMEOUT: - raise Error("timed out while waiting for {}".format(Mismatch.raw(str_output))) + raise Error("did not find output {}".format(Mismatch.raw(str_output))) except UnicodeDecodeError: raise Error("output not valid ASCII text") except Exception: From 6be7e1b66c344d749a3336befa4857e9533a2d94 Mon Sep 17 00:00:00 2001 From: Brian Yu Date: Fri, 4 Aug 2017 10:09:53 -0400 Subject: [PATCH 18/21] Bump version number --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index a874d37d..7a67eba0 100644 --- a/setup.py +++ b/setup.py @@ -20,5 +20,5 @@ "console_scripts": ["check50=check50:main"] }, url="https://github.com/cs50/check50", - version="2.0.1" + version="2.0.2" ) From 2aa05a613321c597183826d482e4e9f8ed1ef947 Mon Sep 17 00:00:00 2001 From: Brian Yu Date: Fri, 4 Aug 2017 12:21:25 -0400 Subject: [PATCH 19/21] Accept any response back from CS50.me --- check50.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/check50.py b/check50.py index ba57dd2a..0cfef494 100755 --- a/check50.py +++ b/check50.py @@ -103,7 +103,7 @@ def main(): if res.status_code != 200: continue payload = res.json() - if payload["complete"] and payload["checks"] != []: + if payload["complete"]: break print(".", end="") sys.stdout.flush() From 68f3e57fa91c05b37f5fb5dcfe9c2b31aae83526 Mon Sep 17 00:00:00 2001 From: Brian Yu Date: Fri, 4 Aug 2017 12:28:30 -0400 Subject: [PATCH 20/21] Fix bug where --log throws an exception nonlocally --- check50.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/check50.py b/check50.py index 0cfef494..bd93a45f 100755 --- a/check50.py +++ b/check50.py @@ -207,7 +207,7 @@ def print_results(results, log=False): cprint(" {}".format(result.get("rationale") or "check skipped"), "yellow") if log: - for line in result["test"].log: + for line in result.get("log", []): print(" {}".format(line)) From c18cfc2e33927815a39b88c099fe5267c6244643 Mon Sep 17 00:00:00 2001 From: Chad Sharp Date: Mon, 7 Aug 2017 16:27:11 -0400 Subject: [PATCH 21/21] Fix style per style50 --- check50.py | 52 +++++++++++++++++++++++++++++++++------------------- 1 file changed, 33 insertions(+), 19 deletions(-) mode change 100755 => 100644 check50.py diff --git a/check50.py b/check50.py old mode 100755 new mode 100644 index bd93a45f..0299db06 --- a/check50.py +++ b/check50.py @@ -79,7 +79,8 @@ def main(): # Submit to check50 repo. import submit50 except ImportError: - raise InternalError("submit50 is not installed. Install submit50 and run check50 again.") + raise InternalError( + "submit50 is not installed. Install submit50 and run check50 again.") else: submit50.run.verbose = config.args.verbose username, commit_hash = submit50.submit("check50", identifier) @@ -94,12 +95,16 @@ def main(): if pings > 45: print() cprint("check50 is taking longer than normal!", "red", file=sys.stderr) - cprint("See https://cs50.me/checks/{} for more detail.".format(commit_hash), "red", file=sys.stderr) + cprint( + "See https://cs50.me/checks/{} for more detail.".format(commit_hash), + "red", + file=sys.stderr) sys.exit(1) pings += 1 # Query for check results. - res = requests.post("https://cs50.me/check50/status/{}/{}".format(username, commit_hash)) + res = requests.post( + "https://cs50.me/check50/status/{}/{}".format(username, commit_hash)) if res.status_code != 200: continue payload = res.json() @@ -252,8 +257,8 @@ def import_checks(identifier): try: org, repo = repo.split("/") except ValueError: - raise InternalError("expected repository to be of the form username/repository, but got \"{}\"".format(repo)) - + raise InternalError( + "expected repository to be of the form username/repository, but got \"{}\"".format(repo)) checks_root = os.path.join(config.args.checkdir, org, repo) config.check_dir = os.path.join(checks_root, slug.replace("/", os.sep), "check50") @@ -274,7 +279,8 @@ def import_checks(identifier): except subprocess.CalledProcessError: raise InternalError("failed to clone checks") - # Install any dependencies from requirements.txt either in the root of the repository or in the directory of the specific check. + # Install any dependencies from requirements.txt either in the root of the + # repository or in the directory of the specific check. for dir in [checks_root, os.path.dirname(config.check_dir)]: requirements = os.path.join(dir, "requirements.txt") if os.path.exists(requirements): @@ -292,15 +298,16 @@ def import_checks(identifier): code = e.code if code: - raise InternalError("failed to install dependencies in ({})".format(requirements[len(config.args.checkdir)+1:])) + raise InternalError("failed to install dependencies in ({})".format( + requirements[len(config.args.checkdir) + 1:])) try: # Import module from file path directly. module = imp.load_source(slug, os.path.join(config.check_dir, "__init__.py")) # Ensure that there is exactly one class decending from Checks defined in this package. checks, = (cls for _, cls in inspect.getmembers(module, inspect.isclass) - if hasattr(cls, "_Checks__sentinel") - and cls.__module__.startswith(slug)) + if hasattr(cls, "_Checks__sentinel") + and cls.__module__.startswith(slug)) except (OSError, IOError) as e: if e.errno != errno.ENOENT: raise @@ -356,6 +363,7 @@ def valgrind(func): raise InternalError("invalid check in {} on line {} of {}:\n" "@valgrind must be placed below @check" .format(frame.name, frame.lineno, frame.filename)) + @wraps(func) def wrapper(self): if not which("valgrind"): @@ -376,6 +384,7 @@ def decorator(func): # add test to list of test, in order of declaration config.test_cases.append(func.__name__) + @wraps(func) def wrapper(self): @@ -409,6 +418,7 @@ def wrapper(self): class File(object): """Generic class to represent file in check directory.""" + def __init__(self, filename): self.filename = filename @@ -469,7 +479,6 @@ def stdout(self, output=None, str_output=None, timeout=1): str_output = output output = output.replace("\n", "\r\n") - self.test.log.append("checking for output \"{}\"...".format(str_output)) try: @@ -548,6 +557,7 @@ def kill(self): self.child.close(force=True) return self + class Checks(unittest.TestCase): PASS = True FAIL = False @@ -573,9 +583,9 @@ def __init__(self, method_name): def diff(self, f1, f2): """Returns boolean indicating whether or not the files are different""" - if type(f1) == File: + if isinstance(f1, File): f1 = f1.filename - if type(f2) == File: + if isinstance(f2, File): f2 = f2.filename return bool(self.spawn("diff {} {}".format(quote(f1), quote(f2))) .wait() @@ -592,7 +602,7 @@ def hash(self, filename): """Hashes a file using SHA-256.""" # Assert that file exists. - if type(filename) == File: + if isinstance(filename, File): filename = filename.filename self.require(filename) @@ -610,8 +620,8 @@ def spawn(self, cmd, env=None): """Spawns a new child process.""" if self._valgrind: self.log.append("running valgrind {}...".format(cmd)) - cmd = "valgrind --show-leak-kinds=all --xml=yes --xml-file={} -- {}" \ - .format(os.path.join(self.dir, self._valgrind_log), cmd) + cmd = "valgrind --show-leak-kinds=all --xml=yes --xml-file={} -- {}".format( + os.path.join(self.dir, self._valgrind_log), cmd) else: self.log.append("running {}...".format(cmd)) @@ -677,7 +687,9 @@ def _check_valgrind(self): if obj is not None and os.path.dirname(obj.text) == self.dir: location = frame.find("file"), frame.find("line") if None not in location: - msg.append(": (file: {}, line: {})".format(location[0].text, location[1].text)) + msg.append( + ": (file: {}, line: {})".format( + location[0].text, location[1].text)) break msg = "".join(msg) @@ -692,13 +704,14 @@ def _check_valgrind(self): class Mismatch(object): """Class which represents that expected output did not match actual output.""" + def __init__(self, expected, actual): self.expected = expected self.actual = actual def __str__(self): return "expected {}, not {}".format(self.raw(self.expected), - self.raw(self.actual)) + self.raw(self.actual)) def __repr__(self): return "Mismatch(expected={}, actual={})".format(repr(expected), repr(actual)) @@ -707,7 +720,7 @@ def __repr__(self): def raw(s): """Get raw representation of s, truncating if too long""" - if type(s) == list: + if isinstance(s, list): s = "\n".join(s) if s == EOF: @@ -720,9 +733,9 @@ def raw(s): return "\"{}\"".format(s) - class Error(Exception): """Class to wrap errors in students' checks.""" + def __init__(self, rationale=None, helpers=None, result=Checks.FAIL): self.rationale = rationale self.helpers = helpers @@ -731,6 +744,7 @@ def __init__(self, rationale=None, helpers=None, result=Checks.FAIL): class InternalError(Exception): """Error during execution of check50.""" + def __init__(self, msg): self.msg = msg