Merge pull request #396 from datacamp/feat/update_has_expr_feedback

[LO-1231] Feat/update_has_expr_feedback

Author: JamesDanielOReilly

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 All notable changes to the pythonwhat project will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
+## 2.23.2
+
+- Update behaviour of has_expr() default feedback message. If the student's evaluation is too long, it is now shortened and an ellipsis is added. 
+
 ## 2.23.1
 
 - Fix string formatting in has_expr(). Strings will now have quotes in error messages. Leading and trailing whitespace is no longer removed.

pythonwhat/checks/has_funcs.py

@@ -349,12 +349,15 @@ def has_expr(
         if isinstance(eval_sol, str):
             fmt_kwargs["sol_eval"] = '\'{}\''.format(fmt_kwargs["sol_eval"])
 
+    # reformat student evaluation string if it is too long
+    fmt_kwargs["stu_eval"] = utils.shorten_string(fmt_kwargs["stu_eval"])
+
     # check if student or solution evaluations are too long or contain newlines
     if incorrect_msg == DEFAULT_INCORRECT_MSG and (
-        utils.unshowable_string(fmt_kwargs["stu_eval"])
-        or utils.unshowable_string(fmt_kwargs["sol_eval"])
-        or fmt_kwargs["stu_eval"] == fmt_kwargs["sol_eval"]
-    ):
+        len(fmt_kwargs["sol_eval"]) > 50 or
+        utils.has_newline(fmt_kwargs["stu_eval"]) or
+        utils.has_newline(fmt_kwargs["sol_eval"]) or
+        fmt_kwargs["stu_eval"] == fmt_kwargs["sol_eval"]):
         fmt_kwargs["stu_eval"] = None
         fmt_kwargs["sol_eval"] = None
         incorrect_msg = "Expected something different."

pythonwhat/utils.py

@@ -10,9 +10,13 @@ def include_v1():
 def v2_only():
     return not include_v1()
 
+def shorten_string(text):
+    if len(text) > 50:
+        text = text[0:45] + "..."
+    return text
 
-def unshowable_string(text):
-    return "\n" in text or len(text) > 50
+def has_newline(text):
+    return "\n" in text
 
 
 def copy_env(env):

tests/test_messaging.py

@@ -728,7 +728,7 @@ def test_has_equal_value_dont_wrap_newline():
     assert output["message"] == "Check your call of <code>print()</code>. Did you correctly specify the first argument? Expected something different."  # nopep8
 
 
-def test_has_equal_value_dont_wrap_too_long():
+def test_has_equal_value_shorten_submission():
     sol = """print('short text')"""
     stu = """print('This text is longer than 50 characters if I copy it 3 times. This text is longer than 50 characters if I copy it 3 times. This text is longer than 50 characters if I copy it 3 times.')"""  # nopep8
     sct = """Ex().check_function('print', index=0, signature=False).check_args(0).has_equal_value()"""
@@ -740,6 +740,21 @@ def test_has_equal_value_dont_wrap_too_long():
         }
     )
     assert not output["correct"]
+    assert output["message"] == "Check your call of <code>print()</code>. Did you correctly specify the first argument? Expected <code>'short text'</code>, but got <code>'This text is longer than 50 characters if I ...</code>."  # nopep8
+
+
+def test_has_equal_value_dont_shorten_solution():
+    sol = """print('This solution is really really really really really really really really long!')"""
+    stu = """print('short text')"""  # nopep8
+    sct = """Ex().check_function('print', index=0, signature=False).check_args(0).has_equal_value()"""
+    output = helper.run(
+        {
+            "DC_CODE": stu,
+            "DC_SOLUTION": sol,
+            "DC_SCT": sct,
+        }
+    )
+    assert not output["correct"]
     assert output["message"] == "Check your call of <code>print()</code>. Did you correctly specify the first argument? Expected something different."  # nopep8
 
 ## Check has no error ---------------------------------------------------------