Improve check_file: support unicode + disable ast parsing

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:

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

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):

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,48 +69,89 @@ 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"
     assert_equal_ast(child.solution_ast, ast.parse(child.solution_code))
     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 + "/")