fixup! CI - restyle script summary and annotations in PRs

Author: mcspr

.github/workflows/style-check.yml

@@ -26,6 +26,7 @@ jobs:
     - name: Style check
       run: |
           sudo apt update
+          python ./tests/test_restyle.py --quiet
           bash ./tests/ci/style_check.sh
 
   # Validate orthography

tests/host/common/ArduinoCatch.hpp

@@ -24,13 +24,15 @@
 
 std::ostream& operator<<(std::ostream&, const String&);
 
-namespace Catch {
+namespace Catch
+{
 
 std::string toString(const String&);
 
 template<>
-struct StringMaker<String> {
+struct StringMaker<String>
+{
     static std::string convert(const String&);
 };
 
-} // namespace Catch
+}  // namespace Catch

tests/restyle.py

@@ -45,6 +45,17 @@ def ls_files(patterns):
     return out
 
 
+def diff_lines():
+    proc = subprocess.run(
+        ["git", "--no-pager", "diff", "--ignore-submodules"],
+        capture_output=True,
+        check=True,
+        universal_newlines=True,
+    )
+
+    return proc.stdout.split("\n")
+
+
 def find_files(patterns):
     """Filesystem search, matches both git and non-git files"""
     return [
@@ -98,7 +109,7 @@ def find_arduino_files():
 @dataclass
 class Changed:
     file: str
-    hunk: list[str]
+    hunk: str
     lines: list[int]
 
 
@@ -126,28 +137,31 @@ def pop(self, out, line):
         self.reset_with_line(line)
 
 
-def changed_files():
+def changed_files_for_diff(lines: list[str] | str) -> list[Changed]:
     """
-    Naive git-diff output parser. Generates list[Changed] for every file changed after clang-format.
+    Naive git-diff output parser. Generates list of objects for every file changed after clang-format.
     """
-    proc = subprocess.run(
-        ["git", "--no-pager", "diff"],
-        capture_output=True,
-        check=True,
-        universal_newlines=True,
-    )
+    match lines:
+        case str():
+            lines = lines.split("\n")
+        case list():
+            pass
+        case _:
+            raise ValueError("Unknown 'lines' type, can be either list[str] or str")
 
     ctx = Context()
     out = []
 
     # TODO: pygit2?
     # ref. https://github.com/cpp-linter/cpp-linter/blob/main/cpp_linter/git/__init__.py ::parse_diff
     # ref. https://github.com/libgit2/pygit2/blob/master/src/diff.c ::parse_diff
-    for line in proc.stdout.split("\n"):
+    for line in lines:
         # '--- a/path/to/changed/file' most likely
-        # '--- a/dev/null' aka created file. should be ignored, same as removed ones
+        # '--- /dev/null' aka created file. should be ignored, same as removed ones
         if line.startswith("---"):
             ctx.pop(out, line)
+
+            _, file = line.split(" ")
             ctx.deleted = "/dev/null" in file
 
         # '+++ b/path/to/changed/file' most likely
@@ -183,6 +197,10 @@ def changed_files():
     return out
 
 
+def changed_files() -> list[Changed]:
+    return changed_files_for_diff(diff_lines())
+
+
 def errors_changed(changed: Changed):
     all_lines = ", ".join(str(x) for x in changed.lines)
     for line in changed.lines:
@@ -204,12 +222,14 @@ def summary_diff(changed: Changed):
 
 
 def stdout_diff():
-    subprocess.run(["git", "--no-pager", "diff"])
+    subprocess.run(["git", "--no-pager", "diff", "--ignore-submodules"])
 
 
 def assert_unchanged():
     subprocess.run(
-        ["git", "diff", "--exit-code"], check=True, stdout=subprocess.DEVNULL
+        ["git", "diff", "--ignore-submodules", "--exit-code"],
+        check=True,
+        stdout=subprocess.DEVNULL,
     )
 
 

tests/test_restyle.py

@@ -0,0 +1,182 @@
+import unittest
+
+from restyle import changed_files_for_diff
+
+# small git-diff samples from https://queirozf.com/entries/git-diff-reference-and-examples
+
+
+class BaseTest(unittest.TestCase):
+    def testNewLine(self):
+        diff = """
+diff --git a/file.txt b/file.txt
+index 257cc56..3bd1f0e 100644
+--- a/file.txt
++++ b/file.txt
+@@ -1 +1,2 @@
+ foo
++bar
+"""
+        changed = changed_files_for_diff(diff)
+        self.assertEqual(1, len(changed))
+        self.assertEqual("file.txt", changed[0].file)
+        self.assertEqual(1, len(changed[0].lines))
+        self.assertEqual(1, changed[0].lines[0])
+
+        expected = """
+--- a/file.txt
++++ b/file.txt
+@@ -1 +1,2 @@
+ foo
++bar
+""".strip()
+        self.assertEqual(expected, changed[0].hunk.strip())
+
+    def testNewLines(self):
+        diff = """
+diff --git a/file.txt b/file.txt
+index 257cc56..3bd1f0e 100644
+--- a/file2.txt
++++ b/file2.txt
+@@ -1 +1,2 @@
+ foo
++bar
+ baz
+@@ -1 +10,2 @@
+ 222
+-222
+ 333
+@@ -1 +100,3 @@
+ aaa
++bbb
++ccc
+ ddd
+"""
+        changed = changed_files_for_diff(diff)
+        self.assertEqual(1, len(changed))
+        self.assertEqual("file2.txt", changed[0].file)
+
+        lines = changed[0].lines
+        self.assertEqual(3, len(lines))
+
+        first, second, third = lines
+        self.assertEqual(1, first)
+        self.assertEqual(10, second)
+        self.assertEqual(100, third)
+
+        expected = """
+--- a/file2.txt
++++ b/file2.txt
+@@ -1 +1,2 @@
+ foo
++bar
+ baz
+@@ -1 +10,2 @@
+ 222
+-222
+ 333
+@@ -1 +100,3 @@
+ aaa
++bbb
++ccc
+ ddd
+""".strip()
+        self.assertEqual(expected, changed[0].hunk.strip())
+
+    def testRemovedLineAndDeletedFile(self):
+        diff = """
+diff --git a/file.txt b/file.txt
+index 3bd1f0e..257cc56 100644
+--- a/file.txt
++++ b/file.txt
+@@ -1,2 +1 @@
+ foo
+-bar
+diff --git a/file2.txt b/file2.txt
+deleted file mode 100644
+index 85553e8..0000000
+--- a/file2.txt
++++ /dev/null
+@@ -1,2 +0,0 @@
+-aaaaaa
+-bbbbbb
+"""
+        changed = changed_files_for_diff(diff)
+        self.assertEqual(1, len(changed))
+        self.assertEqual("file.txt", changed[0].file)
+        self.assertEqual(1, len(changed[0].lines))
+        self.assertEqual(1, changed[0].lines[0])
+
+        expected = """
+--- a/file.txt
++++ b/file.txt
+@@ -1,2 +1 @@
+ foo
+-bar
+""".strip()
+        self.assertEqual(expected, changed[0].hunk.strip())
+
+    def testNewLineAndDeletedFile(self):
+        diff = """
+diff --git a/file.txt b/file.txt
+index 3bd1f0e..86e041d 100644
+--- a/file.txt
++++ b/file.txt
+@@ -1,2 +1,3 @@
+ foo
+ bar
++baz
+diff --git a/file2.txt b/file2.txt
+deleted file mode 100644
+index 85553e8..0000000
+--- a/file2.txt
++++ /dev/null
+@@ -1,2 +0,0 @@
+-aaaaaa
+-bbbbbb
+"""
+        changed = changed_files_for_diff(diff)
+        self.assertEqual(1, len(changed))
+        self.assertEqual("file.txt", changed[0].file)
+        self.assertEqual(1, len(changed[0].lines))
+        self.assertEqual(1, changed[0].lines[0])
+
+        expected = """
+--- a/file.txt
++++ b/file.txt
+@@ -1,2 +1,3 @@
+ foo
+ bar
++baz
+""".strip()
+        self.assertEqual(expected, changed[0].hunk.strip())
+
+    def testDeletedFile(self):
+        diff = """
+diff --git a/file2.txt b/file2.txt
+deleted file mode 100644
+index 85553e8..0000000
+--- a/file2.txt
++++ /dev/null
+@@ -1,2 +0,0 @@
+-aaaaaa
+-bbbbbb
+"""
+        changed = changed_files_for_diff(diff)
+        self.assertEqual(0, len(changed))
+
+    def testNewFile(self):
+        diff = """
+diff --git a/file3.txt b/file3.txt
+new file mode 100644
+index 0000000..a309e46
+--- /dev/null
++++ b/file3.txt
+@@ -0,0 +1 @@
++this is file3
+"""
+        changed = changed_files_for_diff(diff)
+        self.assertEqual(0, len(changed))
+
+
+if __name__ == '__main__':
+    unittest.main()