Merge pull request #34289 from ClickHouse/style-check-verbosity

Add verbosity to stylecheck

.github/workflows/release.yml

@@ -22,7 +22,6 @@ jobs:
     - name: Check out repository code
       uses: actions/checkout@v2
     - name: Download packages and push to Artifactory
-      env:
       run: |
         rm -rf "$TEMP_PATH" && mkdir -p "$REPO_COPY"
         cp -r "$GITHUB_WORKSPACE" "$REPO_COPY"

docker/test/style/Dockerfile

@@ -11,6 +11,7 @@ RUN apt-get update && env DEBIAN_FRONTEND=noninteractive apt-get install --yes \
     curl \
     git \
     libxml2-utils \
+    moreutils \
     pylint \
     python3-pip \
     shellcheck \

docker/test/style/process_style_check_result.py

@@ -10,72 +10,26 @@ def process_result(result_folder):
     status = "success"
     description = ""
     test_results = []
+    checks = (
+        ("header duplicates", "duplicate_output.txt"),
+        ("shellcheck", "shellcheck_output.txt"),
+        ("style", "style_output.txt"),
+        ("typos", "typos_output.txt"),
+        ("whitespaces", "whitespaces_output.txt"),
+        ("workflows", "workflows_output.txt"),
+    )
 
-    duplicate_log_path = "{}/duplicate_output.txt".format(result_folder)
-    if not os.path.exists(duplicate_log_path):
-        logging.info("No header duplicates check log on path %s", duplicate_log_path)
-        return "exception", "No header duplicates check log", []
-    elif os.stat(duplicate_log_path).st_size != 0:
-        description += " Header duplicates check failed. "
-        test_results.append(("Header duplicates check", "FAIL"))
+    for name, out_file in checks:
+        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", []
+        elif os.stat(full_path).st_size != 0:
+            description += f"Check {name} failed. "
+            test_results.append((f"Check {name}", "FAIL"))
             status = "failure"
         else:
-        test_results.append(("Header duplicates check", "OK"))
-
-    shellcheck_log_path = "{}/shellcheck_output.txt".format(result_folder)
-    if not os.path.exists(shellcheck_log_path):
-        logging.info("No shellcheck  log on path %s", shellcheck_log_path)
-        return "exception", "No shellcheck log", []
-    elif os.stat(shellcheck_log_path).st_size != 0:
-        description += " Shellcheck check failed. "
-        test_results.append(("Shellcheck ", "FAIL"))
-        status = "failure"
-    else:
-        test_results.append(("Shellcheck", "OK"))
-
-    style_log_path = "{}/style_output.txt".format(result_folder)
-    if not os.path.exists(style_log_path):
-        logging.info("No style check log on path %s", style_log_path)
-        return "exception", "No style check log", []
-    elif os.stat(style_log_path).st_size != 0:
-        description += "Style check failed. "
-        test_results.append(("Style check", "FAIL"))
-        status = "failure"
-    else:
-        test_results.append(("Style check", "OK"))
-
-    typos_log_path = "{}/typos_output.txt".format(result_folder)
-    if not os.path.exists(typos_log_path):
-        logging.info("No typos check log on path %s", typos_log_path)
-        return "exception", "No typos check log", []
-    elif os.stat(typos_log_path).st_size != 0:
-        description += "Typos check failed. "
-        test_results.append(("Typos check", "FAIL"))
-        status = "failure"
-    else:
-        test_results.append(("Typos check", "OK"))
-
-    whitespaces_log_path = "{}/whitespaces_output.txt".format(result_folder)
-    if not os.path.exists(whitespaces_log_path):
-        logging.info("No whitespaces check log on path %s", whitespaces_log_path)
-        return "exception", "No whitespaces check log", []
-    elif os.stat(whitespaces_log_path).st_size != 0:
-        description += "Whitespaces check failed. "
-        test_results.append(("Whitespaces check", "FAIL"))
-        status = "failure"
-    else:
-        test_results.append(("Whitespaces check", "OK"))
-
-    workflows_log_path = "{}/workflows_output.txt".format(result_folder)
-    if not os.path.exists(workflows_log_path):
-        logging.info("No workflows check log on path %s", style_log_path)
-        return "exception", "No workflows check log", []
-    elif os.stat(whitespaces_log_path).st_size != 0:
-        description += "Workflows check failed. "
-        test_results.append(("Workflows check", "FAIL"))
-        status = "failure"
-    else:
-        test_results.append(("Workflows check", "OK"))
+            test_results.append((f"Check {name}", "OK"))
 
     if not description:
         description += "Style check success"

docker/test/style/run.sh

@@ -3,10 +3,16 @@
 # yaml check is not the best one
 
 cd /ClickHouse/utils/check-style || echo -e "failure\tRepo not found" > /test_output/check_status.tsv
+echo "Check duplicates" | ts
 ./check-duplicate-includes.sh |& tee /test_output/duplicate_output.txt
+echo "Check style" | ts
 ./check-style -n              |& tee /test_output/style_output.txt
+echo "Check typos" | ts
 ./check-typos                 |& tee /test_output/typos_output.txt
+echo "Check whitespaces" | ts
 ./check-whitespaces -n        |& tee /test_output/whitespaces_output.txt
+echo "Check sorkflows" | ts
 ./check-workflows             |& tee /test_output/workflows_output.txt
+echo "Check shell scripts with shellcheck" | ts
 ./shellcheck-run.sh           |& tee /test_output/shellcheck_output.txt
 /process_style_check_result.py || echo -e "failure\tCannot parse results" > /test_output/check_status.tsv

tests/ci/style_check.py

@@ -86,12 +86,18 @@ if __name__ == "__main__":
     docker_image = get_image_with_version(temp_path, "clickhouse/style-test")
     s3_helper = S3Helper("https://s3.amazonaws.com")
 
-    subprocess.check_output(
+    cmd = (
         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"{docker_image}",
+        f"{docker_image}"
+    )
+
+    logging.info("Is going to run the command: %s", cmd)
+    subprocess.check_call(
+        cmd,
         shell=True,
     )
+
     state, description, test_results, additional_files = process_result(temp_path)
     ch_helper = ClickHouseHelper()
     mark_flaky_tests(ch_helper, NAME, test_results)

utils/check-style/check-workflows

@@ -1,5 +1,6 @@
 #!/usr/bin/env bash
 
-act --list 1>/dev/null 2>&1 || act --list 2>&1
+GIT_ROOT=$(git rev-parse --show-cdup)
+act --list --directory="$GIT_ROOT" 1>/dev/null 2>&1 || act --list --directory="$GIT_ROOT" 2>&1
 
 actionlint