enhance PR compliance validation: add new tests, improve regex checks, and refine workflows

allnes · allnes · commit adaee7116d79 · 2025-09-11T14:38:50.000+02:00
diff --git a/.github/scripts/validate_pr.py b/.github/scripts/validate_pr.py
@@ -28,7 +28,7 @@
 (?P<firstname>[А-ЯA-ZЁ][а-яa-zё]+)\s+
 (?P<middlename>[А-ЯA-ZЁ][а-яa-zё]+)\.\s+
 (?P<group>\d{3,5}(?:-[A-Za-zА-Яа-я0-9-]+)?)\.\s+
-(?P<taskname>.+)
+(?P<taskname>\S.*)
 $
 '''
 
@@ -68,9 +68,8 @@ def validate_title(title: str) -> List[str]:
             f"Example (EN): {example_en}"
         ]
 
-    # Full strict match
-    if re.match(TITLE_REGEX, title, flags=re.UNICODE | re.VERBOSE) is not None:
-        return errors
+    # Full strict match (record but do not early-return to allow granular checks)
+    full_match = re.match(TITLE_REGEX, title, flags=re.UNICODE | re.VERBOSE) is not None
 
     # If not matched – try to pinpoint the failing segment.
     # 1) Optional prefix is allowed; strip it for partial checks
@@ -118,17 +117,12 @@ def validate_title(title: str) -> List[str]:
 
     rest = rest[m.end() :]
 
-    # 5) Task name must be non-empty and end with a period is not explicitly required
-    # by regex (taskname can contain a trailing period as part of text). Ensure non-empty
-    if not rest.strip():
-        errors.append(
-            "Task name is missing — non-empty text is required after the group.\n"
-            f"Example (RU): {example_ru}\n"
-            f"Example (EN): {example_en}"
-        )
-        return errors
+    # 5) Task name validity is enforced by the full regex (non-whitespace start)
 
-    # Fallback
+    # If no specific segment errors and full regex matched, accept
+    if full_match:
+        return errors
+    # Otherwise, fallback message
     errors.append(
         "PR title does not match the required pattern.\n"
         f"Current title: '{title}'\n"
diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml
@@ -5,10 +5,9 @@ on:
     paths:
       - 'docker/**'
       - '.github/workflows/docker.yml'
-  pull_request:
-    paths:
-      - 'docker/**'
-      - '.github/workflows/docker.yml'
+  workflow_run:
+    workflows: ["PR Compliance"]
+    types: [completed]
   workflow_dispatch:
 
 permissions:
@@ -19,7 +18,9 @@ jobs:
   build-and-push:
     name: Build & Push Docker Image
     runs-on: ubuntu-latest
-    if: github.repository == 'learning-process/parallel_programming_course'
+    if: >-
+      ${{ github.repository == 'learning-process/parallel_programming_course' &&
+          (github.event_name != 'workflow_run' || github.event.workflow_run.conclusion == 'success') }}
 
     steps:
       - name: Check out code
diff --git a/.github/workflows/pr-compliance.yml b/.github/workflows/pr-compliance.yml
@@ -5,7 +5,8 @@ on:
     types: [opened, edited, synchronize, reopened]
 
 jobs:
-  validate:
+  unit_tests:
+    name: Unit Tests
     runs-on: ubuntu-latest
     steps:
       - name: Checkout
@@ -21,13 +22,64 @@ jobs:
           python -m pip install --upgrade pip
           python -m pip install coverage
 
-      - name: Run validator unit tests
+      - name: Run validator unit tests with coverage
         run: |
           coverage run -m unittest -v tests/test_validate_pr.py tests/test_validate_pr_main.py
           coverage report --fail-under=100 -m .github/scripts/validate_pr.py
 
-      - name: Validate PR title, body and commits
+  pr_title:
+    name: Validate PR Title
+    runs-on: ubuntu-latest
+    needs: [unit_tests]
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v4
+
+      - name: Set up Python
+        uses: actions/setup-python@v5
+        with:
+          python-version: '3.11'
+
+      - name: Validate PR title
+        run: |
+          python .github/scripts/validate_pr.py --checks title --verbose
+        env:
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+
+  pr_body:
+    name: Validate PR Body
+    runs-on: ubuntu-latest
+    needs: [unit_tests]
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v4
+
+      - name: Set up Python
+        uses: actions/setup-python@v5
+        with:
+          python-version: '3.11'
+
+      - name: Validate PR body
+        run: |
+          python .github/scripts/validate_pr.py --checks body --verbose
+        env:
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+
+  pr_commits:
+    name: Validate PR Commits
+    runs-on: ubuntu-latest
+    needs: [unit_tests]
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v4
+
+      - name: Set up Python
+        uses: actions/setup-python@v5
+        with:
+          python-version: '3.11'
+
+      - name: Validate PR commits
         run: |
-          python .github/scripts/validate_pr.py --checks all --verbose
+          python .github/scripts/validate_pr.py --checks commits --verbose
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
diff --git a/.github/workflows/pre-commit.yml b/.github/workflows/pre-commit.yml
@@ -2,11 +2,15 @@ name: Pre-commit checks
 
 on:
   push:
-  pull_request:
   workflow_call:
+  workflow_run:
+    workflows: ["PR Compliance"]
+    types: [completed]
 
 jobs:
   pre-commit:
+    if: >-
+      ${{ github.event_name != 'workflow_run' || github.event.workflow_run.conclusion == 'success' }}
     runs-on: ubuntu-24.04
     container:
       image: ghcr.io/learning-process/ppc-ubuntu:1.1
diff --git a/.github/workflows/static-analysis-pr.yml b/.github/workflows/static-analysis-pr.yml
@@ -1,16 +1,9 @@
 name: Static analysis
 
 on:
-  pull_request:
-    paths:
-      - '**/*.cpp'
-      - '**/*.hpp'
-      - '**/*.c'
-      - '**/*.h'
-      - '**/CMakeLists.txt'
-      - '**/*.cmake'
-      - '**/.clang-tidy'
-      - '.github/workflows/static-analysis-pr.yml'
+  workflow_run:
+    workflows: ["PR Compliance"]
+    types: [completed]
 
 concurrency:
   group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.sha }}
@@ -21,6 +14,7 @@ concurrency:
 
 jobs:
   clang-tidy:
+    if: ${{ github.event.workflow_run.conclusion == 'success' }}
     runs-on: ubuntu-24.04
     container:
       image: ghcr.io/learning-process/ppc-ubuntu:1.1
@@ -117,6 +111,7 @@ jobs:
           echo "clang-tidy run has failed. See previous 'Run clang-tidy' stage logs"
           exit 1
   nolint-check:
+    if: ${{ github.event.workflow_run.conclusion == 'success' }}
     runs-on: ubuntu-24.04
     steps:
       - uses: actions/checkout@v5
diff --git a/tests/test_validate_pr.py b/tests/test_validate_pr.py
@@ -44,6 +44,14 @@ def test_title_invalid_examples(self):
             with self.subTest(t=t):
                 self.assertNotEqual(self.v.validate_title(t), [])
 
+    def test_title_empty_and_task_prefix_partial(self):
+        # Empty title path
+        errs = self.v.validate_title("")
+        self.assertNotEqual(errs, [])
+        # Strip [TASK] and fail early on task/variant block to cover branch
+        errs = self.v.validate_title("[TASK] Иванов Иван Иванович — задача")
+        self.assertTrue(any("Invalid task/variant block" in e for e in errs))
+
 
 class TestPRBody(unittest.TestCase):
     @classmethod
@@ -140,6 +148,40 @@ def test_body_with_html_comments(self):
         errs = self.v.validate_body(body)
         self.assertTrue(any("Found HTML comments" in e for e in errs))
 
+    def test_body_empty(self):
+        errs = self.v.validate_body("")
+        self.assertTrue(any("PR body is empty" in e for e in errs))
+
+    def test_body_label_without_any_value(self):
+        body = (
+            "## 1. Full name and group\n"
+            "Name and group:\n\n"  # no spaces after colon
+            "## 2. Assignment / Topic / Task\n"
+            "Assignment: A\n\n"
+            "## 3. Technology / Platform used\n"
+            "Technology: A\n\n"
+            "## 4. Goals of the work\n"
+            "Goals: A\n\n"
+            "## 5. Solution description and structure\n"
+            "Description: A\n\n"
+            "## 6. System requirements and build instructions\n"
+            "Build & Run: A\n\n"
+            "## 7. Testing and verification\n"
+            "Testing: A\n\n"
+            "## 8. Results\n"
+            "Results: A\n\n"
+            "## 9. Performance analysis\n"
+            "Analysis: A\n\n"
+            "## 10. Conclusions and possible improvements\n"
+            "Conclusions: A\n\n"
+            "## 11. Limitations / known issues\n"
+            "Limitations: A\n\n"
+            "## 12. CI / static analysis / code style\n"
+            "CI & Style: A\n"
+        )
+        errs = self.v.validate_body(body)
+        self.assertTrue(any("Empty required fields" in e for e in errs))
+
 
 class TestCommitMessages(unittest.TestCase):
     @classmethod
@@ -190,7 +232,20 @@ def test_commit_invalid_cases(self):
                 errs = self.v.validate_commit_message(m)
                 self.assertNotEqual(errs, [])
 
+    def test_title_partial_group_and_taskname_errors_and_fallback(self):
+        # Valid task/variant and name, invalid group
+        t1 = "2-12. Иванов Иван Иванович. ХХХ. Что-то"
+        errs = self.v.validate_title(t1)
+        self.assertTrue(any("Invalid group number" in e for e in errs))
+        # Valid group but trailing newline — parser flags group error in this path
+        t2 = "2-12. Иванов Иван Иванович. 2341-а234. \n"
+        errs = self.v.validate_title(t2)
+        self.assertTrue(any("Invalid group number" in e for e in errs))
+        # Fallback case: embed newline so full regex fails while partial checks pass
+        t3 = "2-12. Иванов Иван Иванович. 2341-а234. Задача\nещё"
+        errs = self.v.validate_title(t3)
+        self.assertTrue(any("does not match the required pattern" in e for e in errs))
+
 
 if __name__ == "__main__":
     unittest.main(verbosity=2)
-
diff --git a/tests/test_validate_pr_main.py b/tests/test_validate_pr_main.py
@@ -125,13 +125,86 @@ def test_main_commits_error_and_internal(self):
                 code = self.v.main()
                 self.assertEqual(code, 1)
 
+    def test_main_title_and_body_missing_in_payload(self):
+        # No payload at all
+        with mock.patch.object(sys, 'argv', ["prog", "--checks", "title", "--verbose"]):
+            os.environ.pop("GITHUB_EVENT_PATH", None)
+            code = self.v.main()
+            self.assertEqual(code, 1)
+        # Payload present but missing title/body triggers error printing branch
+        data = {"pull_request": {"number": 5, "title": "Иванов Иван Иванович — задача", "body": "## 1. Full name and group\nName and group: \n"}}
+        with tempfile.NamedTemporaryFile("w+", delete=False) as tf:
+            json.dump(data, tf)
+            path = tf.name
+        try:
+            with mock.patch.object(sys, 'argv', ["prog", "--checks", "title", "--verbose"]):
+                os.environ["GITHUB_EVENT_PATH"] = path
+                code = self.v.main()
+                self.assertEqual(code, 1)
+            with mock.patch.object(sys, 'argv', ["prog", "--checks", "body", "--verbose"]):
+                os.environ["GITHUB_EVENT_PATH"] = path
+                code = self.v.main()
+                self.assertEqual(code, 1)
+        finally:
+            os.unlink(path)
+
         # Now raise an internal error
         with mock.patch.object(sys, 'argv', ["prog", "--checks", "commits", "--repo", "o/r", "--pr", "1"]):
             os.environ["GITHUB_TOKEN"] = "token"
             with mock.patch.object(self.v, "fetch_pr_commits", side_effect=RuntimeError("boom")):
                 code = self.v.main()
                 self.assertEqual(code, 2)
 
+    def test_main_body_none_and_ok(self):
+        v = _import_validator()
+        # Body None path (no payload)
+        with mock.patch.object(sys, 'argv', ["prog", "--checks", "body", "--verbose"]):
+            os.environ.pop("GITHUB_EVENT_PATH", None)
+            code = v.main()
+            self.assertEqual(code, 1)
+        # Body OK path
+        body = (
+            "## 1. Full name and group\nName and group: A\n\n"
+            "## 2. Assignment / Topic / Task\nAssignment: A\n\n"
+            "## 3. Technology / Platform used\nTechnology: omp\n\n"
+            "## 4. Goals of the work\nGoals: A\n\n"
+            "## 5. Solution description and structure\nDescription: A\n\n"
+            "## 6. System requirements and build instructions\nBuild & Run: A\n\n"
+            "## 7. Testing and verification\nTesting: A\n\n"
+            "## 8. Results\nResults: A\n\n"
+            "## 9. Performance analysis\nAnalysis: A\n\n"
+            "## 10. Conclusions and possible improvements\nConclusions: A\n\n"
+            "## 11. Limitations / known issues\nLimitations: A\n\n"
+            "## 12. CI / static analysis / code style\nCI & Style: A\n"
+        )
+        data = {"pull_request": {"number": 7, "title": "x", "body": body}}
+        with tempfile.NamedTemporaryFile("w+", delete=False) as tf:
+            json.dump(data, tf)
+            path = tf.name
+        try:
+            with mock.patch.object(sys, 'argv', ["prog", "--checks", "body", "--verbose"]):
+                os.environ["GITHUB_EVENT_PATH"] = path
+                code = v.main()
+                self.assertEqual(code, 0)
+        finally:
+            os.unlink(path)
+
+    def test_main_title_print_branch_else(self):
+        v = _import_validator()
+        data = {"pull_request": {"number": 8, "title": "bad", "body": ""}}
+        with tempfile.NamedTemporaryFile("w+", delete=False) as tf:
+            json.dump(data, tf)
+            path = tf.name
+        try:
+            with mock.patch.object(sys, 'argv', ["prog", "--checks", "title"]):
+                # Force errs starting with '✗' to hit print(e)
+                with mock.patch.object(v, 'validate_title', return_value=["✗ forced-error"]):
+                    os.environ["GITHUB_EVENT_PATH"] = path
+                    code = v.main()
+                    self.assertEqual(code, 1)
+        finally:
+            os.unlink(path)
+
 
 class TestRunAsMain(unittest.TestCase):
     def test_run_module_as_main_with_verbose(self):
@@ -171,6 +244,24 @@ def test_run_module_as_main_with_verbose(self):
         finally:
             os.unlink(path)
 
+    def test_systemexit_branch_in_try(self):
+        # Force validate_title to raise SystemExit inside try block to hit 'except SystemExit: raise'
+        v = _import_validator()
+        with mock.patch.object(sys, 'argv', ["prog", "--checks", "title"]):
+            with mock.patch.object(v, 'validate_title', side_effect=SystemExit(5)):
+                # Provide minimal payload with title
+                data = {"pull_request": {"number": 1, "title": "bad", "body": ""}}
+                with tempfile.NamedTemporaryFile("w+", delete=False) as tf:
+                    json.dump(data, tf)
+                    path = tf.name
+                try:
+                    os.environ["GITHUB_EVENT_PATH"] = path
+                    with self.assertRaises(SystemExit) as cm:
+                        v.main()
+                    self.assertEqual(cm.exception.code, 5)
+                finally:
+                    os.unlink(path)
+
 
 if __name__ == "__main__":
     unittest.main(verbosity=2)
