Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding python quality checks to CMSSW (https://github.com/cms-sw/cms-bot/pull/2319/ follow-up) #2378

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 54 additions & 0 deletions PFA.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
#!/usr/bin/python3
import os
import argparse


def find_file_path(file):
if not os.path.isfile(file):
return None
return os.path.realpath(file)


def find_python_files(directory):
py_files = []
for root, dirs, files in os.walk(directory):
for file in files:
if file.endswith(".py"):
py_files.append(os.path.join(root, file))
return py_files


def CodeQualityChecks(python_files):
if python_files:
for file in python_files:
file_path = find_file_path(file)
if not file_path:
print("File {} not found or invalid path.".format(file))
continue

format_command = "ruff format {}".format(file_path)
print("Executing command: {}".format(format_command)) # Debug print
os.system(format_command)


def main():
parser = argparse.ArgumentParser(
description="Linting and formatting Python files in directories or file paths."
)
parser.add_argument("paths", nargs="+", help="List of directories or file paths to process")
args = parser.parse_args()

all_python_files = []
for path in args.paths:
if os.path.isdir(path):
all_python_files.extend(find_python_files(path))
elif os.path.isfile(path):
all_python_files.append(path)
else:
print("Error: {} is not a valid file or directory.".format(path))
return
CodeQualityChecks(all_python_files)


if __name__ == "__main__":
main()
64 changes: 64 additions & 0 deletions run-pr-code-checks
100755 → 100644
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ if $CODE_CHECKS ; then
if [ -s ${UP_DIR}/code-checks-files.txt ] ; then
ERR=false
scram build -k -j $NUM_PROC code-checks USER_CODE_CHECKS_FILE="${UP_DIR}/code-checks-files.txt" > ${UP_DIR}/code-checks.log 2>&1 || ERR=true

if $ERR ; then
echo '-code-checks' > ${UP_DIR}/code-checks.md
echo -e "\nLogs: $UP_URL" >> ${UP_DIR}/code-checks.md
Expand Down Expand Up @@ -143,6 +144,7 @@ if $CODE_FORMAT ; then
scram build -k -j $NUM_PROC code-format USER_CODE_FORMAT_FILE="${UP_DIR}/code-format-files.txt" > ${UP_DIR}/code-format.log 2>&1
pushd $CMSSW_BASE/src
git diff > ${UP_DIR}/code-format.patch
cat ${UP_DIR}/code-format.patch
if [ -s ${UP_DIR}/code-format.patch ] ; then
git commit -a -m 'auto applied code-formats changes'
let COMMIT_CHG=${COMMIT_CHG}+1
Expand All @@ -152,6 +154,20 @@ if $CODE_FORMAT ; then
fi
fi

echo "We will now run formatting, and then linting checks"

python3 ../cms-bot/run-python-format.py --inputfile "${UP_DIR}/code-checks-files.txt" --cmsswbase "${CMSSW_BASE}/src"

pushd $CMSSW_BASE/src
git diff > ${UP_DIR}/python-code-format.patch || true
popd

cp "python-linting.txt" "${UP_DIR}/python-linting.txt" || true

if [ ! -f "python-linting.txt" ]; then
echo "Error: python-linting.txt was not created."
fi

if ${MULTIPLE_FILES_CHANGES} ; then
${CMS_BOT_DIR}/github_scripts/simultaneous_files_modifications_by_PRs.py ${PULL_REQUEST} > \
${UP_DIR}/multiple_files_changes.txt || true
Expand All @@ -176,6 +192,7 @@ if [ ${COMMIT_CHG} -gt 0 ] ; then
popd
exit 0
fi

RES="-code-checks"
HOW_TO_RUN="\n\nCode check has found code style and quality issues which could be resolved by applying following patch(s)"
if [ -s ${UP_DIR}/code-checks.patch ] ; then
Expand All @@ -184,14 +201,24 @@ if [ ${COMMIT_CHG} -gt 0 ] ; then
HOW_TO_RUN="${HOW_TO_RUN}\ne.g. \`curl -k https://cmssdt.cern.ch/SDT/${JENKINS_PREFIX_STR}code-checks/${REPO_USER}-PR-${PULL_REQUEST}/${BUILD_NUMBER}/code-checks.patch | patch -p1\`"
HOW_TO_RUN="${HOW_TO_RUN}\nYou can also run \`scram build code-checks\` to apply code checks directly"
fi

if [ -s ${UP_DIR}/code-format.patch ] ; then
HOW_TO_RUN="${HOW_TO_RUN}\n\n- **code-format**:"
HOW_TO_RUN="${HOW_TO_RUN}\nhttps://cmssdt.cern.ch/SDT/${JENKINS_PREFIX_STR}code-checks/${REPO_USER}-PR-${PULL_REQUEST}/${BUILD_NUMBER}/code-format.patch"
HOW_TO_RUN="${HOW_TO_RUN}\ne.g. \`curl -k https://cmssdt.cern.ch/SDT/${JENKINS_PREFIX_STR}code-checks/${REPO_USER}-PR-${PULL_REQUEST}/${BUILD_NUMBER}/code-format.patch | patch -p1\`"
HOW_TO_RUN="${HOW_TO_RUN}\nYou can also run \`scram build code-format\` to apply code format directly"
fi

if [ -s ${UP_DIR}/python-code-format.patch ] ; then
HOW_TO_RUN="${HOW_TO_RUN}\n\n- **python-code-format**:"
HOW_TO_RUN="${HOW_TO_RUN}\nhttps://cmssdt.cern.ch/SDT/${JENKINS_PREFIX_STR}code-checks/${REPO_USER}-PR-${PULL_REQUEST}/${BUILD_NUMBER}/python-code-format.patch"
HOW_TO_RUN="${HOW_TO_RUN}\ne.g. \`curl -k https://cmssdt.cern.ch/SDT/${JENKINS_PREFIX_STR}code-checks/${REPO_USER}-PR-${PULL_REQUEST}/${BUILD_NUMBER}/python-code-format.patch | patch -p1\`"
HOW_TO_RUN="${HOW_TO_RUN}\nYou can also run \`python3 PFA.py <file> or python3 run-python-format.py --inputfile <code-checks-files.txt> --cmsswbase <basepath>\` to apply Python code formatting directly"
fi
fi



MSG="\n\nLogs: $UP_URL"
if [ -s ${UP_DIR}/invalid-filenames.txt ] ; then
RES="-code-checks"
Expand All @@ -217,3 +244,40 @@ if ! $DRY_RUN ; then
eval `scram unset -sh`
${CMS_BOT_DIR}/comment-gh-pr.py -r ${REPOSITORY} -p $PULL_REQUEST -R ${UP_DIR}/code-checks.md
fi

if [ -s ${UP_DIR}/python-code-format.patch ] ; then
echo "INFO: We're applying PEP8 formatting to CMSSW using Ruff; while Ruff found formatting errors in the PR's Python files, this non-blocking check may flag untouched lines, so feel free to contribute by applying the suggested patch." >> ${UP_DIR}/python-code-checks.md

echo -e "\n\n- **python-code-format**:" >> ${UP_DIR}/python-code-checks.md
echo -e "https://cmssdt.cern.ch/SDT/${JENKINS_PREFIX_STR}code-checks/${REPO_USER}-PR-${PULL_REQUEST}/${BUILD_NUMBER}/python-code-format.patch" >> ${UP_DIR}/python-code-checks.md
echo -e "e.g. \`curl -k https://cmssdt.cern.ch/SDT/${JENKINS_PREFIX_STR}code-checks/${REPO_USER}-PR-${PULL_REQUEST}/${BUILD_NUMBER}/python-code-format.patch | patch -p1\`" >> ${UP_DIR}/python-code-checks.md
echo -e "You can also run \`python3 run-python-format.py --inputfile <code-checks-files.txt> --cmsswbase <basepath>\` to apply Python code formatting directly" >> ${UP_DIR}/python-code-checks.md

echo -e "\n\n- **python-code-linting**:" >> ${UP_DIR}/python-code-checks.md
file="python-linting.txt"
content=$(<"$file")
if [ "$content" == "All checks passed!" ]; then
echo -e "Status: All linting checks passed! \xF0\x9F\x98\x8A " >> ${UP_DIR}/python-code-checks.md
else
echo -e "Status: There are some linting recommendations that need your attention. \xF0\x9F\x98\xA2" >> ${UP_DIR}/python-code-checks.md
echo -e "https://cmssdt.cern.ch/SDT/${JENKINS_PREFIX_STR}code-checks/${REPO_USER}-PR-${PULL_REQUEST}/${BUILD_NUMBER}/python-linting.txt" >> ${UP_DIR}/python-code-checks.md
echo -e "You can also run \`ruff check --fix\` to automatically apply fixes." >> ${UP_DIR}/python-code-checks.md
fi
if ! $DRY_RUN ; then
${CMS_BOT_DIR}/comment-gh-pr.py -r ${REPOSITORY} -p $PULL_REQUEST -R ${UP_DIR}/python-code-checks.md
fi
else
echo "INFO: We're applying PEP8 formatting to CMSSW using Ruff; while Ruff found formatting errors in the PR's Python files, this non-blocking check may flag untouched lines, so feel free to contribute by applying the suggested patch." >> ${UP_DIR}/python-code-checks.md
echo -e "\n\n- **python-code-linting**:" >> ${UP_DIR}/python-code-checks.md
file="python-linting.txt"
content=$(<"$file")
if [ "$content" == "All checks passed!" ]; then
echo -e "Status: All linting checks passed! \xF0\x9F\x98\x8A " >> ${UP_DIR}/python-code-checks.md
else
echo -e "Status: There are some linting recommendations that need your attention. \xF0\x9F\x98\xA2" >> ${UP_DIR}/python-code-checks.md
echo -e "https://cmssdt.cern.ch/SDT/${JENKINS_PREFIX_STR}code-checks/${REPO_USER}-PR-${PULL_REQUEST}/${BUILD_NUMBER}/python-linting.txt" >> ${UP_DIR}/python-code-checks.md
echo -e "You can also run \`ruff check --fix\` to automatically apply fixes." >> ${UP_DIR}/python-code-checks.md
${CMS_BOT_DIR}/comment-gh-pr.py -r ${REPOSITORY} -p $PULL_REQUEST -R ${UP_DIR}/python-code-checks.md
fi
fi

61 changes: 61 additions & 0 deletions run-python-format.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
#!/usr/bin/python3
import os
import argparse


def main():
parser = argparse.ArgumentParser(description="Run Python formatting and linting.")
parser.add_argument(
"--inputfile",
required=True,
help="Path to the file containing the list of files to process.",
)
parser.add_argument(
"--cmsswbase",
required=True,
help="Path to the CMSSW base directory.",
)
args = parser.parse_args()

input_file = args.inputfile
cmssw_base = args.cmsswbase

if not os.path.isfile(input_file):
print("Error: {} does not exist.".format(input_file))
return
try:
with open(input_file, "r") as file:
files_list = [os.path.join(cmssw_base, line.strip()) for line in file if line.strip()]
except IOError as e:
print("Error reading {}: {}".format(input_file, e))
return
with open("python-linting.txt", "w") as linting_output:
if not files_list:
linting_output.write("No files to check.\n")
print("No files to check. Exiting.")
return

all_checks_passed = True
for file in files_list:
if os.path.isfile(file):
check_command = "ruff check {} >> python-linting.txt".format(file)
result = os.system(check_command)
if result != 0:
all_checks_passed = False

if all_checks_passed:
linting_output.write("All checks passed!\n")

print("Python linting completed. Check 'python-linting.txt' for details.")
pfa_command = "python3 ../cms-bot/PFA.py " + " ".join(files_list)

format_result = os.system(pfa_command)

if format_result == 0:
print("Successfully formatted files.")
else:
print("An error occurred while running PFA.py. Exit code: {}".format(format_result))


if __name__ == "__main__":
main()
Loading