v10.1.1/ci fix #138
v10.1.1/ci fix #138
Conversation
WalkthroughA GitHub Actions workflow configuration was modified to add diagnostic verbosity to Linux and Windows test steps. A concise permission-fix command was expanded into a comprehensive diagnostic block that outputs system information, environment context, and performs detailed executable analysis. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci-pipeline.yml(3 hunks)
⏰ 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). (4)
- GitHub Check: Agent
- GitHub Check: call-build (Debug) / 🛠️ Build
- GitHub Check: call-build (Release) / 🛠️ Build
- GitHub Check: Analyze (csharp)
🔇 Additional comments (2)
.github/workflows/ci-pipeline.yml (2)
65-82: Confirmverbosity-levelis a supported input forcodebeltnet/jobs-dotnet-test@v3(otherwise it’ll be ignored or fail).If that reusable workflow doesn’t define
verbosity-level, this change won’t do what you expect (or could break the workflow).
83-100: Same concern on Windows: verifyverbosity-levelis supported and maps todotnet test -v diag.Also consider whether
diagnosticis too noisy for PR runs (log size / retention), but that’s a policy choice.
| - name: Fix Linux test apphost permissions | ||
| run: | | ||
| echo "Ensuring xUnit v3 test apphosts are executable on Linux..." | ||
| echo "Configuration: ${{ matrix.configuration }}" | ||
| # Dump a bit of info for debugging | ||
| find . -path "*/bin/${{ matrix.configuration }}/net*/*" -maxdepth 1 -type f -name "*Tests*" -print || true | ||
| # Actually fix permissions | ||
| find . -path "*/bin/${{ matrix.configuration }}/net*/*" -type f -name "*Tests*" -exec chmod +x {} + | ||
| echo "Permissions fixed." | ||
| set -euo pipefail | ||
| echo "=== Context ===" | ||
| echo "Runner: $RUNNER_OS / $RUNNER_ARCH" | ||
| echo "Configuration: ${{ inputs.configuration }}" | ||
| echo "Workspace: $GITHUB_WORKSPACE" | ||
| echo "PWD: $(pwd)" | ||
| echo "Event: $GITHUB_EVENT_NAME" | ||
| echo "Ref: $GITHUB_REF" | ||
| echo "SHA: $GITHUB_SHA" | ||
| echo | ||
| echo "=== .NET info ===" | ||
| dotnet --info || true | ||
| echo | ||
| echo "=== Git state ===" | ||
| git rev-parse HEAD || true | ||
| git status --porcelain || true | ||
| git rev-parse --is-shallow-repository || true | ||
| echo | ||
| # Paths we care about | ||
| BIN_GLOB="*/bin/${{ inputs.configuration }}/net*/*" | ||
| OBJ_GLOB="*/obj/${{ inputs.configuration }}/net*/*" | ||
| echo "=== Brute-force chmod (bin + obj) ===" | ||
| find . -type f \( -path "$BIN_GLOB" -o -path "$OBJ_GLOB" \) -exec chmod a+x {} + 2>/dev/null || true | ||
| echo "chmod completed (errors ignored)." | ||
| echo | ||
| echo "=== Mount options (look for noexec) ===" | ||
| # If binaries live on a noexec mount, chmod won't help. | ||
| mount | sed -n '1,200p' || true | ||
| echo | ||
| echo "=== Candidate executables (top 200) ===" | ||
| # Show what we might execute; exclude obvious managed files | ||
| find . -type f -path "$BIN_GLOB" \ | ||
| ! -name "*.dll" ! -name "*.pdb" ! -name "*.json" ! -name "*.xml" \ | ||
| -printf "%m %u:%g %s %p\n" | head -n 200 || true | ||
| echo | ||
| echo "=== Likely xUnit / test hosts (if present) ===" | ||
| # These names vary; do not rely on just *Tests* | ||
| find . -type f -path "$BIN_GLOB" \( \ | ||
| -name "testhost*" -o \ | ||
| -name "*xunit*" -o \ | ||
| -name "*Tests*" -o \ | ||
| -name "*.runsettings" \ | ||
| \) -printf "%m %u:%g %s %p\n" | head -n 200 || true | ||
| echo | ||
| echo "=== Deep diagnostics for any 'testhost' or apphost candidates ===" | ||
| # For each likely executable, show the facts that explain 'permission denied' vs 'exec format error' | ||
| while IFS= read -r f; do | ||
| echo "--- $f ---" | ||
| ls -la "$f" || true | ||
| # Identify file type and architecture | ||
| file -L "$f" || true | ||
| # If it's an ELF binary, show its dynamic interpreter and linked libs (exec format errors often show up here) | ||
| if file -L "$f" | grep -q "ELF"; then | ||
| echo "readelf -l (interpreter):" | ||
| readelf -l "$f" 2>/dev/null | sed -n '1,80p' || true | ||
| echo "ldd (dependencies):" | ||
| ldd "$f" 2>/dev/null || true | ||
| fi | ||
| # If it's a script, CRLF in the shebang can cause 'Exec format error' | ||
| if head -c 2 "$f" 2>/dev/null | grep -q "#!"; then | ||
| echo "shebang:" | ||
| head -n 1 "$f" | cat -A || true | ||
| fi | ||
| echo | ||
| done < <( | ||
| find . -type f -path "$BIN_GLOB" \( \ | ||
| -name "testhost*" -o \ | ||
| -name "*xunit*" -o \ | ||
| -name "*Tests*" \ | ||
| \) | head -n 50 | ||
| ) || true | ||
| echo "=== Done diagnostics step ===" | ||
| shell: bash |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Let me verify the exact lines using the matrix and inputs context
sed -n '9,50p' .github/workflows/ci-pipeline.yml | cat -nRepository: codebeltnet/cuemon
Length of output: 1418
🏁 Script executed:
# Verify the exact usage of configuration variable in the diagnostic step
sed -n '136,224p' .github/workflows/ci-pipeline.yml | grep -n "configuration\|ldd"Repository: codebeltnet/cuemon
Length of output: 343
🏁 Script executed:
# Find the job definition that contains the diagnostic step
sed -n '100,300p' .github/workflows/ci-pipeline.yml | head -n 200Repository: codebeltnet/cuemon
Length of output: 6397
🏁 Script executed:
# Search for the test job and its matrix
grep -n "test:" .github/workflows/ci-pipeline.ymlRepository: codebeltnet/cuemon
Length of output: 104
🏁 Script executed:
# Get the full structure around lines 100-250 to see job definitions
sed -n '80,250p' .github/workflows/ci-pipeline.yml | cat -nRepository: codebeltnet/cuemon
Length of output: 7281
Use matrix.configuration instead of undefined inputs.configuration in the diagnostic step. The integration_test job defines and uses matrix.configuration throughout (download artifacts, test execution), but this diagnostic step incorrectly references ${{ inputs.configuration }}, which is undefined in this context, causing the chmod and find globs to fail.
- echo "Configuration: ${{ inputs.configuration }}"
+ echo "Configuration: ${{ matrix.configuration }}"
...
- BIN_GLOB="*/bin/${{ inputs.configuration }}/net*/*"
- OBJ_GLOB="*/obj/${{ inputs.configuration }}/net*/*"
+ BIN_GLOB="*/bin/${{ matrix.configuration }}/net*/*"
+ OBJ_GLOB="*/obj/${{ matrix.configuration }}/net*/*"Replace ldd with non-executing ELF inspection tools. The ldd command executes code from binaries to resolve dependencies, which is unnecessary in a CI environment and presents an avoidable security risk. Use readelf -d and objdump -p instead to inspect NEEDED sections without execution.
- echo "ldd (dependencies):"
- ldd "$f" 2>/dev/null || true
+ echo "readelf -d (NEEDED):"
+ readelf -d "$f" 2>/dev/null | sed -n '1,120p' || true
+ echo "objdump -p (NEEDED):"
+ objdump -p "$f" 2>/dev/null | sed -n '1,120p' || true🤖 Prompt for AI Agents
.github/workflows/ci-pipeline.yml around lines 136 to 224: the diagnostic step
incorrectly references the undefined inputs.configuration and calls ldd (which
executes code); update all occurrences of ${{ inputs.configuration }} to ${{
matrix.configuration }} (including BIN_GLOB and OBJ_GLOB) so the find/chmod
globs work, and remove the ldd "$f" call — replace it with non-executing
inspection commands such as readelf -d "$f" and objdump -p "$f" (run both
conditionally for ELF files) to show NEEDED/DT entries without executing the
binary.
There was a problem hiding this comment.
Pull request overview
This pull request enhances CI pipeline diagnostics by increasing logging verbosity and adding comprehensive diagnostic capabilities for Linux test executable issues. The changes aim to help troubleshoot "permission denied" and "exec format error" problems when running xUnit v3 tests on Linux.
Key changes:
- Added
verbosity-level: diagnosticto both Linux and Windows test jobs for more detailed build/test output - Replaced a simple permission-fix script with an extensive diagnostic shell script that examines runner context, .NET environment, file permissions, mount options, and binary characteristics
| -name "*xunit*" -o \ | ||
| -name "*Tests*" \ | ||
| \) | head -n 50 | ||
| ) || true |
There was a problem hiding this comment.
The process substitution syntax done < <(find ...) on line 215 is a bash-specific feature. While the script explicitly sets shell: bash on line 224, it's good that this is specified. However, the set -euo pipefail on line 138 combined with the || true on line 221 means that if the process substitution or the while loop fails, it will be silently ignored. Consider whether you want to catch failures in the diagnostic loop itself, or if silent failure is acceptable for this diagnostic-only step.
| ) || true | |
| ) |
| echo "=== Context ===" | ||
| echo "Runner: $RUNNER_OS / $RUNNER_ARCH" | ||
| echo "Configuration: ${{ inputs.configuration }}" |
There was a problem hiding this comment.
The script references inputs.configuration but this job uses matrix.configuration instead. The integration_test job doesn't have access to inputs.configuration (which is only available in workflow_dispatch events), so this will evaluate to an empty string, causing the script to search for paths like */bin//net*/* which won't match the intended directories. This should be changed to matrix.configuration to match the job's configuration matrix.
| echo "Configuration: ${{ inputs.configuration }}" | |
| echo "Configuration: ${{ matrix.configuration }}" |
| BIN_GLOB="*/bin/${{ inputs.configuration }}/net*/*" | ||
| OBJ_GLOB="*/obj/${{ inputs.configuration }}/net*/*" |
There was a problem hiding this comment.
The BIN_GLOB and OBJ_GLOB variables reference inputs.configuration which is not available in this job context. The integration_test job uses matrix.configuration instead. These paths will resolve incorrectly (e.g., */bin//net*/*), causing the find commands and chmod operations throughout the script to fail to locate the actual test binaries. Change both occurrences to matrix.configuration.
| BIN_GLOB="*/bin/${{ inputs.configuration }}/net*/*" | |
| OBJ_GLOB="*/obj/${{ inputs.configuration }}/net*/*" | |
| BIN_GLOB="*/bin/${{ matrix.configuration }}/net*/*" | |
| OBJ_GLOB="*/obj/${{ matrix.configuration }}/net*/*" |
| echo "=== Brute-force chmod (bin + obj) ===" | ||
| find . -type f \( -path "$BIN_GLOB" -o -path "$OBJ_GLOB" \) -exec chmod a+x {} + 2>/dev/null || true |
There was a problem hiding this comment.
Applying executable permissions to ALL files in bin and obj directories (chmod a+x on every file) is overly broad and could pose a security risk. Many files in these directories (like .dll, .pdb, .json, .xml, configuration files) should never be executable. This could inadvertently make data files or configuration files executable. Consider being more selective by only applying executable permissions to specific patterns that are actually test host executables (e.g., files named testhost, testhost.exe, files matching the test project names without extensions, or using file type detection first).
| echo "=== Brute-force chmod (bin + obj) ===" | |
| find . -type f \( -path "$BIN_GLOB" -o -path "$OBJ_GLOB" \) -exec chmod a+x {} + 2>/dev/null || true | |
| echo "=== Selective chmod (testhost executables in bin + obj) ===" | |
| find . \( -path "$BIN_GLOB" -o -path "$OBJ_GLOB" \) -type f \( -name "testhost" -o -name "testhost.exe" \) -exec chmod a+x {} + 2>/dev/null || true |
| echo "=== Deep diagnostics for any 'testhost' or apphost candidates ===" | ||
| # For each likely executable, show the facts that explain 'permission denied' vs 'exec format error' | ||
| while IFS= read -r f; do | ||
| echo "--- $f ---" | ||
| ls -la "$f" || true | ||
| # Identify file type and architecture | ||
| file -L "$f" || true | ||
| # If it's an ELF binary, show its dynamic interpreter and linked libs (exec format errors often show up here) | ||
| if file -L "$f" | grep -q "ELF"; then | ||
| echo "readelf -l (interpreter):" | ||
| readelf -l "$f" 2>/dev/null | sed -n '1,80p' || true | ||
| echo "ldd (dependencies):" | ||
| ldd "$f" 2>/dev/null || true | ||
| fi | ||
| # If it's a script, CRLF in the shebang can cause 'Exec format error' | ||
| if head -c 2 "$f" 2>/dev/null | grep -q "#!"; then | ||
| echo "shebang:" | ||
| head -n 1 "$f" | cat -A || true | ||
| fi | ||
| echo | ||
| done < <( | ||
| find . -type f -path "$BIN_GLOB" \( \ | ||
| -name "testhost*" -o \ | ||
| -name "*xunit*" -o \ | ||
| -name "*Tests*" \ | ||
| \) | head -n 50 | ||
| ) || true |
There was a problem hiding this comment.
The diagnostic script processes up to 50 test binaries with extensive checks (file, readelf, ldd commands) which could significantly increase CI run time, especially with many test projects. While this is valuable for debugging, consider adding a condition to only run this extensive diagnostic when tests are actually failing, or make it configurable via an input parameter. This would reduce noise and CI time in successful runs while still providing diagnostics when needed.
| find . -type f -path "$BIN_GLOB" \ | ||
| ! -name "*.dll" ! -name "*.pdb" ! -name "*.json" ! -name "*.xml" \ | ||
| -printf "%m %u:%g %s %p\n" | head -n 200 || true |
There was a problem hiding this comment.
The find command uses -printf which is a GNU findutils extension. While Ubuntu runners should support this, the use of GNU-specific options could cause portability issues if the script is ever adapted for other platforms. The same applies to sed -n '1,200p' which could be replaced with head -n 200. Consider documenting this GNU dependency or using more portable alternatives like find ... -ls or find ... | xargs ls -la for broader compatibility.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #138 +/- ##
=======================================
Coverage 80.37% 80.37%
=======================================
Files 595 595
Lines 18485 18485
Branches 1895 1895
=======================================
Hits 14857 14857
Misses 3562 3562
Partials 66 66 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
gimlichael
requested a deployment
to
Production
GitHub Actions
Waiting
This pull request enhances the CI pipeline diagnostics and verbosity for .NET builds and test jobs, with a particular focus on improving troubleshooting for Linux test executables. The main changes increase the level of logging and add comprehensive diagnostic steps to help debug issues related to test binary execution, especially around permissions and environment context.
CI pipeline diagnostics and verbosity improvements:
Increased the verbosity level to
diagnosticfor both the build and test jobs, providing more detailed logs during CI runs. [1] [2]Replaced the simple Linux test apphost permissions fix step with an extensive diagnostic script. This script now:
chmod a+x) to bothbinandobjdirectories for the relevant configuration.noexecmount options that could prevent execution.Summary by CodeRabbit
Release Notes
This release contains no user-facing changes. Updates were made exclusively to internal CI/CD infrastructure and testing diagnostics to improve development workflow efficiency.
✏️ Tip: You can customize this high-level summary in your review settings.