Skip to content

fix: Split ALL_CHANGED_FILES on any whitespace #1566

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom
Merged
HofmeisterAn merged 1 commit into from
Nov 6, 2025
Merged

fix: Split ALL_CHANGED_FILES on any whitespace #1566

HofmeisterAn merged 1 commit into from
Nov 6, 2025

Conversation

@HofmeisterAn
Copy link
Collaborator

@HofmeisterAn HofmeisterAn commented Nov 6, 2025

What does this PR do?

This PR fixes an issue in the incremental build and test setup. The GH Action step that collects all changed files doesn't use \n as a separator. It uses spaces. I updated the split implementation to split on any whitespace.

Previously, the allChangedFiles variable only contained a single string with all changed files, which caused the wrong test configurations to run. @digital88, we should double-check whether filenames with spaces might break this. According to the docs, the GH Action step lets you configure the separator, and the default is supposed to be \n, but that doesn't seem to be the case (probably I misunderstood the docs).

Why is it important?

To select and run the appropriate test projects.

Related issues

-

@HofmeisterAn HofmeisterAn added the bug Something isn't working label Nov 6, 2025
@netlify
Copy link

netlify bot commented Nov 6, 2025 •
edited
Loading

✅ Deploy Preview for testcontainers-dotnet ready!

Name Link
🔨 Latest commit 815d75c
🔍 Latest deploy log https://app.netlify.com/projects/testcontainers-dotnet/deploys/690cfcadc67cbd000882a670
😎 Deploy Preview https://deploy-preview-1566--testcontainers-dotnet.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@coderabbitai
Copy link

coderabbitai bot commented Nov 6, 2025 •
edited
Loading

Caution

Review failed

The head commit changed during the review from 4118c3b to 815d75c.

Summary by CodeRabbit

  • Chores
    • Updated CI/CD pipeline configuration to trigger builds when documentation and examples are modified.
    • Adjusted file change detection logic in test project filtering.

Walkthrough

The CI/CD workflow now responds to changes in docs/** and examples/** directories by removing path-based exclusions. Simultaneously, a PowerShell script's file parsing logic changes from newline-only splitting to whitespace-based splitting for processing changed files.

Changes

Cohort / File(s) Summary
CI/CD Workflow Configuration
.github/workflows/cicd.yml		 Removed path filtering exclusions for docs/** and examples/** directories, causing the workflow to trigger on changes to these paths
File Parsing Logic
.github/scripts/Filter-TestProjects.ps1		 Modified ALL_CHANGED_FILES splitting behavior from newline-only (\n) to whitespace-based splitting (spaces, tabs, newlines)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5–10 minutes

  • Attention areas:
    • Verify the whitespace-based splitting in the PowerShell script doesn't inadvertently parse file paths differently (especially paths containing spaces or special characters)
    • Confirm that removing docs/examples path exclusions aligns with intended CI trigger scope and won't cause unnecessary workflow executions
    • Ensure the splitting change doesn't break compatibility with how ALL_CHANGED_FILES is formatted by the calling workflow

Possibly related PRs

Poem

🐰 Path filters vanish, docs now wake the dance,
Whitespace splits the files with new expanse—
CI pipelines quicken, no docs left behind,
A rabbit's refactor of process and mind! 🌟

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and accurately summarizes the main change: splitting ALL_CHANGED_FILES on any whitespace instead of just newlines.
Description check ✅ Passed The PR description covers both mandatory sections (What and Why), provides context about the GH Action behavior, and notes potential edge cases with filenames containing spaces.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot]
coderabbitai bot reviewed Nov 6, 2025
View reviewed changes
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
.github/scripts/Filter-TestProjects.ps1 (1)

120-120: Minor: Redundant regex pattern.

The character class [\s\t\n] is redundant because \s already matches all whitespace characters including tabs and newlines.

If you proceed with whitespace splitting (though see my critical issue comment above), simplify to:

-        $script:allChangedFiles = $env:ALL_CHANGED_FILES -Split "[\s\t\n]"
+        $script:allChangedFiles = $env:ALL_CHANGED_FILES -Split '\s+'

The + quantifier also prevents creating empty string entries from consecutive whitespace characters.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d4c61d1 and 815d75c.

📒 Files selected for processing (2)
  • .github/scripts/Filter-TestProjects.ps1 (1 hunks)
  • .github/workflows/cicd.yml (0 hunks)
💤 Files with no reviewable changes (1)
  • .github/workflows/cicd.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (127)
  • GitHub Check: ci (Testcontainers.BigQuery, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
  • GitHub Check: analyze (csharp)
  • GitHub Check: ci (Testcontainers.Oracle21, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Oracle18, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Oracle11, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.OpenSearch, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Oracle, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Ollama, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.BigQuery, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
  • GitHub Check: analyze (csharp)
  • GitHub Check: ci (Testcontainers.Oracle21, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Oracle18, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Oracle11, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.OpenSearch, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Oracle, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Ollama, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.BigQuery, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
  • GitHub Check: analyze (csharp)
  • GitHub Check: ci (Testcontainers.Oracle21, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Oracle18, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Oracle11, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.OpenSearch, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Oracle, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Ollama, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
  • GitHub Check: analyze (csharp)
  • GitHub Check: ci (Testcontainers.Oracle21, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Oracle18, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Oracle11, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.OpenSearch, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Oracle, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Ollama, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ClickHouse, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
  • GitHub Check: analyze (csharp)
  • GitHub Check: ci (Testcontainers.Oracle21, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Oracle18, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Oracle11, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.OpenSearch, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Oracle, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Ollama, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.CockroachDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ClickHouse, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
  • GitHub Check: analyze (csharp)
  • GitHub Check: ci (Testcontainers.Oracle21, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Oracle18, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Oracle11, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.OpenSearch, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Oracle, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Ollama, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.CockroachDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ClickHouse, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
  • GitHub Check: analyze (csharp)
  • GitHub Check: ci (Testcontainers.Oracle21, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Oracle18, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Oracle11, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.OpenSearch, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Oracle, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Ollama, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.CockroachDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ClickHouse, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
  • GitHub Check: analyze (csharp)
  • GitHub Check: ci (Testcontainers.Oracle21, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Oracle18, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Oracle11, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.OpenSearch, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Oracle, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Ollama, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.CockroachDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ClickHouse, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
  • GitHub Check: analyze (csharp)
  • GitHub Check: ci (Testcontainers.Oracle21, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Oracle18, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Oracle11, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.OpenSearch, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Oracle, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Ollama, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.CockroachDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ClickHouse, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
  • GitHub Check: analyze (csharp)
  • GitHub Check: ci (Testcontainers.Oracle21, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Oracle18, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Oracle11, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.OpenSearch, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Oracle, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Ollama, ubuntu-24.04)

@HofmeisterAn HofmeisterAn force-pushed the bugfix/collect-all-changed-files branch from 4118c3b to 815d75c Compare November 6, 2025 19:53
@HofmeisterAn HofmeisterAn merged commit a652a9e into develop Nov 6, 2025
231 of 273 checks passed
@HofmeisterAn HofmeisterAn deleted the bugfix/collect-all-changed-files branch November 6, 2025 20:24
@digital88
Copy link
Contributor

digital88 commented Nov 7, 2025 •
edited
Loading

@HofmeisterAn
Maybe something like .Split([Environment]::NewLine) would be 'safer'? WDYT? Or add [Environment]::NewLine as additional argument to array of \s\t\n.

I beleive this is what we need and it's indeed equals to " " (space) by default:
image

@HofmeisterAn
Copy link
Collaborator Author

HofmeisterAn commented Nov 8, 2025

Yep, I noticed that too after going through the docs again. I even tried it, but I ran into another issue, which was probably a misconfiguration with the string split. I haven't had the time to look into it yet, so I just stuck with this quick fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

Assignees

No one assigned

Labels

bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@HofmeisterAn @digital88