From 947988ed87a70115a7bb19cdfd9f7d2052650971 Mon Sep 17 00:00:00 2001 From: Michael Weiner Date: Tue, 2 Sep 2025 19:51:43 -0500 Subject: [PATCH 1/5] First pass at script refactor --- scripts/pr_tests.py | 71 ++++++++++++++++++++++++++++----------------- 1 file changed, 44 insertions(+), 27 deletions(-) diff --git a/scripts/pr_tests.py b/scripts/pr_tests.py index 14f637bff4c3..6e13a233fad3 100644 --- a/scripts/pr_tests.py +++ b/scripts/pr_tests.py @@ -72,37 +72,54 @@ def get_script_dependencies_graph(path_prefix, file_suffix="test.sh"): test_paths = set() - commits = pr.get_commits() - for commit in commits: - files = commit.files - for file in files: - filename = file.filename - if filename.startswith(doc_file_prefix): - if filename.endswith("test.sh") or filename.endswith("/snips.sh"): - relative_file = filename[len(doc_file_prefix):] - test_paths.add(relative_file.rsplit('/', 1)[0]) - # Tentatively add tests that externally imported other module files - checked = set() - to_check = {os.path.dirname(filename)} - while to_check: - el = to_check.pop() - checked.add(el) - if el in script_dependencies: - to_add = script_dependencies[el] - checked - to_check.update(to_add) - test_paths.update(map(lambda file_path: file_path[len(doc_file_prefix):], to_add)) - elif filename == istio_go_dependency or \ - filename.startswith(test_framework_pkg) or \ - filename.startswith(test_framework_util) or \ - filename.startswith(prow_dir) or \ - filename.startswith(boilerplate_snip_prefix): - print("ALL") - sys.exit(0) + # NOTE(mweiner): View [1] for schema of GitHub's API to list files in a PR. + # [1] https://docs.github.com/en/rest/pulls/pulls#list-pull-requests-files + + for file in pr.get_files(): + # Check for short-circuits up-front for efficiency. + if ( + file.filename == istio_go_dependency or + file.filename.startswith(test_framework_pkg) or + file.filename.startswith(test_framework_util) or + file.filename.startswith(prow_dir) or + file.filename.startswith(boilerplate_snip_prefix) + ): + print("ALL") + sys.exit(0) + + # Files being removed no longer require testing, so they will be skipped. + if file.status == "removed" or file.status == "unchanged": + continue + + # At this point, only files with a status of "added", "modified", + # "renamed", "copied", or "changed" will be analyzed. + + # No need to consider files outside of the doc_file_prefix. Skip them. + if not file.filename.startswith(doc_file_prefix): + continue + + # Of the files that remain, only those with certain naming conventions + # require testing. + if file.filename.endswith("test.sh") or file.filename.endswith("/snips.sh"): + relative_file = file.filename[len(doc_file_prefix):] + test_paths.add(relative_file.rsplit('/', 1)[0]) + + # Tentatively add tests that externally imported other module files + checked = set() + to_check = {os.path.dirname(file.filename)} + while to_check: + el = to_check.pop() + checked.add(el) + if el in script_dependencies: + to_add = script_dependencies[el] - checked + to_check.update(to_add) + # keep paths relative to doc_file_prefix + test_paths.update(map(lambda p: p[len(doc_file_prefix):], to_add)) if len(test_paths) == 0: print("NONE") else: - print(*test_paths, sep=",") + print(*sorted(test_paths), sep=",") except Exception as e: # fall back to running all tests if anything goes wrong (e.g., rate-limiting) From 477eee2260b0805156f001ba196e8997002db468 Mon Sep 17 00:00:00 2001 From: Michael Weiner Date: Tue, 2 Sep 2025 19:57:35 -0500 Subject: [PATCH 2/5] Remove name in note --- scripts/pr_tests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/pr_tests.py b/scripts/pr_tests.py index 6e13a233fad3..c6263676348f 100644 --- a/scripts/pr_tests.py +++ b/scripts/pr_tests.py @@ -72,7 +72,7 @@ def get_script_dependencies_graph(path_prefix, file_suffix="test.sh"): test_paths = set() - # NOTE(mweiner): View [1] for schema of GitHub's API to list files in a PR. + # NOTE: View [1] for schema of GitHub's API to list files in a PR. # [1] https://docs.github.com/en/rest/pulls/pulls#list-pull-requests-files for file in pr.get_files(): From 2c9959ce6398c23798b652a270ea2e2f3c81e1dd Mon Sep 17 00:00:00 2001 From: Michael Weiner Date: Tue, 2 Sep 2025 19:58:59 -0500 Subject: [PATCH 3/5] Typo fixes --- scripts/pr_tests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/pr_tests.py b/scripts/pr_tests.py index c6263676348f..2a8a51305555 100644 --- a/scripts/pr_tests.py +++ b/scripts/pr_tests.py @@ -87,7 +87,7 @@ def get_script_dependencies_graph(path_prefix, file_suffix="test.sh"): print("ALL") sys.exit(0) - # Files being removed no longer require testing, so they will be skipped. + # Files that are being removed or that are unchanged don't require tests. Skip them. if file.status == "removed" or file.status == "unchanged": continue From 9a5ca1acef7dd692d892ae30ae223fd2980cd84e Mon Sep 17 00:00:00 2001 From: Michael Weiner Date: Tue, 2 Sep 2025 20:09:04 -0500 Subject: [PATCH 4/5] Resolve linter complaining about extra EOL space --- scripts/pr_tests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/pr_tests.py b/scripts/pr_tests.py index 2a8a51305555..5039bea8e208 100644 --- a/scripts/pr_tests.py +++ b/scripts/pr_tests.py @@ -87,7 +87,7 @@ def get_script_dependencies_graph(path_prefix, file_suffix="test.sh"): print("ALL") sys.exit(0) - # Files that are being removed or that are unchanged don't require tests. Skip them. + # Files that are being removed or that are unchanged don't require tests. Skip them if file.status == "removed" or file.status == "unchanged": continue From 1abb08c01b6b873434ab3c90e2bbe1198e7d8e02 Mon Sep 17 00:00:00 2001 From: Michael Weiner Date: Tue, 2 Sep 2025 20:16:45 -0500 Subject: [PATCH 5/5] Actually fix the right line...sigh --- scripts/pr_tests.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/pr_tests.py b/scripts/pr_tests.py index 5039bea8e208..54b9836e11b4 100644 --- a/scripts/pr_tests.py +++ b/scripts/pr_tests.py @@ -87,11 +87,11 @@ def get_script_dependencies_graph(path_prefix, file_suffix="test.sh"): print("ALL") sys.exit(0) - # Files that are being removed or that are unchanged don't require tests. Skip them + # Files that are being removed or that are unchanged don't require tests. Skip them. if file.status == "removed" or file.status == "unchanged": continue - # At this point, only files with a status of "added", "modified", + # At this point, only files with a status of "added", "modified", # "renamed", "copied", or "changed" will be analyzed. # No need to consider files outside of the doc_file_prefix. Skip them.