Check if condition and needs input for pr-builder

With rapidsai/shared-workflows#237, workflows may now specify `if: always()` to allow skipping jobs. This requires passing a `needs` input with `${{ toJSON(needs) }}`. Enforce this new rule in `rapids-check-pr-job-dependencies`, and add some tests for the tool.
rapidsai · Aug 23, 2024 · 2e100c9 · 2e100c9
1 parent 78b92f7
commit 2e100c9
Show file tree

Hide file tree

Showing 2 changed files with 228 additions and 0 deletions.
diff --git a/tests/test_rapids-check-pr-job-dependencies.py b/tests/test_rapids-check-pr-job-dependencies.py
@@ -0,0 +1,209 @@
+import os.path
+import pytest
+import subprocess
+from textwrap import dedent
+
+TOOLS_DIR = os.path.join(os.path.dirname(os.path.realpath(__file__)), "..", "tools")
+
+
+@pytest.mark.parametrize(
+    ["contents", "ignored_jobs", "output", "exit_code"],
+    [
+        (
+            dedent(
+                """\
+                jobs:
+                    pr-builder:
+                        needs:
+                            - job1
+                            - job2
+                    job1:
+                    job2:
+                """
+            ),
+            None,
+            "pr-builder depends on all other jobs.\n",
+            0,
+        ),
+        (
+            dedent(
+                """\
+                jobs:
+                    pr-builder:
+                        needs:
+                            - job1
+                            - job2
+                    job1:
+                    job2:
+                """
+            ),
+            [],
+            "pr-builder depends on all other jobs.\n",
+            0,
+        ),
+        (
+            dedent(
+                """\
+                jobs:
+                    pr-builder:
+                        needs:
+                            - job1
+                            - job2
+                    job1:
+                    job2:
+                    job3:
+                """
+            ),
+            ["job3"],
+            "pr-builder depends on all other jobs.\n",
+            0,
+        ),
+        (
+            dedent(
+                """\
+                jobs:
+                    pr-builder:
+                        needs:
+                            - job1
+                            - job2
+                    job1:
+                    job2:
+                    job3:
+                """
+            ),
+            None,
+            dedent(
+                """\
+                'pr-builder' job is missing the following dependent jobs:
+                  - job3
+                
+                Update '{filename}' to include these missing jobs for 'pr-builder'.
+                Alternatively, you may ignore these jobs by passing them as an argument to this script.
+                """
+            ),
+            1,
+        ),
+        (
+            dedent(
+                """\
+                jobs:
+                    pr-builder:
+                        needs:
+                            - job1
+                            - job2
+                        if: true
+                    job1:
+                    job2:
+                """
+            ),
+            None,
+            dedent(
+                """\
+                If 'pr-builder' job has an 'if' condition, it must be set to 'always()'.
+                
+                Update '{filename}' to set the correct 'if' condition.
+                """
+            ),
+            1,
+        ),
+        (
+            dedent(
+                """\
+                jobs:
+                    pr-builder:
+                        needs:
+                            - job1
+                            - job2
+                        if: always()
+                    job1:
+                    job2:
+                """
+            ),
+            None,
+            dedent(
+                """\
+                If 'pr-builder' job has an 'if' condition, it must also set the 'needs' input to '${{{{ toJSON(needs) }}}}'.
+                
+                Update '{filename}' to add the following:
+
+                with:
+                  needs: ${{{{ toJSON(needs) }}}}
+                """
+            ),
+            1,
+        ),
+        (
+            dedent(
+                """\
+                jobs:
+                    pr-builder:
+                        needs:
+                            - job1
+                            - job2
+                        if: always()
+                        with:
+                            needs: invalid
+                    job1:
+                    job2:
+                """
+            ),
+            None,
+            dedent(
+                """\
+                If 'pr-builder' job has an 'if' condition, it must also set the 'needs' input to '${{{{ toJSON(needs) }}}}'.
+                
+                Update '{filename}' to add the following:
+
+                with:
+                  needs: ${{{{ toJSON(needs) }}}}
+                """
+            ),
+            1,
+        ),
+        (
+            dedent(
+                """\
+                jobs:
+                    pr-builder:
+                        needs:
+                            - job1
+                            - job2
+                        if: always()
+                        with:
+                            needs: ${{ toJSON(needs) }}
+                    job1:
+                    job2:
+                """
+            ),
+            None,
+            "pr-builder depends on all other jobs.\n",
+            0,
+        ),
+    ],
+)
+def test_rapids_check_pr_job_dependency(
+    tmp_path: os.PathLike,
+    contents: str,
+    ignored_jobs: list[str],
+    output: str,
+    exit_code: int,
+):
+    filename = os.path.join(tmp_path, "pr.yaml")
+    with open(filename, "w") as f:
+        f.write(contents)
+    result = subprocess.run(
+        [
+            os.path.join(TOOLS_DIR, "rapids-check-pr-job-dependencies"),
+            *([] if ignored_jobs is None else [" ".join(ignored_jobs)]),
+
+        ],
+        env={
+            **os.environ,
+            "WORKFLOW_FILE": filename,
+        },
+        text=True,
+        capture_output=True,
+    )
+    assert result.stdout == output.format(filename=filename)
+    assert result.stderr == ""
+    assert result.returncode == exit_code
diff --git a/tools/rapids-check-pr-job-dependencies b/tools/rapids-check-pr-job-dependencies
@@ -37,4 +37,23 @@ if yq -en 'env(MISSING_JOBS) | length != 0' >/dev/null 2>&1; then
   exit 1
 fi
 
+if if_condition="$(yq -e '.jobs.[env(PR_BUILDER_JOB_NAME)].if' "${WORKFLOW_FILE}" 2>/dev/null)"; then
+  if [[ "${if_condition}" != "always()" ]]; then
+    echo "If '${PR_BUILDER_JOB_NAME}' job has an 'if' condition, it must be set to 'always()'."
+    echo ""
+    echo "Update '${WORKFLOW_FILE}' to set the correct 'if' condition."
+    exit 1
+  fi
+
+  if yq -e '.jobs.[env(PR_BUILDER_JOB_NAME)].with.needs != "${{ toJSON(needs) }}"' "${WORKFLOW_FILE}" >/dev/null 2>&1; then
+    echo "If '${PR_BUILDER_JOB_NAME}' job has an 'if' condition, it must also set the 'needs' input to '\${{ toJSON(needs) }}'."
+    echo ""
+    echo "Update '${WORKFLOW_FILE}' to add the following:"
+    echo ""
+    echo "with:"
+    echo "  needs: \${{ toJSON(needs) }}"
+    exit 1
+  fi
+fi
+
 echo "${PR_BUILDER_JOB_NAME} depends on all other jobs."