From 76698ea37b31b4eb9910450dabd601bcfa0fdaa7 Mon Sep 17 00:00:00 2001 From: Ian Maia Date: Tue, 4 Feb 2025 15:59:32 +0100 Subject: [PATCH 01/28] Add `pr_changed_files` script to detect if files changed in a PR --- bin/pr_changed_files | 124 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 124 insertions(+) create mode 100755 bin/pr_changed_files diff --git a/bin/pr_changed_files b/bin/pr_changed_files new file mode 100755 index 00000000..8840eb65 --- /dev/null +++ b/bin/pr_changed_files @@ -0,0 +1,124 @@ +#!/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 --includes # Check if changes include files matching patterns +# pr_changed_files --only-in # Check if changes are limited to given patterns +# +# Examples: +# # Check if any files changed +# pr_changed_files +# +# # Check if only documentation files changed +# pr_changed_files --only-in "*.md" "docs/**" +# +# # Check if any Swift files changed +# pr_changed_files --includes "*.swift" +# +# Returns: +# Prints "true" if the condition is met, "false" otherwise +# Always exits with code 0 to allow easy variable assignment + +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 + +mode="" +patterns=() + +while [[ "$#" -gt 0 ]]; do + case $1 in + --only-in) + if [[ -n "$mode" ]]; then + echo "Error: cannot specify both --only-in and --includes" >&2 + exit 1 + fi + mode="only-in" + shift + while [[ "$#" -gt 0 && "$1" != "--"* ]]; do + patterns+=("$1") + shift + done + ;; + --includes) + if [[ -n "$mode" ]]; then + echo "Error: cannot specify both --only-in and --includes" >&2 + exit 1 + fi + mode="includes" + shift + while [[ "$#" -gt 0 && "$1" != "--"* ]]; do + patterns+=("$1") + shift + done + ;; + --*) + echo "Error: unknown option $1" >&2 + exit 1 + ;; + *) + echo "Error: unexpected argument $1" >&2 + exit 1 + ;; + esac +done + +# Get list of changed files +changed_files=$(git --no-pager diff --name-only --merge-base "$BUILDKITE_PULL_REQUEST_BASE_BRANCH" HEAD | sort) + +if [[ -z "$mode" ]]; then + # No arguments = any change + if [[ -n "$changed_files" ]]; then + echo "true" + else + echo "false" + fi + exit 0 +fi + +if [[ ${#patterns[@]} -eq 0 ]]; then + echo "Error: must specify at least one file pattern" >&2 + exit 1 +fi + +# No files changed +if [[ -z "$changed_files" ]]; then + if [[ "$mode" == "only-in" ]]; then + echo "true" + else + echo "false" + fi + exit 0 +fi + +if [[ "$mode" == "only-in" ]]; then + # Check if all changed files match at least one pattern + for file in $changed_files; do + matches_any=false + for pattern in "${patterns[@]}"; do + if [[ "$file" == $pattern ]]; then + matches_any=true + break + fi + done + if [[ "$matches_any" == "false" ]]; then + echo "false" + exit 0 + fi + done + echo "true" +elif [[ "$mode" == "includes" ]]; then + # Check if any changed file matches any pattern + for file in $changed_files; do + for pattern in "${patterns[@]}"; do + if [[ "$file" == $pattern ]]; then + echo "true" + exit 0 + fi + done + done + echo "false" +fi From ea1458346e3d2dc6465a77584bbac00ce4e344a7 Mon Sep 17 00:00:00 2001 From: Ian Maia Date: Tue, 4 Feb 2025 20:42:04 +0100 Subject: [PATCH 02/28] Fix shellsheck warning --- bin/pr_changed_files | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/bin/pr_changed_files b/bin/pr_changed_files index 8840eb65..b4115a93 100755 --- a/bin/pr_changed_files +++ b/bin/pr_changed_files @@ -99,7 +99,8 @@ if [[ "$mode" == "only-in" ]]; then for file in $changed_files; do matches_any=false for pattern in "${patterns[@]}"; do - if [[ "$file" == $pattern ]]; then + # shellcheck disable=SC2053 + if [[ "$file" == $pattern ]]; then # glob match with pattern matches_any=true break fi @@ -114,7 +115,8 @@ elif [[ "$mode" == "includes" ]]; then # Check if any changed file matches any pattern for file in $changed_files; do for pattern in "${patterns[@]}"; do - if [[ "$file" == $pattern ]]; then + # shellcheck disable=SC2053 + if [[ "$file" == $pattern ]]; then # glob match with pattern echo "true" exit 0 fi From 1be6a12f2629b9cfa594c109fb801588ffa52091 Mon Sep 17 00:00:00 2001 From: Ian Maia Date: Tue, 4 Feb 2025 20:43:52 +0100 Subject: [PATCH 03/28] Update `CHANGELOG.md` --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ecd9bd09..26b81de8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,7 +38,7 @@ _None._ ### New Features -_None._ +- Add `pr_changed_files` command to detect changes made in a Pull Request [#148] ### Bug Fixes From b528c2eb02e7c0ef1aff4f7cc64422d20d08297d Mon Sep 17 00:00:00 2001 From: Ian Maia Date: Wed, 5 Feb 2025 11:44:00 +0100 Subject: [PATCH 04/28] DRY error parameter validation message --- bin/pr_changed_files | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/bin/pr_changed_files b/bin/pr_changed_files index b4115a93..38002c67 100755 --- a/bin/pr_changed_files +++ b/bin/pr_changed_files @@ -29,11 +29,14 @@ fi mode="" patterns=() +# Define error message for mutually exclusive options +EXCLUSIVE_OPTIONS_ERROR="Error: either specify --only-in or --includes; cannot specify both" + while [[ "$#" -gt 0 ]]; do case $1 in --only-in) if [[ -n "$mode" ]]; then - echo "Error: cannot specify both --only-in and --includes" >&2 + echo "$EXCLUSIVE_OPTIONS_ERROR" >&2 exit 1 fi mode="only-in" @@ -45,7 +48,7 @@ while [[ "$#" -gt 0 ]]; do ;; --includes) if [[ -n "$mode" ]]; then - echo "Error: cannot specify both --only-in and --includes" >&2 + echo "$EXCLUSIVE_OPTIONS_ERROR" >&2 exit 1 fi mode="includes" From 61741eae476f56944ca424a82d91352c60eca88b Mon Sep 17 00:00:00 2001 From: "Ian G. Maia" Date: Wed, 5 Feb 2025 14:57:18 +0100 Subject: [PATCH 05/28] Apply suggestions from code review Co-authored-by: Olivier Halligon --- bin/pr_changed_files | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/bin/pr_changed_files b/bin/pr_changed_files index 38002c67..949a9e1b 100755 --- a/bin/pr_changed_files +++ b/bin/pr_changed_files @@ -11,15 +11,16 @@ # # Check if any files changed # pr_changed_files # -# # Check if only documentation files changed +# # Check if only documentation files changed. Could be useful to decide to skip UI tests for example # pr_changed_files --only-in "*.md" "docs/**" # -# # Check if any Swift files changed -# pr_changed_files --includes "*.swift" +# # Check if any Swift files changed, e.g. to decide if we should run Swiftlint +# pr_changed_files --includes "*.swift" ".swiftlint.yml" # # Returns: # Prints "true" if the condition is met, "false" otherwise -# Always exits with code 0 to allow easy variable assignment +# 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 @@ -99,7 +100,7 @@ fi if [[ "$mode" == "only-in" ]]; then # Check if all changed files match at least one pattern - for file in $changed_files; do + for file in "$changed_files"; do matches_any=false for pattern in "${patterns[@]}"; do # shellcheck disable=SC2053 From 0790ee23ce113edff9bda00ee2ccc26584b4df4d Mon Sep 17 00:00:00 2001 From: "Ian G. Maia" Date: Wed, 5 Feb 2025 19:02:22 +0100 Subject: [PATCH 06/28] DRY arguments parsing code Co-authored-by: Olivier Halligon --- bin/pr_changed_files | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/bin/pr_changed_files b/bin/pr_changed_files index 949a9e1b..16752222 100755 --- a/bin/pr_changed_files +++ b/bin/pr_changed_files @@ -35,24 +35,12 @@ EXCLUSIVE_OPTIONS_ERROR="Error: either specify --only-in or --includes; cannot s while [[ "$#" -gt 0 ]]; do case $1 in - --only-in) + --only-in | --includes) if [[ -n "$mode" ]]; then echo "$EXCLUSIVE_OPTIONS_ERROR" >&2 exit 1 fi - mode="only-in" - shift - while [[ "$#" -gt 0 && "$1" != "--"* ]]; do - patterns+=("$1") - shift - done - ;; - --includes) - if [[ -n "$mode" ]]; then - echo "$EXCLUSIVE_OPTIONS_ERROR" >&2 - exit 1 - fi - mode="includes" + mode="${1%--}" shift while [[ "$#" -gt 0 && "$1" != "--"* ]]; do patterns+=("$1") From 80894eccdc3a66edd2594dfac3b2362af3f3aa06 Mon Sep 17 00:00:00 2001 From: Ian Maia Date: Wed, 5 Feb 2025 19:23:49 +0100 Subject: [PATCH 07/28] Fix mode parsing --- bin/pr_changed_files | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/pr_changed_files b/bin/pr_changed_files index 16752222..b15138ab 100755 --- a/bin/pr_changed_files +++ b/bin/pr_changed_files @@ -40,7 +40,7 @@ while [[ "$#" -gt 0 ]]; do echo "$EXCLUSIVE_OPTIONS_ERROR" >&2 exit 1 fi - mode="${1%--}" + mode="${1#--}" shift while [[ "$#" -gt 0 && "$1" != "--"* ]]; do patterns+=("$1") From d7e91732333131ba937d3ffab05900c7fdac334d Mon Sep 17 00:00:00 2001 From: Ian Maia Date: Wed, 5 Feb 2025 14:35:25 +0100 Subject: [PATCH 08/28] Add tests for `pr_changed_files` --- .buildkite/pipeline.yml | 26 ++++++++ tests/pr_changed_files/test_basic_changes.sh | 32 +++++++++ tests/pr_changed_files/test_edge_cases.sh | 65 +++++++++++++++++++ tests/pr_changed_files/test_helpers.sh | 59 +++++++++++++++++ .../test_includes_patterns.sh | 57 ++++++++++++++++ .../pr_changed_files/test_only_in_patterns.sh | 49 ++++++++++++++ 6 files changed, 288 insertions(+) create mode 100755 tests/pr_changed_files/test_basic_changes.sh create mode 100755 tests/pr_changed_files/test_edge_cases.sh create mode 100755 tests/pr_changed_files/test_helpers.sh create mode 100755 tests/pr_changed_files/test_includes_patterns.sh create mode 100755 tests/pr_changed_files/test_only_in_patterns.sh diff --git a/.buildkite/pipeline.yml b/.buildkite/pipeline.yml index b2692307..e7eaed43 100644 --- a/.buildkite/pipeline.yml +++ b/.buildkite/pipeline.yml @@ -92,3 +92,29 @@ steps: notify: - github_commit_status: context: "install_swiftpm_dependencies Tests: Xcode Workspace and Project - Explicit" + + - group: ":git: pr_changed_files Tests" + steps: + - label: ":git: Basic Changes Detection" + command: tests/pr_changed_files/test_basic_changes.sh + notify: + - github_commit_status: + context: "pr_changed_files Tests: Basic Changes Detection" + + - label: ":git: Pattern Matching - Includes" + command: tests/pr_changed_files/test_includes_patterns.sh + notify: + - github_commit_status: + context: "pr_changed_files Tests: Pattern Matching - Includes" + + - label: ":git: Pattern Matching - Only In" + command: tests/pr_changed_files/test_only_in_patterns.sh + notify: + - github_commit_status: + context: "pr_changed_files Tests: Pattern Matching - Only In" + + - label: ":git: Edge Cases" + command: tests/pr_changed_files/test_edge_cases.sh + notify: + - github_commit_status: + context: "pr_changed_files Tests: Edge Cases" diff --git a/tests/pr_changed_files/test_basic_changes.sh b/tests/pr_changed_files/test_basic_changes.sh new file mode 100755 index 00000000..0108a57c --- /dev/null +++ b/tests/pr_changed_files/test_basic_changes.sh @@ -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" diff --git a/tests/pr_changed_files/test_edge_cases.sh b/tests/pr_changed_files/test_edge_cases.sh new file mode 100755 index 00000000..cff15d69 --- /dev/null +++ b/tests/pr_changed_files/test_edge_cases.sh @@ -0,0 +1,65 @@ +#!/bin/bash -eu + +set -o pipefail + +source "$(dirname "${BASH_SOURCE[0]}")/test_helpers.sh" + +echo "--- :git: Testing edge cases" + +# 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] Invalid PR environment +unset BUILDKITE_PULL_REQUEST +if pr_changed_files 2>/dev/null; then + echo "Should fail when not in PR environment" + exit 1 +fi + +export BUILDKITE_PULL_REQUEST="123" + +# [Test] No patterns provided +if pr_changed_files --includes 2>/dev/null; then + echo "Should fail when no patterns provided" + exit 1 +fi + +# [Test] Mutually exclusive options +if pr_changed_files --includes "*.txt" --only-in "*.md" 2>/dev/null; then + echo "Should fail when using both --includes and --only-in" + exit 1 +fi + +# [Test] Files with spaces and special characters +mkdir -p "folder with spaces/nested\!\@\#\$folder" +echo "test" > "folder with spaces/file with spaces.txt" +echo "test" > "folder with spaces/nested\!\@\#\$folder/file_with_\!\@\#.txt" +git add . +git commit -m "Add files with special characters" + +result=$(pr_changed_files) +assert_output "true" "$result" "Should handle files with spaces and special characters" + +# [Test] Pattern matching with spaces and special characters +result=$(pr_changed_files --includes "*spaces.txt") +assert_output "true" "$result" "Should match files with special characters in path" + +result=$(pr_changed_files --only-in "folder with spaces/*") +assert_output "true" "$result" "Should handle directory patterns with spaces" + +# [Test] Empty repository state +git checkout --orphan empty +git rm -rf . + +result=$(pr_changed_files) +assert_output "false" "$result" "Should handle empty repository state" + +echo "✅ Edge cases tests passed" diff --git a/tests/pr_changed_files/test_helpers.sh b/tests/pr_changed_files/test_helpers.sh new file mode 100755 index 00000000..2879a25f --- /dev/null +++ b/tests/pr_changed_files/test_helpers.sh @@ -0,0 +1,59 @@ +#!/bin/bash -eu + +set -o pipefail + +# Create a temporary git repository for testing +create_tmp_repo_dir() { + local temp_dir + temp_dir=$(mktemp -d) + echo "$temp_dir" +} + +# Initialize the test repository +init_test_repo() { + local repo_dir="$1" + ORIGINAL_DIR=$(pwd) + pushd "$repo_dir" + + # Initialize git repo + git init + git config user.email "test@example.com" + git config user.name "Test User" + + # Create and commit initial files on main branch + echo "initial" > initial.txt + git add initial.txt + git commit -m "Initial commit" + + # Create base branch + git checkout -b base + echo "base" > base.txt + git add base.txt + git commit -m "Base branch commit" + + # Create PR branch + git checkout -b pr +} + +# Clean up the temporary repository +cleanup_git_repo() { + # Return to original directory if we're still in the temp dir + if [[ "$(pwd)" == "$1" ]]; then + cd "$ORIGINAL_DIR" + fi + rm -rf "$1" +} + +# Helper to assert expected output +assert_output() { + local expected="$1" + local actual="$2" + local message="${3:-}" + + if [[ "$actual" != "$expected" ]]; then + echo "❌ Assertion failed: $message" + echo "Expected: $expected" + echo "Actual : $actual" + exit 1 + fi +} diff --git a/tests/pr_changed_files/test_includes_patterns.sh b/tests/pr_changed_files/test_includes_patterns.sh new file mode 100755 index 00000000..72472251 --- /dev/null +++ b/tests/pr_changed_files/test_includes_patterns.sh @@ -0,0 +1,57 @@ +#!/bin/bash -eu + +set -o pipefail + +source "$(dirname "${BASH_SOURCE[0]}")/test_helpers.sh" + +echo "--- :git: Testing includes 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 +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 --includes "*.swift") +assert_output "true" "$result" "Should match .swift files" + +# [Test] Match multiple patterns +result=$(pr_changed_files --includes "docs/*.md" "*.rb") +assert_output "true" "$result" "Should match multiple patterns" + +# [Test] Match files with spaces and special characters +result=$(pr_changed_files --includes "docs/read me.md" "docs/special\!\@\#\$chars.md") +assert_output "true" "$result" "Should match files with spaces and special characters" + +# [Test] No matches +result=$(pr_changed_files --includes "*.js") +assert_output "false" "$result" "Should not match non-existent patterns" + +# [Test] Directory pattern +result=$(pr_changed_files --includes "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 --includes "*.swift") +assert_output "true" "$result" "Should only match exact patterns" + +echo "✅ Includes pattern tests passed" diff --git a/tests/pr_changed_files/test_only_in_patterns.sh b/tests/pr_changed_files/test_only_in_patterns.sh new file mode 100755 index 00000000..9d45c742 --- /dev/null +++ b/tests/pr_changed_files/test_only_in_patterns.sh @@ -0,0 +1,49 @@ +#!/bin/bash -eu + +set -o pipefail + +source "$(dirname "${BASH_SOURCE[0]}")/test_helpers.sh" + +echo "--- :git: Testing only-in 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 +mkdir -p docs src/swift +echo "doc" > "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 --only-in "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 +result=$(pr_changed_files --only-in "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] 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 --only-in "docs/*") +assert_output "false" "$result" "Should return false when changes exist outside patterns" + +# [Test] Multiple patterns, all matching +result=$(pr_changed_files --only-in "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" + +echo "✅ Only-in pattern tests passed" From a6296b0f1a1bfa415e33f4f3327723e343ad23f2 Mon Sep 17 00:00:00 2001 From: Ian Maia Date: Wed, 5 Feb 2025 18:32:50 +0100 Subject: [PATCH 09/28] Improve handling of filenames with spaces / multiple files in `pr_changed_file` --- bin/pr_changed_files | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/bin/pr_changed_files b/bin/pr_changed_files index b15138ab..9693359d 100755 --- a/bin/pr_changed_files +++ b/bin/pr_changed_files @@ -59,7 +59,8 @@ while [[ "$#" -gt 0 ]]; do done # Get list of changed files -changed_files=$(git --no-pager diff --name-only --merge-base "$BUILDKITE_PULL_REQUEST_BASE_BRANCH" HEAD | sort) +# Use -z to get null-terminated output and avoid escaping issues +changed_files=$(git --no-pager diff --name-only -z --merge-base "$BUILDKITE_PULL_REQUEST_BASE_BRANCH" HEAD | tr '\0' '\n' | sort) if [[ -z "$mode" ]]; then # No arguments = any change @@ -88,11 +89,14 @@ fi if [[ "$mode" == "only-in" ]]; then # Check if all changed files match at least one pattern - for file in "$changed_files"; do + while IFS= read -r file; do + [[ -z "$file" ]] && continue + # Remove any escaping of special characters + file="${file//\\/}" matches_any=false for pattern in "${patterns[@]}"; do # shellcheck disable=SC2053 - if [[ "$file" == $pattern ]]; then # glob match with pattern + if [[ "$file" == ${pattern} ]]; then matches_any=true break fi @@ -101,18 +105,21 @@ if [[ "$mode" == "only-in" ]]; then echo "false" exit 0 fi - done + done <<< "$changed_files" echo "true" elif [[ "$mode" == "includes" ]]; then # Check if any changed file matches any pattern - for file in $changed_files; do + while IFS= read -r file; do + [[ -z "$file" ]] && continue + # Remove any escaping of special characters + file="${file//\\/}" for pattern in "${patterns[@]}"; do # shellcheck disable=SC2053 - if [[ "$file" == $pattern ]]; then # glob match with pattern + if [[ "$file" == ${pattern} ]]; then echo "true" exit 0 fi done - done + done <<< "$changed_files" echo "false" fi From cedbacc064a42368b5314081bfa53ea4bc388244 Mon Sep 17 00:00:00 2001 From: Ian Maia Date: Wed, 5 Feb 2025 19:34:25 +0100 Subject: [PATCH 10/28] Improve shellcheck comment --- bin/pr_changed_files | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bin/pr_changed_files b/bin/pr_changed_files index 9693359d..c94bda80 100755 --- a/bin/pr_changed_files +++ b/bin/pr_changed_files @@ -95,7 +95,7 @@ if [[ "$mode" == "only-in" ]]; then file="${file//\\/}" matches_any=false for pattern in "${patterns[@]}"; do - # shellcheck disable=SC2053 + # 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 matches_any=true break @@ -114,7 +114,7 @@ elif [[ "$mode" == "includes" ]]; then # Remove any escaping of special characters file="${file//\\/}" for pattern in "${patterns[@]}"; do - # shellcheck disable=SC2053 + # 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 echo "true" exit 0 From 98fd7050b62ba24cf50d8b090afca0597fce97f3 Mon Sep 17 00:00:00 2001 From: Ian Maia Date: Wed, 5 Feb 2025 19:56:08 +0100 Subject: [PATCH 11/28] Fix for running tests on CI --- .buildkite/pipeline.yml | 16 ++++++++-------- tests/pr_changed_files/test_helpers.sh | 4 ++++ 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/.buildkite/pipeline.yml b/.buildkite/pipeline.yml index e7eaed43..ebde99ea 100644 --- a/.buildkite/pipeline.yml +++ b/.buildkite/pipeline.yml @@ -93,27 +93,27 @@ steps: - github_commit_status: context: "install_swiftpm_dependencies Tests: Xcode Workspace and Project - Explicit" - - group: ":git: pr_changed_files Tests" + - group: ":github: pr_changed_files Tests" steps: - - label: ":git: Basic Changes Detection" + - label: ":github: pr_changed_files Tests - Basic Changes" command: tests/pr_changed_files/test_basic_changes.sh notify: - github_commit_status: - context: "pr_changed_files Tests: Basic Changes Detection" + context: "pr_changed_files Tests: Basic Changes" - - label: ":git: Pattern Matching - Includes" + - label: ":github: pr_changed_files Tests - Includes" command: tests/pr_changed_files/test_includes_patterns.sh notify: - github_commit_status: - context: "pr_changed_files Tests: Pattern Matching - Includes" + context: "pr_changed_files Tests: Includes" - - label: ":git: Pattern Matching - Only In" + - label: ":github: pr_changed_files Tests - Only In" command: tests/pr_changed_files/test_only_in_patterns.sh notify: - github_commit_status: - context: "pr_changed_files Tests: Pattern Matching - Only In" + context: "pr_changed_files Tests: Only In" - - label: ":git: Edge Cases" + - label: ":github: pr_changed_files Tests - Edge Cases" command: tests/pr_changed_files/test_edge_cases.sh notify: - github_commit_status: diff --git a/tests/pr_changed_files/test_helpers.sh b/tests/pr_changed_files/test_helpers.sh index 2879a25f..9faa4f20 100755 --- a/tests/pr_changed_files/test_helpers.sh +++ b/tests/pr_changed_files/test_helpers.sh @@ -2,6 +2,10 @@ set -o pipefail +# Add bin directory to PATH +REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" +export PATH="$REPO_ROOT/bin:$PATH" + # Create a temporary git repository for testing create_tmp_repo_dir() { local temp_dir From f33a25bc99c94b8ad698b768d2ab491fcc20bc1f Mon Sep 17 00:00:00 2001 From: "Ian G. Maia" Date: Wed, 5 Feb 2025 20:10:06 +0100 Subject: [PATCH 12/28] Update documented examples Co-authored-by: Olivier Halligon --- bin/pr_changed_files | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/pr_changed_files b/bin/pr_changed_files index c94bda80..e9228155 100755 --- a/bin/pr_changed_files +++ b/bin/pr_changed_files @@ -12,7 +12,7 @@ # pr_changed_files # # # Check if only documentation files changed. Could be useful to decide to skip UI tests for example -# pr_changed_files --only-in "*.md" "docs/**" +# pr_changed_files --only-in "*.md" "docs/*" # # # Check if any Swift files changed, e.g. to decide if we should run Swiftlint # pr_changed_files --includes "*.swift" ".swiftlint.yml" From 198940207d21cb9563569baa564ff350489b81d4 Mon Sep 17 00:00:00 2001 From: Ian Maia Date: Wed, 5 Feb 2025 21:13:12 +0100 Subject: [PATCH 13/28] Update to get list of changed files as an array --- bin/pr_changed_files | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/bin/pr_changed_files b/bin/pr_changed_files index e9228155..2bee2d5e 100755 --- a/bin/pr_changed_files +++ b/bin/pr_changed_files @@ -58,13 +58,15 @@ while [[ "$#" -gt 0 ]]; do esac done -# Get list of changed files -# Use -z to get null-terminated output and avoid escaping issues -changed_files=$(git --no-pager diff --name-only -z --merge-base "$BUILDKITE_PULL_REQUEST_BASE_BRANCH" HEAD | tr '\0' '\n' | sort) +# Get list of changed files as an array +changed_files=() +while IFS= read -r file; do + changed_files+=("$file") +done < <(git --no-pager diff --name-only --merge-base "$BUILDKITE_PULL_REQUEST_BASE_BRANCH" HEAD | sort) if [[ -z "$mode" ]]; then # No arguments = any change - if [[ -n "$changed_files" ]]; then + if [[ ${#changed_files[@]} -gt 0 ]]; then echo "true" else echo "false" @@ -78,7 +80,7 @@ if [[ ${#patterns[@]} -eq 0 ]]; then fi # No files changed -if [[ -z "$changed_files" ]]; then +if [[ ${#changed_files[@]} -eq 0 ]]; then if [[ "$mode" == "only-in" ]]; then echo "true" else @@ -89,10 +91,7 @@ fi if [[ "$mode" == "only-in" ]]; then # Check if all changed files match at least one pattern - while IFS= read -r file; do - [[ -z "$file" ]] && continue - # Remove any escaping of special characters - file="${file//\\/}" + for file in "${changed_files[@]}"; do matches_any=false for pattern in "${patterns[@]}"; 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 @@ -105,14 +104,11 @@ if [[ "$mode" == "only-in" ]]; then echo "false" exit 0 fi - done <<< "$changed_files" + done echo "true" elif [[ "$mode" == "includes" ]]; then # Check if any changed file matches any pattern - while IFS= read -r file; do - [[ -z "$file" ]] && continue - # Remove any escaping of special characters - file="${file//\\/}" + for file in "${changed_files[@]}"; do for pattern in "${patterns[@]}"; 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 @@ -120,6 +116,6 @@ elif [[ "$mode" == "includes" ]]; then exit 0 fi done - done <<< "$changed_files" + done echo "false" fi From a8294c77fcf6947bb0e2176b0bebcf4829cfbde3 Mon Sep 17 00:00:00 2001 From: Ian Maia Date: Thu, 6 Feb 2025 14:55:19 +0100 Subject: [PATCH 14/28] Fix code to unescape file names --- bin/pr_changed_files | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/bin/pr_changed_files b/bin/pr_changed_files index 2bee2d5e..bc672e6e 100755 --- a/bin/pr_changed_files +++ b/bin/pr_changed_files @@ -61,6 +61,10 @@ done # Get list of changed files as an array changed_files=() while IFS= read -r file; do + # Remove surrounding quotes if present and unescape special characters + file="${file#\"}" + file="${file%\"}" + file="${file//\\/}" changed_files+=("$file") done < <(git --no-pager diff --name-only --merge-base "$BUILDKITE_PULL_REQUEST_BASE_BRANCH" HEAD | sort) From 8ba279f3179222d90b60778dbb67964b08d34bb8 Mon Sep 17 00:00:00 2001 From: Ian Maia Date: Thu, 6 Feb 2025 16:02:57 +0100 Subject: [PATCH 15/28] Rename arguments `--includes` to `--any-match` and `--only-in` to `--all-match` --- .buildkite/pipeline.yml | 12 ++++++------ bin/pr_changed_files | 18 +++++++++--------- ..._patterns.sh => test_all_match_patterns.sh} | 12 ++++++------ ..._patterns.sh => test_any_match_patterns.sh} | 16 ++++++++-------- tests/pr_changed_files/test_edge_cases.sh | 10 +++++----- 5 files changed, 34 insertions(+), 34 deletions(-) rename tests/pr_changed_files/{test_only_in_patterns.sh => test_all_match_patterns.sh} (75%) rename tests/pr_changed_files/{test_includes_patterns.sh => test_any_match_patterns.sh} (75%) diff --git a/.buildkite/pipeline.yml b/.buildkite/pipeline.yml index ebde99ea..8aa34993 100644 --- a/.buildkite/pipeline.yml +++ b/.buildkite/pipeline.yml @@ -101,17 +101,17 @@ steps: - github_commit_status: context: "pr_changed_files Tests: Basic Changes" - - label: ":github: pr_changed_files Tests - Includes" - command: tests/pr_changed_files/test_includes_patterns.sh + - label: ":github: pr_changed_files Tests - Any Match" + command: tests/pr_changed_files/test_any_match_patterns.sh notify: - github_commit_status: - context: "pr_changed_files Tests: Includes" + context: "pr_changed_files Tests: Any Match" - - label: ":github: pr_changed_files Tests - Only In" - command: tests/pr_changed_files/test_only_in_patterns.sh + - label: ":github: pr_changed_files Tests - All Match" + command: tests/pr_changed_files/test_all_match_patterns.sh notify: - github_commit_status: - context: "pr_changed_files Tests: Only In" + context: "pr_changed_files Tests: All Match" - label: ":github: pr_changed_files Tests - Edge Cases" command: tests/pr_changed_files/test_edge_cases.sh diff --git a/bin/pr_changed_files b/bin/pr_changed_files index bc672e6e..c6ed7b8c 100755 --- a/bin/pr_changed_files +++ b/bin/pr_changed_files @@ -4,18 +4,18 @@ # # Usage: # pr_changed_files # Check if any files changed -# pr_changed_files --includes # Check if changes include files matching patterns -# pr_changed_files --only-in # Check if changes are limited to given patterns +# pr_changed_files --any-match # Check if changes include files matching patterns +# pr_changed_files --all-match # Check if changes are limited to given patterns # # Examples: # # Check if any files changed # pr_changed_files # # # Check if only documentation files changed. Could be useful to decide to skip UI tests for example -# pr_changed_files --only-in "*.md" "docs/*" +# pr_changed_files --all-match "*.md" "docs/*" # # # Check if any Swift files changed, e.g. to decide if we should run Swiftlint -# pr_changed_files --includes "*.swift" ".swiftlint.yml" +# pr_changed_files --any-match "*.swift" ".swiftlint.yml" # # Returns: # Prints "true" if the condition is met, "false" otherwise @@ -31,11 +31,11 @@ mode="" patterns=() # Define error message for mutually exclusive options -EXCLUSIVE_OPTIONS_ERROR="Error: either specify --only-in or --includes; cannot specify both" +EXCLUSIVE_OPTIONS_ERROR="Error: either specify --all-match or --any-match; cannot specify both" while [[ "$#" -gt 0 ]]; do case $1 in - --only-in | --includes) + --all-match | --any-match) if [[ -n "$mode" ]]; then echo "$EXCLUSIVE_OPTIONS_ERROR" >&2 exit 1 @@ -85,7 +85,7 @@ fi # No files changed if [[ ${#changed_files[@]} -eq 0 ]]; then - if [[ "$mode" == "only-in" ]]; then + if [[ "$mode" == "all-match" ]]; then echo "true" else echo "false" @@ -93,7 +93,7 @@ if [[ ${#changed_files[@]} -eq 0 ]]; then exit 0 fi -if [[ "$mode" == "only-in" ]]; then +if [[ "$mode" == "all-match" ]]; then # Check if all changed files match at least one pattern for file in "${changed_files[@]}"; do matches_any=false @@ -110,7 +110,7 @@ if [[ "$mode" == "only-in" ]]; then fi done echo "true" -elif [[ "$mode" == "includes" ]]; then +elif [[ "$mode" == "any-match" ]]; then # Check if any changed file matches any pattern for file in "${changed_files[@]}"; do for pattern in "${patterns[@]}"; do diff --git a/tests/pr_changed_files/test_only_in_patterns.sh b/tests/pr_changed_files/test_all_match_patterns.sh similarity index 75% rename from tests/pr_changed_files/test_only_in_patterns.sh rename to tests/pr_changed_files/test_all_match_patterns.sh index 9d45c742..9887f594 100755 --- a/tests/pr_changed_files/test_only_in_patterns.sh +++ b/tests/pr_changed_files/test_all_match_patterns.sh @@ -4,7 +4,7 @@ set -o pipefail source "$(dirname "${BASH_SOURCE[0]}")/test_helpers.sh" -echo "--- :git: Testing only-in pattern matching" +echo "--- :git: Testing all-match pattern matching" # Create test repository repo_path=$(create_tmp_repo_dir) @@ -26,11 +26,11 @@ git add . git commit -m "Add doc files" # [Test] All changes in docs -result=$(pr_changed_files --only-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 -result=$(pr_changed_files --only-in "docs/read me.md" "docs/guide with spaces.md" "docs/special\!\@\#\$chars.md") +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] Changes outside pattern @@ -39,11 +39,11 @@ echo "swift" > "src/swift/special\!\@\#\$chars.swift" git add . git commit -m "Add swift file" -result=$(pr_changed_files --only-in "docs/*") +result=$(pr_changed_files --all-match "docs/*") assert_output "false" "$result" "Should return false when changes exist outside patterns" # [Test] Multiple patterns, all matching -result=$(pr_changed_files --only-in "docs/*" "src/swift/main with spaces.swift" "src/swift/special\!\@\#\$chars.swift") +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" -echo "✅ Only-in pattern tests passed" +echo "✅ All-match pattern tests passed" \ No newline at end of file diff --git a/tests/pr_changed_files/test_includes_patterns.sh b/tests/pr_changed_files/test_any_match_patterns.sh similarity index 75% rename from tests/pr_changed_files/test_includes_patterns.sh rename to tests/pr_changed_files/test_any_match_patterns.sh index 72472251..0c06678e 100755 --- a/tests/pr_changed_files/test_includes_patterns.sh +++ b/tests/pr_changed_files/test_any_match_patterns.sh @@ -4,7 +4,7 @@ set -o pipefail source "$(dirname "${BASH_SOURCE[0]}")/test_helpers.sh" -echo "--- :git: Testing includes pattern matching" +echo "--- :git: Testing any-match pattern matching" # Create test repository repo_path=$(create_tmp_repo_dir) @@ -27,23 +27,23 @@ git add . git commit -m "Add test files" # [Test] Match specific extension -result=$(pr_changed_files --includes "*.swift") +result=$(pr_changed_files --any-match "*.swift") assert_output "true" "$result" "Should match .swift files" # [Test] Match multiple patterns -result=$(pr_changed_files --includes "docs/*.md" "*.rb") +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 --includes "docs/read me.md" "docs/special\!\@\#\$chars.md") +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] No matches -result=$(pr_changed_files --includes "*.js") +result=$(pr_changed_files --any-match "*.js") assert_output "false" "$result" "Should not match non-existent patterns" # [Test] Directory pattern -result=$(pr_changed_files --includes "docs/*") +result=$(pr_changed_files --any-match "docs/*") assert_output "true" "$result" "Should match directory patterns" # [Test] Exact pattern matching @@ -51,7 +51,7 @@ echo "swiftfile" > swiftfile.txt git add swiftfile.txt git commit -m "Add file with swift in name" -result=$(pr_changed_files --includes "*.swift") +result=$(pr_changed_files --any-match "*.swift") assert_output "true" "$result" "Should only match exact patterns" -echo "✅ Includes pattern tests passed" +echo "✅ Any-match pattern tests passed" \ No newline at end of file diff --git a/tests/pr_changed_files/test_edge_cases.sh b/tests/pr_changed_files/test_edge_cases.sh index cff15d69..af358582 100755 --- a/tests/pr_changed_files/test_edge_cases.sh +++ b/tests/pr_changed_files/test_edge_cases.sh @@ -27,14 +27,14 @@ fi export BUILDKITE_PULL_REQUEST="123" # [Test] No patterns provided -if pr_changed_files --includes 2>/dev/null; then +if pr_changed_files --any-match 2>/dev/null; then echo "Should fail when no patterns provided" exit 1 fi # [Test] Mutually exclusive options -if pr_changed_files --includes "*.txt" --only-in "*.md" 2>/dev/null; then - echo "Should fail when using both --includes and --only-in" +if pr_changed_files --any-match "*.txt" --all-match "*.md" 2>/dev/null; then + echo "Should fail when using both --any-match and --all-match" exit 1 fi @@ -49,10 +49,10 @@ result=$(pr_changed_files) assert_output "true" "$result" "Should handle files with spaces and special characters" # [Test] Pattern matching with spaces and special characters -result=$(pr_changed_files --includes "*spaces.txt") +result=$(pr_changed_files --any-match "*spaces.txt") assert_output "true" "$result" "Should match files with special characters in path" -result=$(pr_changed_files --only-in "folder with spaces/*") +result=$(pr_changed_files --all-match "folder with spaces/*") assert_output "true" "$result" "Should handle directory patterns with spaces" # [Test] Empty repository state From ff689a2b763977da9e79c87ea82bc430d62a7e02 Mon Sep 17 00:00:00 2001 From: Ian Maia Date: Thu, 6 Feb 2025 16:24:06 +0100 Subject: [PATCH 16/28] Add `git fetch origin` to make sure the branch is fetched in the agent --- bin/pr_changed_files | 6 ++++++ tests/pr_changed_files/test_helpers.sh | 17 +++++++++++++++-- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/bin/pr_changed_files b/bin/pr_changed_files index c6ed7b8c..b26be287 100755 --- a/bin/pr_changed_files +++ b/bin/pr_changed_files @@ -27,6 +27,12 @@ if [[ ! "${BUILDKITE_PULL_REQUEST:-invalid}" =~ ^[0-9]+$ ]]; then 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=() diff --git a/tests/pr_changed_files/test_helpers.sh b/tests/pr_changed_files/test_helpers.sh index 9faa4f20..f0bcb18a 100755 --- a/tests/pr_changed_files/test_helpers.sh +++ b/tests/pr_changed_files/test_helpers.sh @@ -17,13 +17,23 @@ create_tmp_repo_dir() { init_test_repo() { local repo_dir="$1" ORIGINAL_DIR=$(pwd) - pushd "$repo_dir" + + # Create a bare repo to act as remote + mkdir -p "$repo_dir/remote" + git init --bare "$repo_dir/remote" + + # Create the working repo + mkdir -p "$repo_dir/local" + pushd "$repo_dir/local" # Initialize git repo git init git config user.email "test@example.com" git config user.name "Test User" + # Add remote + git remote add origin "$repo_dir/remote" + # Create and commit initial files on main branch echo "initial" > initial.txt git add initial.txt @@ -34,6 +44,9 @@ init_test_repo() { echo "base" > base.txt git add base.txt git commit -m "Base branch commit" + + # Push base branch to remote + git push -u origin base # Create PR branch git checkout -b pr @@ -42,7 +55,7 @@ init_test_repo() { # Clean up the temporary repository cleanup_git_repo() { # Return to original directory if we're still in the temp dir - if [[ "$(pwd)" == "$1" ]]; then + if [[ "$(pwd)" == "$1/local" ]]; then cd "$ORIGINAL_DIR" fi rm -rf "$1" From 15e7a3c93f14a3afb0aa1d683840bec5db89d13d Mon Sep 17 00:00:00 2001 From: Ian Maia Date: Thu, 6 Feb 2025 16:36:50 +0100 Subject: [PATCH 17/28] Extract file pattern match to a function --- bin/pr_changed_files | 36 ++++++++++--------- .../test_all_match_patterns.sh | 2 +- .../test_any_match_patterns.sh | 2 +- 3 files changed, 22 insertions(+), 18 deletions(-) diff --git a/bin/pr_changed_files b/bin/pr_changed_files index b26be287..60b409b4 100755 --- a/bin/pr_changed_files +++ b/bin/pr_changed_files @@ -99,18 +99,25 @@ if [[ ${#changed_files[@]} -eq 0 ]]; then exit 0 fi +# Returns 0 if the file matches any of the patterns, 1 otherwise +file_matches_any_pattern() { + local file="$1" + shift + local patterns=("$@") + + for pattern in "${patterns[@]}"; 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 - matches_any=false - for pattern in "${patterns[@]}"; 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 - matches_any=true - break - fi - done - if [[ "$matches_any" == "false" ]]; then + if ! file_matches_any_pattern "$file" "${patterns[@]}"; then echo "false" exit 0 fi @@ -119,13 +126,10 @@ if [[ "$mode" == "all-match" ]]; then elif [[ "$mode" == "any-match" ]]; then # Check if any changed file matches any pattern for file in "${changed_files[@]}"; do - for pattern in "${patterns[@]}"; 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 - echo "true" - exit 0 - fi - done + if file_matches_any_pattern "$file" "${patterns[@]}"; then + echo "true" + exit 0 + fi done echo "false" fi diff --git a/tests/pr_changed_files/test_all_match_patterns.sh b/tests/pr_changed_files/test_all_match_patterns.sh index 9887f594..d2e70d38 100755 --- a/tests/pr_changed_files/test_all_match_patterns.sh +++ b/tests/pr_changed_files/test_all_match_patterns.sh @@ -46,4 +46,4 @@ assert_output "false" "$result" "Should return false when changes exist outside 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" -echo "✅ All-match pattern tests passed" \ No newline at end of file +echo "✅ All-match pattern tests passed" diff --git a/tests/pr_changed_files/test_any_match_patterns.sh b/tests/pr_changed_files/test_any_match_patterns.sh index 0c06678e..6790caad 100755 --- a/tests/pr_changed_files/test_any_match_patterns.sh +++ b/tests/pr_changed_files/test_any_match_patterns.sh @@ -54,4 +54,4 @@ 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" \ No newline at end of file +echo "✅ Any-match pattern tests passed" From 45fefcfc67a30b74602e3e5771969cd1181e9957 Mon Sep 17 00:00:00 2001 From: Ian Maia Date: Thu, 6 Feb 2025 16:50:25 +0100 Subject: [PATCH 18/28] Add more examples and details in the command documentation --- bin/pr_changed_files | 34 +++++++++++++++++++++++++++------- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/bin/pr_changed_files b/bin/pr_changed_files index 60b409b4..e5402a0d 100755 --- a/bin/pr_changed_files +++ b/bin/pr_changed_files @@ -7,15 +7,35 @@ # pr_changed_files --any-match # Check if changes include files matching patterns # pr_changed_files --all-match # Check if changes are limited to given patterns # -# Examples: -# # Check if any files changed -# pr_changed_files +# Behavior: +# With no arguments: +# Returns "true" if the PR contains any changes, "false" otherwise # -# # Check if only documentation files changed. Could be useful to decide to skip UI tests for example -# pr_changed_files --all-match "*.md" "docs/*" +# 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 # -# # Check if any Swift files changed, e.g. to decide if we should run Swiftlint -# pr_changed_files --any-match "*.swift" ".swiftlint.yml" +# 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 +# +# 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/guide.md (not all patterns need to match) +# → "false" if PR changes docs/guide.md AND src/main.swift +# +# # 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 +# → "true" if PR changes .swiftlint.yml +# → "false" if PR only changes README.md # # Returns: # Prints "true" if the condition is met, "false" otherwise From bd983b81c1a139316b0436917067a8354cc0b633 Mon Sep 17 00:00:00 2001 From: Ian Maia Date: Thu, 6 Feb 2025 17:16:00 +0100 Subject: [PATCH 19/28] Improve file pattern arguments handling & tests --- bin/pr_changed_files | 20 +++++--------------- tests/pr_changed_files/test_edge_cases.sh | 10 ++++++---- 2 files changed, 11 insertions(+), 19 deletions(-) diff --git a/bin/pr_changed_files b/bin/pr_changed_files index e5402a0d..28ef4652 100755 --- a/bin/pr_changed_files +++ b/bin/pr_changed_files @@ -68,6 +68,11 @@ while [[ "$#" -gt 0 ]]; do fi mode="${1#--}" shift + # Check if there are any patterns after the flag + if [[ "$#" -eq 0 || "$1" == "--"* ]]; then + echo "Error: must specify at least one file pattern" >&2 + exit 1 + fi while [[ "$#" -gt 0 && "$1" != "--"* ]]; do patterns+=("$1") shift @@ -104,21 +109,6 @@ if [[ -z "$mode" ]]; then exit 0 fi -if [[ ${#patterns[@]} -eq 0 ]]; then - echo "Error: must specify at least one file pattern" >&2 - exit 1 -fi - -# No files changed -if [[ ${#changed_files[@]} -eq 0 ]]; then - if [[ "$mode" == "all-match" ]]; 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" diff --git a/tests/pr_changed_files/test_edge_cases.sh b/tests/pr_changed_files/test_edge_cases.sh index af358582..c6ad6fe4 100755 --- a/tests/pr_changed_files/test_edge_cases.sh +++ b/tests/pr_changed_files/test_edge_cases.sh @@ -32,11 +32,13 @@ if pr_changed_files --any-match 2>/dev/null; then exit 1 fi +# [Test] Flag followed by another flag +output=$(pr_changed_files --any-match --something 2>&1 || true) +assert_output "Error: must specify at least one file pattern" "$output" "Should fail with correct error when flag is followed by another flag" + # [Test] Mutually exclusive options -if pr_changed_files --any-match "*.txt" --all-match "*.md" 2>/dev/null; then - echo "Should fail when using both --any-match and --all-match" - exit 1 -fi +output=$(pr_changed_files --any-match "*.txt" --all-match "*.md" 2>&1 || true) +assert_output "Error: either specify --all-match or --any-match; cannot specify both" "$output" "Should fail with correct error when using mutually exclusive options" # [Test] Files with spaces and special characters mkdir -p "folder with spaces/nested\!\@\#\$folder" From 5617e470856cbef557b480126cbb09ab00d31f33 Mon Sep 17 00:00:00 2001 From: "Ian G. Maia" Date: Fri, 7 Feb 2025 19:46:20 +0100 Subject: [PATCH 20/28] Update documentation (suggestions from code review) Co-authored-by: Olivier Halligon --- bin/pr_changed_files | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/bin/pr_changed_files b/bin/pr_changed_files index 28ef4652..f0356d80 100755 --- a/bin/pr_changed_files +++ b/bin/pr_changed_files @@ -14,28 +14,30 @@ # 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 +# 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 # # 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 +# 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) # # 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/guide.md (not all patterns need to match) -# → "false" if PR changes docs/guide.md AND src/main.swift -# # # 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 -# → "true" if PR changes .swiftlint.yml -# → "false" if PR only changes README.md +# → "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) +# +# # 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) # # Returns: # Prints "true" if the condition is met, "false" otherwise From 0e208ef7c1544ccc5536e43f7be78aabecd3e349 Mon Sep 17 00:00:00 2001 From: "Ian G. Maia" Date: Fri, 7 Feb 2025 20:00:46 +0100 Subject: [PATCH 21/28] Update bin/pr_changed_files Co-authored-by: Olivier Halligon --- bin/pr_changed_files | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/bin/pr_changed_files b/bin/pr_changed_files index f0356d80..8c9bfc9d 100755 --- a/bin/pr_changed_files +++ b/bin/pr_changed_files @@ -71,14 +71,14 @@ while [[ "$#" -gt 0 ]]; do mode="${1#--}" shift # Check if there are any patterns after the flag - if [[ "$#" -eq 0 || "$1" == "--"* ]]; then - echo "Error: must specify at least one file pattern" >&2 - exit 1 - fi 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 From 2277ed929d29e9635ab7146568c3ee2239ed58e1 Mon Sep 17 00:00:00 2001 From: "Ian G. Maia" Date: Mon, 10 Feb 2025 18:10:16 +0100 Subject: [PATCH 22/28] Apply suggestions from code review Co-authored-by: Olivier Halligon --- bin/pr_changed_files | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/bin/pr_changed_files b/bin/pr_changed_files index 8c9bfc9d..ef351601 100755 --- a/bin/pr_changed_files +++ b/bin/pr_changed_files @@ -34,7 +34,7 @@ # → "false" if PR only changes `README.md` (none of files match any of the patterns) # # # Check if only documentation files changed (to skip UI tests for example) -# $ pr_changed_files --all-match "*.md" "docs/**" +# $ 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) @@ -115,9 +115,7 @@ fi file_matches_any_pattern() { local file="$1" shift - local patterns=("$@") - - for pattern in "${patterns[@]}"; do + 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 From 13d4dfbafdfb0d9e0d0f8ec39f240b5173a79837 Mon Sep 17 00:00:00 2001 From: Ian Maia Date: Mon, 10 Feb 2025 18:11:54 +0100 Subject: [PATCH 23/28] Consistently use the order 1. "all-match" 2. "any-match" --- bin/pr_changed_files | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/bin/pr_changed_files b/bin/pr_changed_files index ef351601..8ecd5e83 100755 --- a/bin/pr_changed_files +++ b/bin/pr_changed_files @@ -4,41 +4,41 @@ # # Usage: # pr_changed_files # Check if any files changed -# pr_changed_files --any-match # Check if changes include files matching patterns # pr_changed_files --all-match # Check if changes are limited to given patterns +# pr_changed_files --any-match # Check if changes include files matching patterns # # Behavior: # With no arguments: # Returns "true" if the PR contains any changes, "false" otherwise # -# 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 -# # 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 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) -# # # 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. From 262a0d2b6ffa0a4101a7f757910e14fbd4f94fb5 Mon Sep 17 00:00:00 2001 From: Ian Maia Date: Mon, 10 Feb 2025 19:24:00 +0100 Subject: [PATCH 24/28] Print result for succeeded tests and improve empty states tests --- tests/pr_changed_files/test_edge_cases.sh | 16 ++++++++++++++-- tests/pr_changed_files/test_helpers.sh | 4 +++- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/tests/pr_changed_files/test_edge_cases.sh b/tests/pr_changed_files/test_edge_cases.sh index c6ad6fe4..28771c61 100755 --- a/tests/pr_changed_files/test_edge_cases.sh +++ b/tests/pr_changed_files/test_edge_cases.sh @@ -57,11 +57,23 @@ assert_output "true" "$result" "Should match files with special characters in pa result=$(pr_changed_files --all-match "folder with spaces/*") assert_output "true" "$result" "Should handle directory patterns with spaces" +# [Test] No changes between branches +git checkout -b no_changes base +result=$(pr_changed_files) +assert_output "false" "$result" "Should handle no changes between branches" + +# [Test] Empty commit +git checkout -b empty_commit base +git commit --allow-empty -m "Empty commit" +result=$(pr_changed_files) +assert_output "false" "$result" "Should handle empty commit" + # [Test] Empty repository state git checkout --orphan empty git rm -rf . +git commit --allow-empty -m "Empty initial commit" -result=$(pr_changed_files) +result=$(pr_changed_files 2>/dev/null) assert_output "false" "$result" "Should handle empty repository state" -echo "✅ Edge cases tests passed" +echo -e "\n✅ Edge cases tests passed" diff --git a/tests/pr_changed_files/test_helpers.sh b/tests/pr_changed_files/test_helpers.sh index f0bcb18a..753d851a 100755 --- a/tests/pr_changed_files/test_helpers.sh +++ b/tests/pr_changed_files/test_helpers.sh @@ -67,7 +67,9 @@ assert_output() { local actual="$2" local message="${3:-}" - if [[ "$actual" != "$expected" ]]; then + if [[ "$actual" == "$expected" ]]; then + echo "🟢 Assertion succeeded: $message" + elif [[ "$actual" != "$expected" ]]; then echo "❌ Assertion failed: $message" echo "Expected: $expected" echo "Actual : $actual" From d794f967188853ed4999391ae9c8c77b8f0004b2 Mon Sep 17 00:00:00 2001 From: "Ian G. Maia" Date: Tue, 11 Feb 2025 19:19:59 +0100 Subject: [PATCH 25/28] Remove post-process of the quotes and escaped characters in the files in the diff Co-authored-by: Olivier Halligon --- bin/pr_changed_files | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/bin/pr_changed_files b/bin/pr_changed_files index 8ecd5e83..c6bbb8a6 100755 --- a/bin/pr_changed_files +++ b/bin/pr_changed_files @@ -93,13 +93,9 @@ done # Get list of changed files as an array changed_files=() -while IFS= read -r file; do - # Remove surrounding quotes if present and unescape special characters - file="${file#\"}" - file="${file%\"}" - file="${file//\\/}" - changed_files+=("$file") -done < <(git --no-pager diff --name-only --merge-base "$BUILDKITE_PULL_REQUEST_BASE_BRANCH" HEAD | sort) +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 From 4cc76b299fa894747fba4b15f66e319bfd267f43 Mon Sep 17 00:00:00 2001 From: Ian Maia Date: Tue, 11 Feb 2025 20:06:24 +0100 Subject: [PATCH 26/28] Fix escaping in all_match_patterns tests --- tests/pr_changed_files/test_all_match_patterns.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/pr_changed_files/test_all_match_patterns.sh b/tests/pr_changed_files/test_all_match_patterns.sh index d2e70d38..efc1319b 100755 --- a/tests/pr_changed_files/test_all_match_patterns.sh +++ b/tests/pr_changed_files/test_all_match_patterns.sh @@ -30,7 +30,7 @@ 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 -result=$(pr_changed_files --all-match "docs/read me.md" "docs/guide with spaces.md" "docs/special\!\@\#\$chars.md") +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] Changes outside pattern @@ -43,7 +43,7 @@ result=$(pr_changed_files --all-match "docs/*") assert_output "false" "$result" "Should return false when changes exist outside patterns" # [Test] Multiple patterns, all matching -result=$(pr_changed_files --all-match "docs/*" "src/swift/main with spaces.swift" "src/swift/special\!\@\#\$chars.swift") +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" echo "✅ All-match pattern tests passed" From 40cac254efda7e175b42f9ad1e0fb66d550236e6 Mon Sep 17 00:00:00 2001 From: Olivier Halligon Date: Tue, 11 Feb 2025 21:02:35 +0100 Subject: [PATCH 27/28] Improve tests with special characters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Using single quotes to make it easier to understand the use of special characters—including in the code creating the test files --- .../test_all_match_patterns.sh | 26 +++++++++++++------ 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/tests/pr_changed_files/test_all_match_patterns.sh b/tests/pr_changed_files/test_all_match_patterns.sh index efc1319b..2314c94e 100755 --- a/tests/pr_changed_files/test_all_match_patterns.sh +++ b/tests/pr_changed_files/test_all_match_patterns.sh @@ -17,11 +17,11 @@ export BUILDKITE_PULL_REQUEST_BASE_BRANCH="base" # Initialize the repository init_test_repo "$repo_path" -# Create test files +# Create test files (using single quotes to avoid special chars being interpreted by the shell) mkdir -p docs src/swift -echo "doc" > "docs/read me.md" -echo "doc2" > "docs/guide with spaces.md" -echo "doc3" > "docs/special\!\@\#\$chars.md" +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" @@ -30,12 +30,17 @@ 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 -result=$(pr_changed_files --all-match 'docs/read me.md' 'docs/guide with spaces.md' 'docs/special\\!\\@\\#\$chars.md') +# 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" +echo "swift" > 'src/swift/main with spaces.swift' +echo "swift" > 'src/swift/special!\@#*$chars.swift' git add . git commit -m "Add swift file" @@ -43,7 +48,12 @@ result=$(pr_changed_files --all-match "docs/*") assert_output "false" "$result" "Should return false when changes exist outside patterns" # [Test] Multiple patterns, all matching -result=$(pr_changed_files --all-match 'docs/*' 'src/swift/main with spaces.swift' 'src/swift/special\\!\\@\\#\$chars.swift') +# 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" From ee11f586d3a8d89b3355659bf42698d46c7b99f8 Mon Sep 17 00:00:00 2001 From: Olivier Halligon Date: Tue, 11 Feb 2025 21:12:22 +0100 Subject: [PATCH 28/28] 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 --- .../test_any_match_patterns.sh | 26 +++++++++++-------- tests/pr_changed_files/test_edge_cases.sh | 10 +++---- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/tests/pr_changed_files/test_any_match_patterns.sh b/tests/pr_changed_files/test_any_match_patterns.sh index 6790caad..f3f5f442 100755 --- a/tests/pr_changed_files/test_any_match_patterns.sh +++ b/tests/pr_changed_files/test_any_match_patterns.sh @@ -17,33 +17,37 @@ export BUILDKITE_PULL_REQUEST_BASE_BRANCH="base" # Initialize the repository init_test_repo "$repo_path" -# Create test files +# 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 +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") +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") +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") +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") +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/*") +result=$(pr_changed_files --any-match 'docs/*') assert_output "true" "$result" "Should match directory patterns" # [Test] Exact pattern matching @@ -51,7 +55,7 @@ echo "swiftfile" > swiftfile.txt git add swiftfile.txt git commit -m "Add file with swift in name" -result=$(pr_changed_files --any-match "*.swift") +result=$(pr_changed_files --any-match '*.swift') assert_output "true" "$result" "Should only match exact patterns" echo "✅ Any-match pattern tests passed" diff --git a/tests/pr_changed_files/test_edge_cases.sh b/tests/pr_changed_files/test_edge_cases.sh index 28771c61..7ad1bb8e 100755 --- a/tests/pr_changed_files/test_edge_cases.sh +++ b/tests/pr_changed_files/test_edge_cases.sh @@ -41,9 +41,9 @@ output=$(pr_changed_files --any-match "*.txt" --all-match "*.md" 2>&1 || true) assert_output "Error: either specify --all-match or --any-match; cannot specify both" "$output" "Should fail with correct error when using mutually exclusive options" # [Test] Files with spaces and special characters -mkdir -p "folder with spaces/nested\!\@\#\$folder" -echo "test" > "folder with spaces/file with spaces.txt" -echo "test" > "folder with spaces/nested\!\@\#\$folder/file_with_\!\@\#.txt" +mkdir -p 'folder with spaces/nested!\@*#$folder' +echo "test" > 'folder with spaces/file with spaces.txt' +echo "test" > 'folder with spaces/nested!\@*#$folder/file_with_!@*#$chars.txt' git add . git commit -m "Add files with special characters" @@ -51,10 +51,10 @@ result=$(pr_changed_files) assert_output "true" "$result" "Should handle files with spaces and special characters" # [Test] Pattern matching with spaces and special characters -result=$(pr_changed_files --any-match "*spaces.txt") +result=$(pr_changed_files --any-match '*spaces.txt') assert_output "true" "$result" "Should match files with special characters in path" -result=$(pr_changed_files --all-match "folder with spaces/*") +result=$(pr_changed_files --all-match 'folder with spaces/*') assert_output "true" "$result" "Should handle directory patterns with spaces" # [Test] No changes between branches