-
Notifications
You must be signed in to change notification settings - Fork 5
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add
pr_changed_files
script to detect changed files in a PR (#148)
* Add `pr_changed_files` script to detect if files changed in a PR * Fix shellsheck warning * Update `CHANGELOG.md` * DRY error parameter validation message * Apply suggestions from code review Co-authored-by: Olivier Halligon <olivier.halligon@automattic.com> * DRY arguments parsing code Co-authored-by: Olivier Halligon <olivier.halligon@automattic.com> * Fix mode parsing * Add tests for `pr_changed_files` * Improve handling of filenames with spaces / multiple files in `pr_changed_file` * Improve shellcheck comment * Fix for running tests on CI * Update documented examples Co-authored-by: Olivier Halligon <olivier.halligon@automattic.com> * Update to get list of changed files as an array * Fix code to unescape file names * Rename arguments `--includes` to `--any-match` and `--only-in` to `--all-match` * Add `git fetch origin` to make sure the branch is fetched in the agent * Extract file pattern match to a function * Add more examples and details in the command documentation * Improve file pattern arguments handling & tests * Update documentation (suggestions from code review) Co-authored-by: Olivier Halligon <olivier.halligon@automattic.com> * Update bin/pr_changed_files Co-authored-by: Olivier Halligon <olivier.halligon@automattic.com> * Apply suggestions from code review Co-authored-by: Olivier Halligon <olivier.halligon@automattic.com> * Consistently use the order 1. "all-match" 2. "any-match" * Print result for succeeded tests and improve empty states tests * Remove post-process of the quotes and escaped characters in the files in the diff Co-authored-by: Olivier Halligon <olivier.halligon@automattic.com> * Fix escaping in all_match_patterns tests * Improve tests with special characters Using single quotes to make it easier to understand the use of special characters—including in the code creating the test files * Use single quotes on other tests too For consistency and to make it easier to understand the special characters we are playing with without having to escape them in double-quote contexts --------- Co-authored-by: Olivier Halligon <olivier.halligon@automattic.com>
- Loading branch information
1 parent
7cc4099
commit 79cfdf1
Showing
8 changed files
with
477 additions
and
1 deletion.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,141 @@ | ||
#!/bin/bash -eu | ||
|
||
# This script checks if files are changed in a PR. | ||
# | ||
# Usage: | ||
# pr_changed_files # Check if any files changed | ||
# pr_changed_files --all-match <file-patterns...> # Check if changes are limited to given patterns | ||
# pr_changed_files --any-match <file-patterns...> # Check if changes include files matching patterns | ||
# | ||
# Behavior: | ||
# With no arguments: | ||
# Returns "true" if the PR contains any changes, "false" otherwise | ||
# | ||
# With --all-match: | ||
# Returns "true" if ALL changed files match AT LEAST ONE of the patterns | ||
# Returns "false" if ANY changed file doesn't match ANY pattern | ||
# Note: Will return "true" even if not all patterns are matched by the changed files. | ||
# This mode is especially useful to check if the PR _only_ touches a particular subset of files/folders (but nothing else) | ||
# | ||
# With --any-match: | ||
# Returns "true" if ANY changed file matches ANY of the patterns | ||
# Returns "false" if NONE of the changed files match ANY pattern | ||
# Note: Will return "true" even if the PR includes other files not matching the patterns. | ||
# This mode is especially useful to check if the PR _includes_ (aka _contains at least_) particular files/folders | ||
# | ||
# Examples with expected outputs: | ||
# # Check if any files changed, returning "true" if PR has changes, "false" otherwise | ||
# $ pr_changed_files | ||
# | ||
# # Check if only documentation files changed (to skip UI tests for example) | ||
# $ pr_changed_files --all-match "*.md" "docs/*" | ||
# → "true" if PR only changes `docs/guide.md` and `README.md` | ||
# → "true" if PR only changes `docs/image.png` (not all patterns need to match, ok if no *.md) | ||
# → "false" if PR changes `docs/guide.md` and `src/main.swift` (ALL files need to match at least one pattern) | ||
# | ||
# # Check if any Swift files changed (to decide if we should run SwiftLint) | ||
# $ pr_changed_files --any-match "*.swift" ".swiftlint.yml" | ||
# → "true" if PR changes `src/main.swift` and `README.md` (AT LEAST one file matches one of the patterns) | ||
# → "true" if PR changes `.swiftlint.yml` | ||
# → "false" if PR only changes `README.md` (none of files match any of the patterns) | ||
# | ||
# Returns: | ||
# Prints "true" if the condition is met, "false" otherwise | ||
# Exits with code 0 regardless of if the condition was met or not, to allow easy variable assignment. | ||
# Only exits with non-zero if the command invocation itself was incorrect (called outside of a PR context, incorrect arguments…) | ||
|
||
if [[ ! "${BUILDKITE_PULL_REQUEST:-invalid}" =~ ^[0-9]+$ ]]; then | ||
echo "Error: this tool can only be called from a Buildkite PR job" >&2 | ||
exit 1 | ||
fi | ||
|
||
# Ensure we have the base branch locally | ||
git fetch origin "$BUILDKITE_PULL_REQUEST_BASE_BRANCH" >/dev/null 2>&1 || { | ||
echo "Error: failed to fetch base branch '$BUILDKITE_PULL_REQUEST_BASE_BRANCH'" >&2 | ||
exit 1 | ||
} | ||
|
||
mode="" | ||
patterns=() | ||
|
||
# Define error message for mutually exclusive options | ||
EXCLUSIVE_OPTIONS_ERROR="Error: either specify --all-match or --any-match; cannot specify both" | ||
|
||
while [[ "$#" -gt 0 ]]; do | ||
case $1 in | ||
--all-match | --any-match) | ||
if [[ -n "$mode" ]]; then | ||
echo "$EXCLUSIVE_OPTIONS_ERROR" >&2 | ||
exit 1 | ||
fi | ||
mode="${1#--}" | ||
shift | ||
# Check if there are any patterns after the flag | ||
while [[ "$#" -gt 0 && "$1" != "--"* ]]; do | ||
patterns+=("$1") | ||
shift | ||
done | ||
if [[ "${#patterns[@]}" -eq 0 ]]; then | ||
echo "Error: must specify at least one file pattern" >&2 | ||
exit 1 | ||
fi | ||
;; | ||
--*) | ||
echo "Error: unknown option $1" >&2 | ||
exit 1 | ||
;; | ||
*) | ||
echo "Error: unexpected argument $1" >&2 | ||
exit 1 | ||
;; | ||
esac | ||
done | ||
|
||
# Get list of changed files as an array | ||
changed_files=() | ||
while IFS= read -r -d '' file; do | ||
changed_files+=("$file") | ||
done < <(git --no-pager diff --name-only -z --merge-base "$BUILDKITE_PULL_REQUEST_BASE_BRANCH" HEAD | sort) | ||
|
||
if [[ -z "$mode" ]]; then | ||
# No arguments = any change | ||
if [[ ${#changed_files[@]} -gt 0 ]]; then | ||
echo "true" | ||
else | ||
echo "false" | ||
fi | ||
exit 0 | ||
fi | ||
|
||
# Returns 0 if the file matches any of the patterns, 1 otherwise | ||
file_matches_any_pattern() { | ||
local file="$1" | ||
shift | ||
for pattern in "$@"; do | ||
# shellcheck disable=SC2053 # We don't quote the rhs in the condition on the next line because we want to interpret pattern as a glob pattern | ||
if [[ "$file" == ${pattern} ]]; then | ||
return 0 | ||
fi | ||
done | ||
return 1 | ||
} | ||
|
||
if [[ "$mode" == "all-match" ]]; then | ||
# Check if all changed files match at least one pattern | ||
for file in "${changed_files[@]}"; do | ||
if ! file_matches_any_pattern "$file" "${patterns[@]}"; then | ||
echo "false" | ||
exit 0 | ||
fi | ||
done | ||
echo "true" | ||
elif [[ "$mode" == "any-match" ]]; then | ||
# Check if any changed file matches any pattern | ||
for file in "${changed_files[@]}"; do | ||
if file_matches_any_pattern "$file" "${patterns[@]}"; then | ||
echo "true" | ||
exit 0 | ||
fi | ||
done | ||
echo "false" | ||
fi |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,59 @@ | ||
#!/bin/bash -eu | ||
|
||
set -o pipefail | ||
|
||
source "$(dirname "${BASH_SOURCE[0]}")/test_helpers.sh" | ||
|
||
echo "--- :git: Testing all-match pattern matching" | ||
|
||
# Create test repository | ||
repo_path=$(create_tmp_repo_dir) | ||
trap 'cleanup_git_repo "$repo_path"' EXIT | ||
|
||
# Set up environment variables | ||
export BUILDKITE_PULL_REQUEST="123" | ||
export BUILDKITE_PULL_REQUEST_BASE_BRANCH="base" | ||
|
||
# Initialize the repository | ||
init_test_repo "$repo_path" | ||
|
||
# Create test files (using single quotes to avoid special chars being interpreted by the shell) | ||
mkdir -p docs src/swift | ||
echo "doc1" > 'docs/read me.md' | ||
echo "doc2" > 'docs/guide with spaces.md' | ||
echo "doc3" > 'docs/special\!@*#$chars.md' | ||
git add . | ||
git commit -m "Add doc files" | ||
|
||
# [Test] All changes in docs | ||
result=$(pr_changed_files --all-match "docs/*") | ||
assert_output "true" "$result" "Should return true when all changes match patterns" | ||
|
||
# [Test] All changes in docs with explicit patterns including spaces and special chars | ||
# Note: we need to escape the '\` and `*` special chars in the pattern to match them literally instead of as special characters | ||
result=$(pr_changed_files --all-match 'docs/read me.md' 'docs/guide with spaces.md' 'docs/special\\!@\*#$chars.md') | ||
assert_output "true" "$result" "Should return true when all changes match patterns with spaces and special chars" | ||
|
||
# [Test] All changes in docs with globbing patterns including spaces and special chars | ||
result=$(pr_changed_files --all-match 'docs/read me.md' 'docs/guide with spaces.md' 'docs/special\\!*.md') | ||
assert_output "true" "$result" "Should return true when all changes match patterns with spaces and special chars, even when using globbing" | ||
|
||
# [Test] Changes outside pattern | ||
echo "swift" > 'src/swift/main with spaces.swift' | ||
echo "swift" > 'src/swift/special!\@#*$chars.swift' | ||
git add . | ||
git commit -m "Add swift file" | ||
|
||
result=$(pr_changed_files --all-match "docs/*") | ||
assert_output "false" "$result" "Should return false when changes exist outside patterns" | ||
|
||
# [Test] Multiple patterns, all matching | ||
# Note: we need to escape the '\` and `*` special chars in the pattern to match them literally instead of as special characters | ||
result=$(pr_changed_files --all-match 'docs/*' 'src/swift/main with spaces.swift' 'src/swift/special\!\\@#\*$chars.swift') | ||
assert_output "true" "$result" "Should return true when all changes match multiple patterns" | ||
|
||
# [Test] Multiple patterns, all matching, including some using globbing | ||
result=$(pr_changed_files --all-match 'docs/*' 'src/swift/main with spaces.swift' 'src/swift/special*chars.swift') | ||
assert_output "true" "$result" "Should return true when all changes match multiple patterns, including some using globbing" | ||
|
||
echo "✅ All-match pattern tests passed" |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
#!/bin/bash -eu | ||
|
||
set -o pipefail | ||
|
||
source "$(dirname "${BASH_SOURCE[0]}")/test_helpers.sh" | ||
|
||
echo "--- :git: Testing any-match pattern matching" | ||
|
||
# Create test repository | ||
repo_path=$(create_tmp_repo_dir) | ||
trap 'cleanup_git_repo "$repo_path"' EXIT | ||
|
||
# Set up environment variables | ||
export BUILDKITE_PULL_REQUEST="123" | ||
export BUILDKITE_PULL_REQUEST_BASE_BRANCH="base" | ||
|
||
# Initialize the repository | ||
init_test_repo "$repo_path" | ||
|
||
# Create test files (using single quotes to avoid special chars being interpreted by the shell) | ||
mkdir -p docs src/swift src/ruby | ||
echo "doc" > 'docs/read me.md' | ||
echo "doc" > 'docs/special!@*#$chars.md' | ||
echo "swift" > 'src/swift/main.swift' | ||
echo "ruby" > 'src/ruby/main.rb' | ||
git add . | ||
git commit -m "Add test files" | ||
|
||
# [Test] Match specific extension | ||
result=$(pr_changed_files --any-match '*.swift') | ||
assert_output "true" "$result" "Should match .swift files" | ||
|
||
# [Test] Match multiple patterns | ||
result=$(pr_changed_files --any-match 'docs/*.md' '*.rb') | ||
assert_output "true" "$result" "Should match multiple patterns" | ||
|
||
# [Test] Match files with spaces and special characters | ||
result=$(pr_changed_files --any-match 'docs/read me.md' 'docs/special!@\*#$chars.md') | ||
assert_output "true" "$result" "Should match files with spaces and special characters" | ||
|
||
# [Test] Match files with spaces and special characters, even when using globbing | ||
result=$(pr_changed_files --any-match 'docs/read me.md' 'docs/special*chars.md') | ||
assert_output "true" "$result" "Should match files with spaces and special characters, even when using globbing" | ||
|
||
# [Test] No matches | ||
result=$(pr_changed_files --any-match '*.js') | ||
assert_output "false" "$result" "Should not match non-existent patterns" | ||
|
||
# [Test] Directory pattern | ||
result=$(pr_changed_files --any-match 'docs/*') | ||
assert_output "true" "$result" "Should match directory patterns" | ||
|
||
# [Test] Exact pattern matching | ||
echo "swiftfile" > swiftfile.txt | ||
git add swiftfile.txt | ||
git commit -m "Add file with swift in name" | ||
|
||
result=$(pr_changed_files --any-match '*.swift') | ||
assert_output "true" "$result" "Should only match exact patterns" | ||
|
||
echo "✅ Any-match pattern tests passed" |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
#!/bin/bash -eu | ||
|
||
set -o pipefail | ||
|
||
source "$(dirname "${BASH_SOURCE[0]}")/test_helpers.sh" | ||
|
||
echo "--- :git: Testing basic changes detection" | ||
|
||
# Create test repository | ||
repo_path=$(create_tmp_repo_dir) | ||
trap 'cleanup_git_repo "$repo_path"' EXIT | ||
|
||
# Set up environment variables | ||
export BUILDKITE_PULL_REQUEST="123" | ||
export BUILDKITE_PULL_REQUEST_BASE_BRANCH="base" | ||
|
||
# Initialize the repository | ||
init_test_repo "$repo_path" | ||
|
||
# [Test] No changes | ||
result=$(pr_changed_files) | ||
assert_output "false" "$result" "Should return false when no files changed" | ||
|
||
# [Test] Single file change | ||
echo "change" > new.txt | ||
git add new.txt | ||
git commit -m "Add new file" | ||
|
||
result=$(pr_changed_files) | ||
assert_output "true" "$result" "Should return true when files changed" | ||
|
||
echo "✅ Basic changes tests passed" |
Oops, something went wrong.