From fcec8e025675b3c05c59f88bd5c7a4262ca76bdd Mon Sep 17 00:00:00 2001
From: Taus <tausbn@github.com>
Date: Tue, 22 Oct 2024 15:11:51 +0000
Subject: [PATCH] Python: Fail tests when errors/warnings are logged

This is primarily useful for ensuring that errors where a node does not
have an appropriate context set in `python.tsg` actually have an effect
on the pass/fail status of the parser tests. Previously, these would
just be logged to stdout, but test could still succeed when there were
errors present.

Also fixes one of the logging lines in `tsg_parser.py` to be more
consistent with the others.
---
 .../extractor/semmle/python/parser/dump_ast.py | 18 ++++++++++++++++++
 .../semmle/python/parser/tsg_parser.py         |  2 +-
 python/extractor/tests/test_parser.py          |  8 ++++++++
 3 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/python/extractor/semmle/python/parser/dump_ast.py b/python/extractor/semmle/python/parser/dump_ast.py
index fbeaabb2939..23e1a973dfc 100644
--- a/python/extractor/semmle/python/parser/dump_ast.py
+++ b/python/extractor/semmle/python/parser/dump_ast.py
@@ -97,9 +97,27 @@ class AstDumper(object):
 
 
 class StdoutLogger(logging.Logger):
+    error_count = 0
     def log(self, level, fmt, *args):
         sys.stdout.write(fmt % args + "\n")
 
+    def info(self, fmt, *args):
+        self.log(logging.INFO, fmt, *args)
+
+    def warn(self, fmt, *args):
+        self.log(logging.WARN, fmt, *args)
+        self.error_count += 1
+
+    def error(self, fmt, *args):
+        self.log(logging.ERROR, fmt, *args)
+        self.error_count += 1
+
+    def had_errors(self):
+        return self.error_count > 0
+
+    def reset_error_count(self):
+        self.error_count = 0
+
 def old_parser(inputfile, logger):
     mod = PythonSourceModule(None, inputfile, logger)
     logger.close()
diff --git a/python/extractor/semmle/python/parser/tsg_parser.py b/python/extractor/semmle/python/parser/tsg_parser.py
index 46784c4e860..7ddd6813da7 100644
--- a/python/extractor/semmle/python/parser/tsg_parser.py
+++ b/python/extractor/semmle/python/parser/tsg_parser.py
@@ -440,7 +440,7 @@ def concatenate_stringparts(stringparts, logger):
     try:
         return "".join(decode_str(stringpart.s) for stringpart in stringparts)
     except Exception as ex:
-        logger.error("Unable to concatenate string %s getting error %s", stringparts, ex)
+        logger.error("Unable to concatenate string {} getting error {}".format(stringparts, ex))
         return stringparts[0].s
 
 
diff --git a/python/extractor/tests/test_parser.py b/python/extractor/tests/test_parser.py
index e93ffd1ffd7..0c74a62f452 100644
--- a/python/extractor/tests/test_parser.py
+++ b/python/extractor/tests/test_parser.py
@@ -49,6 +49,8 @@ class ParserTest(unittest.TestCase):
             diff = e.output
         if diff:
             pytest.fail(diff.decode("utf-8"))
+        self.check_for_stdout_errors(logger)
+
         self.assertEqual(self.capsys.readouterr().err, "")
         os.remove(oldfile)
         os.remove(newfile)
@@ -84,9 +86,15 @@ class ParserTest(unittest.TestCase):
             diff = e.output
         if diff:
             pytest.fail(diff.decode("utf-8"))
+
+        self.check_for_stdout_errors(logger)
         self.assertEqual(self.capsys.readouterr().err, "")
         os.remove(actual)
 
+    def check_for_stdout_errors(self, logger):
+        if logger.had_errors():
+            logger.reset_error_count()
+            pytest.fail("Errors/warnings were logged to stdout during testing.")
 
 def setup_tests():
     test_folder = os.path.join(os.path.dirname(__file__), "parser")