Merge branch 'mr/pmderodat/test-opt-check' into 'master'

Enhance user experience for syntax errors in "opt" files

See merge request it/e3-testsuite!34

.flake8

@@ -1,6 +1,6 @@
 [flake8]
 exclude = .git,__pycache__,build,dist,.tox
-ignore = A003, C901, E203, E266, E501, W503,D100,D101,D102,D102,D103,D104,D105,D106,D107,D203,D403,D213,B028,B906,B907,E704
+ignore = A003, C901, E203, E266, E501, W503,D100,D101,D102,D102,D103,D104,D105,D106,D107,D203,D403,D213,B028,B906,B907,C420,E704
 # line length is intentionally set to 80 here because black uses Bugbear
 # See https://github.com/psf/black/blob/master/README.md#line-length for more details
 max-line-length = 80

NEWS.md

@@ -1,6 +1,9 @@
 27.0 (Not released yet)
 =======================
 
+* `e3-opt-check`: new script to look for syntax errors in opt files.
+* `e3-opt-parse`: exit gracefully and give line number information in case of
+  syntax error.
 * `e3-convert-xunit`: truncate too long test result messages.
 * `e3-convert-xunit`: warn about dangling XFAILs annotations.
 * `DiffTestDriver`: tolerate missing baseline files when rewriting baselines.

pyproject.toml

@@ -21,9 +21,10 @@ classifiers = [
 ]
 
 [project.scripts]
-e3-find-skipped-tests = "e3.testsuite.find_skipped_tests:main"
 e3-convert-xunit = "e3.testsuite.report.xunit:convert_main"
-e3-opt-parser = "e3.testsuite.optfileparser:main"
+e3-find-skipped-tests = "e3.testsuite.find_skipped_tests:main"
+e3-opt-parser = "e3.testsuite.optfileparser:eval_main"
+e3-opt-check = "e3.testsuite.optfileparser:check_syntax_main"
 e3-run-test-fragment = "e3.testsuite.fragment:run_fragment"
 e3-test = "e3.testsuite.main:main"
 e3-testsuite-report = "e3.testsuite.report.display:main"

src/e3/testsuite/optfileparser.py

@@ -11,6 +11,7 @@
 import logging
 import os.path
 import re
+import sys
 from typing import Dict, List, Optional, TYPE_CHECKING, Tuple, Union
 
 
@@ -148,7 +149,7 @@ def get_note(self, sep: Optional[str] = None) -> Union[str, List[str]]:
                 return ""
 
     # INTERNAL FUNCTIONS
-    def __process_opt_line(self, line: str) -> None:
+    def __process_opt_line(self, line: str, lineno: int) -> None:
         """process one line of a test.opt type file.
 
         :raise BadFormattingError: in case the line cannot be parsed
@@ -168,7 +169,9 @@ def __process_opt_line(self, line: str) -> None:
 
         m = OPTLINE_REGEXPS.match(processed_line)
         if m is None:
-            raise BadFormattingError("Can not parse line: " + line)
+            raise BadFormattingError(
+                f"Can not parse line {lineno}: {line.rstrip()}"
+            )
 
         # find command, tags and argument
         tags = m.group(1).split(",")
@@ -240,13 +243,13 @@ def __match(self, tag_list: List[str]) -> bool:
     def __parse_file(self, filename: Union[str, List[str]]) -> None:
         have_opt_data = False
         if isinstance(filename, list):
-            for line in filename:
-                self.__process_opt_line(line)
+            for lineno, line in enumerate(filename, 1):
+                self.__process_opt_line(line, lineno)
             have_opt_data = True
         elif os.path.isfile(filename):
             with open(filename, "r") as optfile:
-                for line in optfile:
-                    self.__process_opt_line(line)
+                for lineno, line in enumerate(optfile, 1):
+                    self.__process_opt_line(line, lineno)
             have_opt_data = True
 
         if have_opt_data:
@@ -288,7 +291,7 @@ def __str__(self) -> str:
         return result
 
 
-def main(argv: Optional[List[str]] = None) -> None:
+def eval_main(argv: Optional[List[str]] = None) -> None:
     parser = argparse.ArgumentParser(
         description="Evaluate test.opt against set of tags"
     )
@@ -312,9 +315,37 @@ def main(argv: Optional[List[str]] = None) -> None:
     for tag in args.tags_list:
         system_tags.extend(tag.split(","))
 
-    opt_result = OptFileParse(system_tags, args.opt_filename)
+    try:
+        opt_result = OptFileParse(system_tags, args.opt_filename)
+    except BadFormattingError as exc:
+        print(str(exc))
+        sys.exit(1)
     print(str(opt_result))
 
 
+def check_syntax_main(argv: list[str] | None = None) -> None:
+    parser = argparse.ArgumentParser(
+        description="""
+        Look for syntax errors in "opt" files.
+
+        Print nothing and exit with status code 0 if no syntax error was found.
+        Print error messages and exit with status code 1 otherwise.
+        """
+    )
+    parser.add_argument(
+        "filenames", nargs="+", help='The name of the "opt" file to parse.'
+    )
+    args = parser.parse_args(argv)
+
+    has_errors = False
+    for filename in args.filenames:
+        try:
+            OptFileParse([], filename)
+        except BadFormattingError as exc:
+            print(f"{filename}: {exc}")
+            has_errors = True
+    sys.exit(1 if has_errors else 0)
+
+
 if __name__ == "__main__":  # no coverage
-    main()
+    eval_main()

tests/tests/optfiles/syntax_error.opt

@@ -1 +1,3 @@
+ALL DEAD
 ? ?
+x86_64-linux OUT linux64.out

tests/tests/optfiles/syntax_error_2.opt

@@ -0,0 +1,2 @@
+--  Leading spaces are forbidden
+ ALL DEAD

tests/tests/test_optfileparser.py

@@ -6,6 +6,9 @@
 from e3.testsuite.optfileparser import BadFormattingError, OptFileParse
 
 
+optfiles_dir = os.path.join(os.path.dirname(__file__), "optfiles")
+
+
 def parse_file(filename, tags=None):
     """Parse an optfile in the "optfiles" subdirectory."""
     tags = tags if tags is not None else []
@@ -133,7 +136,7 @@ def test_syntax_error():
     try:
         parse_file("syntax_error.opt")
     except BadFormattingError as exc:
-        assert str(exc) == "Can not parse line: ? ?\n"
+        assert str(exc) == "Can not parse line 2: ? ?"
     else:
         raise AssertionError()
 
@@ -152,16 +155,13 @@ def test_required():
 
 def run_opt_parser_script(filename, tags=None):
     """Call e3-opt-parser and return the corresponding Run object."""
-    parser_cmd = [
-        "e3-opt-parser",
-        os.path.join(os.path.dirname(__file__), "optfiles", filename),
-    ]
+    parser_cmd = ["e3-opt-parser", os.path.join(optfiles_dir, filename)]
     if tags is not None:
         parser_cmd.extend(tags)
     return Run(parser_cmd)
 
 
-def test_main():
+def test_eval_main():
     """Test the function called by the command-line wrapper to OptFileParse."""
     p = run_opt_parser_script("tags.opt", None)
     assert p.status == 0
@@ -190,3 +190,33 @@ def test_main():
 cmd="linux.cmd"
 """
     )
+
+
+def test_eval_main_syntax_error():
+    """Check that e3-opt-parser exits cleanly in case of parsing error."""
+    p = run_opt_parser_script("syntax_error.opt", None)
+    assert p.status == 1
+    assert p.out == "Can not parse line 2: ? ?\n"
+
+
+def test_check_syntax_main():
+    """Check that e3-opt-check behaves as expected."""
+    p = Run(
+        [
+            "e3-opt-check",
+            "dead.opt",
+            "syntax_error.opt",
+            "tags.opt",
+            "syntax_error_2.opt",
+        ],
+        cwd=optfiles_dir,
+    )
+    assert p.status == 1
+    assert p.out == (
+        "syntax_error.opt: Can not parse line 2: ? ?\n"
+        "syntax_error_2.opt: Can not parse line 2:  ALL DEAD\n"
+    )
+
+    p = Run(["e3-opt-check", "dead.opt", "tags.opt"], cwd=optfiles_dir)
+    assert p.status == 0
+    assert p.out == ""