Merge pull request #61148 from ClickHouse/ci_fast_style

CI: style check: conditionally skip cpp or py files check in prs

tests/ci/report.py

@@ -288,6 +288,9 @@ class JobReport:
     # if False no GH commit status will be created by CI
     need_commit_status: bool = True
 
+    def __post_init__(self):
+        assert self.status in (SUCCESS, ERROR, FAILURE, PENDING)
+
     @classmethod
     def exist(cls) -> bool:
         return JOB_REPORT_FILE.is_file()

tests/ci/style_check.py

@@ -4,13 +4,14 @@ from concurrent.futures import ProcessPoolExecutor
 import csv
 import logging
 import os
+import shutil
 import subprocess
 import sys
 from pathlib import Path
 from typing import List, Tuple
 
 from docker_images_helper import get_docker_image, pull_image
-from env_helper import REPO_COPY, TEMP_PATH
+from env_helper import CI, REPO_COPY, TEMP_PATH
 from git_helper import GIT_PREFIX, git_runner
 from pr_info import PRInfo
 from report import ERROR, FAILURE, SUCCESS, JobReport, TestResults, read_test_results
@@ -126,34 +127,57 @@ def main():
 
     repo_path = Path(REPO_COPY)
     temp_path = Path(TEMP_PATH)
+    if temp_path.is_dir():
+        shutil.rmtree(temp_path)
     temp_path.mkdir(parents=True, exist_ok=True)
 
-    # pr_info = PRInfo()
+    pr_info = PRInfo()
 
     IMAGE_NAME = "clickhouse/style-test"
     image = pull_image(get_docker_image(IMAGE_NAME))
-    cmd_1 = (
+    cmd_cpp = (
         f"docker run -u $(id -u ${{USER}}):$(id -g ${{USER}}) --cap-add=SYS_PTRACE "
         f"--volume={repo_path}:/ClickHouse --volume={temp_path}:/test_output "
         f"--entrypoint= -w/ClickHouse/utils/check-style "
-        f"{image} ./check_cpp_docs.sh"
+        f"{image} ./check_cpp.sh"
     )
-    cmd_2 = (
+    cmd_py = (
         f"docker run -u $(id -u ${{USER}}):$(id -g ${{USER}}) --cap-add=SYS_PTRACE "
         f"--volume={repo_path}:/ClickHouse --volume={temp_path}:/test_output "
         f"--entrypoint= -w/ClickHouse/utils/check-style "
         f"{image} ./check_py.sh"
     )
-    logging.info("Is going to run the command: %s", cmd_1)
-    logging.info("Is going to run the command: %s", cmd_2)
+    cmd_docs = (
+        f"docker run -u $(id -u ${{USER}}):$(id -g ${{USER}}) --cap-add=SYS_PTRACE "
+        f"--volume={repo_path}:/ClickHouse --volume={temp_path}:/test_output "
+        f"--entrypoint= -w/ClickHouse/utils/check-style "
+        f"{image} ./check_docs.sh"
+    )
 
     with ProcessPoolExecutor(max_workers=2) as executor:
-        # Submit commands for execution in parallel
-        future1 = executor.submit(subprocess.run, cmd_1, shell=True)
-        future2 = executor.submit(subprocess.run, cmd_2, shell=True)
-        # Wait for both commands to complete
-        _ = future1.result()
-        _ = future2.result()
+        logging.info("Run docs files check: %s", cmd_docs)
+        future = executor.submit(subprocess.run, cmd_docs, shell=True)
+        # Parallelization  does not make it faster - run subsequently
+        _ = future.result()
+
+        run_cppcheck = True
+        run_pycheck = True
+        if CI and pr_info.number > 0:
+            pr_info.fetch_changed_files()
+            if not any(file.endswith(".py") for file in pr_info.changed_files):
+                run_pycheck = False
+            if all(file.endswith(".py") for file in pr_info.changed_files):
+                run_cppcheck = False
+
+        if run_cppcheck:
+            logging.info("Run source files check: %s", cmd_cpp)
+            future1 = executor.submit(subprocess.run, cmd_cpp, shell=True)
+            _ = future1.result()
+
+        if run_pycheck:
+            logging.info("Run py files check: %s", cmd_py)
+            future2 = executor.submit(subprocess.run, cmd_py, shell=True)
+            _ = future2.result()
 
     # if args.push:
     #     checkout_head(pr_info)

utils/check-style/check_cpp.sh

@@ -4,31 +4,35 @@
 
 cd /ClickHouse/utils/check-style || echo -e "failure\tRepo not found" > /test_output/check_status.tsv
 
+start_total=`date +%s`
+
 # FIXME: 30 sec to wait
 # echo "Check duplicates" | ts
 # ./check-duplicate-includes.sh |& tee /test_output/duplicate_includes_output.txt
 
-echo "Check style" | ts
+start=`date +%s`
 ./check-style -n              |& tee /test_output/style_output.txt
-echo "Check typos" | ts
-./check-typos                 |& tee /test_output/typos_output.txt
-echo "Check docs spelling" | ts
-./check-doc-aspell            |& tee /test_output/docs_spelling_output.txt
-echo "Check whitespaces" | ts
+runtime=$((`date +%s`-start))
+echo "Check style. Done. $runtime seconds."
+
+start=`date +%s`
 ./check-whitespaces -n        |& tee /test_output/whitespaces_output.txt
-echo "Check workflows" | ts
+runtime=$((`date +%s`-start))
+echo "Check whitespaces. Done. $runtime seconds."
+
+start=`date +%s`
 ./check-workflows             |& tee /test_output/workflows_output.txt
-echo "Check submodules" | ts
+runtime=$((`date +%s`-start))
+echo "Check workflows. Done. $runtime seconds."
+
+start=`date +%s`
 ./check-submodules            |& tee /test_output/submodules_output.txt
-echo "Check style. Done" | ts
+runtime=$((`date +%s`-start))
+echo "Check submodules. Done. $runtime seconds."
 
 # FIXME: 6 min to wait
 # echo "Check shell scripts with shellcheck" | ts
 # ./shellcheck-run.sh           |& tee /test_output/shellcheck_output.txt
 
-
-# FIXME: move out
-# /process_style_check_result.py || echo -e "failure\tCannot parse results" > /test_output/check_status.tsv
-# echo "Check help for changelog generator works" | ts
-# cd ../changelog || exit 1
-# ./changelog.py -h 2>/dev/null 1>&2
+runtime=$((`date +%s`-start_total))
+echo "Check style total. Done. $runtime seconds."

utils/check-style/check_docs.sh

@@ -0,0 +1,20 @@
+#!/bin/bash
+
+# yaml check is not the best one
+
+cd /ClickHouse/utils/check-style || echo -e "failure\tRepo not found" > /test_output/check_status.tsv
+
+start_total=`date +%s`
+
+start=`date +%s`
+./check-typos                 |& tee /test_output/typos_output.txt
+runtime=$((`date +%s`-start))
+echo "Check typos. Done. $runtime seconds."
+
+start=`date +%s`
+./check-doc-aspell            |& tee /test_output/docs_spelling_output.txt
+runtime=$((`date +%s`-start))
+echo "Check docs spelling. Done. $runtime seconds."
+
+runtime=$((`date +%s`-start_total))
+echo "Check Docs, total. Done. $runtime seconds."

utils/check-style/check_py.sh

@@ -1,17 +1,22 @@
 #!/bin/bash
 
-# yaml check is not the best one
-
 cd /ClickHouse/utils/check-style || echo -e "failure\tRepo not found" > /test_output/check_status.tsv
 
+start_total=`date +%s`
+
 # FIXME: 1 min to wait + head checkout
 # echo "Check python formatting with black" | ts
 # ./check-black -n              |& tee /test_output/black_output.txt
 
-echo "Check pylint" | ts
+start=`date +%s`
 ./check-pylint -n               |& tee /test_output/pylint_output.txt
-echo "Check pylint. Done" | ts
+runtime=$((`date +%s`-start))
+echo "Check pylint. Done. $runtime seconds."
 
-echo "Check python type hinting with mypy" | ts
+start=`date +%s`
 ./check-mypy -n               |& tee /test_output/mypy_output.txt
-echo "Check python type hinting with mypy. Done" | ts
+runtime=$((`date +%s`-start))
+echo "Check python type hinting with mypy. Done. $runtime seconds."
+
+runtime=$((`date +%s`-start_total))
+echo "Check python total. Done. $runtime seconds."

utils/check-style/process_style_check_result.py

@@ -13,11 +13,11 @@ def process_result(result_folder):
     description = ""
     test_results = []
     checks = (
-        #"duplicate includes",
-        #"shellcheck",
+        # "duplicate includes",
+        # "shellcheck",
         "style",
         "pylint",
-        #"black",
+        # "black",
         "mypy",
         "typos",
         "whitespaces",
@@ -30,11 +30,15 @@ def process_result(result_folder):
         out_file = name.replace(" ", "_") + "_output.txt"
         full_path = os.path.join(result_folder, out_file)
         if not os.path.exists(full_path):
-            logging.info("No %s check log on path %s", name, full_path)
-            return "exception", f"No {name} check log", []
+            test_results.append((f"Check {name}", "SKIPPED"))
         elif os.stat(full_path).st_size != 0:
+            with open(full_path, 'r') as file:
+                lines = file.readlines()
+                if len(lines) > 100:
+                    lines = lines[:100] + ['====TRIMMED====']
+                content = "\n".join(lines)
             description += f"Check {name} failed. "
-            test_results.append((f"Check {name}", "FAIL"))
+            test_results.append((f"Check {name}", "FAIL", None, content))
             status = "failure"
         else:
             test_results.append((f"Check {name}", "OK"))
@@ -42,6 +46,8 @@ def process_result(result_folder):
     if not description:
         description += "Style check success"
 
+    assert test_results, "No single style-check output found"
+
     return status, description, test_results