diff --git a/protowhat/State.py b/protowhat/State.py index 55e2d9a..af36f58 100644 --- a/protowhat/State.py +++ b/protowhat/State.py @@ -59,6 +59,7 @@ def __init__( self.ast_dispatcher = self.get_dispatcher() # Parse solution and student code + # if possible, not done yet and wanted (ast arguments not False) if isinstance(self.solution_code, str) and self.solution_ast is None: self.solution_ast = self.parse(self.solution_code, test=False) if isinstance(self.student_code, str) and self.student_ast is None: diff --git a/protowhat/checks/check_files.py b/protowhat/checks/check_files.py index b2343ac..a7db01f 100644 --- a/protowhat/checks/check_files.py +++ b/protowhat/checks/check_files.py @@ -1,7 +1,5 @@ from pathlib import Path -from protowhat.Feedback import Feedback - def check_file( state, @@ -16,24 +14,24 @@ def check_file( Note: this SCT fails if the file is a directory. """ - p = Path(path) - if not p.exists(): + path_obj = Path(path) + if not path_obj.exists(): state.report(missing_msg.format(path)) # test file exists - if p.is_dir(): + if path_obj.is_dir(): state.report(is_dir_msg.format(path)) # test its not a dir - code = p.read_text() + code = get_file_content(path_obj) sol_kwargs = {"solution_code": solution_code, "solution_ast": None} if solution_code: sol_kwargs["solution_ast"] = ( - state.parse(solution_code, test=False) if parse else None + state.parse(solution_code, test=False) if parse else False ) return state.to_child( append_message="We checked the file `{}`. ".format(path), student_code=code, - student_ast=state.parse(code) if parse else None, + student_ast=state.parse(code) if parse else False, **sol_kwargs ) @@ -50,6 +48,18 @@ def has_dir(state, path, incorrect_msg="Did you create a directory `{}`?"): def load_file(relative_path, prefix=""): # the prefix can be partialed # so it's not needed to copy the common part of paths - p = Path(prefix, relative_path) + path = Path(prefix, relative_path) + + return get_file_content(path) + + +def get_file_content(path): + if not isinstance(path, Path): + path = Path(path) + + try: + content = path.read_text(encoding="utf-8") + except: + content = None - return p.read_text() + return content diff --git a/protowhat/sct_syntax.py b/protowhat/sct_syntax.py index 20f7218..0c97e4b 100644 --- a/protowhat/sct_syntax.py +++ b/protowhat/sct_syntax.py @@ -63,7 +63,7 @@ def __getattr__(self, attr): self._double_attr_error() else: # make a copy to return, - # in case someone does: a = chain.a; b = chain.b + # in case someone does: chain = chain_of_checks; chain.a; chain.b return self._sct_copy(attr_scts[attr]) def __call__(self, *args, **kwargs): diff --git a/tests/test_check_files.py b/tests/test_check_files.py index 7fc5907..7906772 100644 --- a/tests/test_check_files.py +++ b/tests/test_check_files.py @@ -1,12 +1,15 @@ import os +import ast import pytest + from functools import partial from tempfile import NamedTemporaryFile, TemporaryDirectory +from unittest.mock import patch, mock_open +from protowhat.Test import TestFail as TF from protowhat.selectors import Dispatcher from protowhat.State import State from protowhat.Reporter import Reporter -import ast from protowhat.sct_syntax import F, Ex from protowhat.checks import check_files as cf @@ -29,15 +32,28 @@ def assert_equal_ast(a, b): assert ast.dump(a) == ast.dump(b) -@pytest.fixture(scope="function") -def tf(): +@pytest.fixture +def temp_file_sum(): with NamedTemporaryFile() as tmp: tmp.file.write(b"1 + 1") tmp.file.flush() yield tmp -@pytest.fixture(scope="function") +@pytest.fixture +def temp_file_unicode(): + with NamedTemporaryFile() as tmp: + tmp.file.write("Hervé".encode("utf-8")) + tmp.file.flush() + yield tmp + + +@pytest.fixture(params=["temp_file_sum", "temp_file_unicode"]) +def temp_file(request): + return request.getfuncargvalue(request.param) + + +@pytest.fixture def state(): return State( # only Reporter and Dispatcher are used @@ -53,8 +69,30 @@ def state(): ) -def test_check_file(state, tf): - child = cf.check_file(state, tf.name, solution_code="3 + 3") +def test_get_file_content_simple(temp_file_sum): + content = cf.get_file_content(temp_file_sum.name) + assert content == "1 + 1" + + +def test_get_file_content_unicode(temp_file_unicode): + content = cf.get_file_content(temp_file_unicode.name) + assert content == "Hervé" + + +def test_get_file_content_missing(): + content = cf.get_file_content("foo") + assert content is None + + +def test_get_file_content_error(temp_file_sum): + with patch("io.open") as mock_file: + mock_file.side_effect = IOError() + content = cf.get_file_content(temp_file_sum.name) + assert content is None + + +def test_check_file(state, temp_file_sum): + child = cf.check_file(state, temp_file_sum.name, solution_code="3 + 3") assert child.student_code == "1 + 1" assert_equal_ast(child.student_ast, ast.parse(child.student_code)) assert child.solution_code == "3 + 3" @@ -62,39 +100,58 @@ def test_check_file(state, tf): assert check_node(child, "Expr", 0) -def test_check_file_no_parse(state, tf): - child = cf.check_file(state, tf.name, parse=False) +def test_check_file_no_parse(state, temp_file_sum): + child = cf.check_file(state, temp_file_sum.name, parse=False) assert child.student_code == "1 + 1" - assert child.student_ast is None - assert child.solution_ast is None + assert child.student_ast is False + assert child.solution_ast is None # no solution code is provided with pytest.raises(TypeError): assert check_node(child, "Expr", 0) -def test_check_no_sol(state, tf): - child = cf.check_file(state, tf.name) +def test_check_no_sol(state, temp_file): + child = cf.check_file(state, temp_file.name) assert child.solution_code is None +def test_check_file_missing(state): + with pytest.raises(TF) as exception: + cf.check_file(state, "foo") + assert "Did you create the file" in str(exception) + + +def test_check_file_dir(state): + with pytest.raises(TF) as exception: + with TemporaryDirectory() as td: + cf.check_file(state, td) + assert "found a directory" in str(exception) + + def test_check_dir(state): with TemporaryDirectory() as td: cf.has_dir(state, td) -def test_check_file_fchain(state, tf): +def test_missing_check_dir(state): + with pytest.raises(TF) as exception: + cf.has_dir(state, "foo") + assert "Did you create a directory" in str(exception) + + +def test_check_file_fchain(state, temp_file): f = F(attr_scts={"check_file": cf.check_file}) - Ex(state) >> f.check_file(tf.name) + Ex(state) >> f.check_file(temp_file.name) -def test_load_file(state, tf): - assert "1 + 1" == cf.load_file(tf.name) +def test_load_file(state, temp_file): + expected_content = cf.get_file_content(temp_file.name) + assert expected_content == cf.load_file(temp_file.name) - filename = os.path.basename(os.path.normpath(tf.name)) - common_path = os.path.dirname(tf.name) + "/" + filename = os.path.basename(os.path.normpath(temp_file.name)) + common_path = os.path.dirname(temp_file.name) load_file = partial(cf.load_file, prefix=common_path) - assert "1 + 1" == load_file(filename) - - assert "1 + 1" == cf.load_file(filename, prefix=common_path) + assert expected_content == load_file(filename) - assert "1 + 1" == cf.load_file(filename, prefix=os.path.dirname(tf.name)) + assert expected_content == cf.load_file(filename, prefix=common_path) + assert expected_content == cf.load_file(filename, prefix=common_path + "/")