refactor: streamline output handling for GITHUB_OUTPUT in workflows #404
Conversation
- Add a check to ensure captured stdout is not empty - Add steps to capture and verify multiline stdout output - Add verification that specific lines and the username are present in captured output - Add steps to handle and verify stdout containing special characters and file paths Signed-off-by: appleboy <appleboy.tw@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes a critical bug where stdout was being duplicated when capture_stdout: true was enabled. The issue was introduced in a previous refactoring that wrapped the output capture logic in a subshell, causing the output to be written twice to GITHUB_OUTPUT.
Changes:
- Removed the subshell wrapper in entrypoint.sh that was causing output duplication
- Added comprehensive tests to verify stdout capture works correctly with empty checks, multiline output, and special characters
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| entrypoint.sh | Simplified stdout capture by removing the problematic subshell and writing delimiters directly to GITHUB_OUTPUT |
| .github/workflows/main.yml | Added three new test cases to validate stdout capture functionality: empty output check, multiline output verification, and special characters handling |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - name: check special characters output | ||
| run: | | ||
| echo "stdout: ${{ steps.stdout-with-special-chars.outputs.stdout }}" | ||
| if ! echo "${{ steps.stdout-with-special-chars.outputs.stdout }}" | grep -q "special chars"; then | ||
| echo "Error: special characters test failed" | ||
| exit 1 | ||
| fi | ||
| if ! echo "${{ steps.stdout-with-special-chars.outputs.stdout }}" | grep -q "/home/user/test"; then | ||
| echo "Error: path not found in output" | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
The test verifies special characters and file paths but doesn't verify the JSON output line. Consider adding a check to ensure the JSON line with escaped quotes is captured correctly, as this is an important edge case for validating special character handling.
| - name: check multiline output | ||
| run: | | ||
| echo "stdout: ${{ steps.stdout-multiline.outputs.stdout }}" | ||
| # Check if all lines are present | ||
| if ! echo "${{ steps.stdout-multiline.outputs.stdout }}" | grep -q "Line 1"; then | ||
| echo "Error: 'Line 1' not found in output" | ||
| exit 1 | ||
| fi | ||
| if ! echo "${{ steps.stdout-multiline.outputs.stdout }}" | grep -q "Line 2"; then | ||
| echo "Error: 'Line 2' not found in output" | ||
| exit 1 | ||
| fi | ||
| if ! echo "${{ steps.stdout-multiline.outputs.stdout }}" | grep -q "Line 3"; then | ||
| echo "Error: 'Line 3' not found in output" | ||
| exit 1 | ||
| fi | ||
| if ! echo "${{ steps.stdout-multiline.outputs.stdout }}" | grep -q "linuxserver.io"; then | ||
| echo "Error: username not found in output" | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
Consider adding a test that explicitly verifies the output is not duplicated. Since this PR fixes issue #397 about duplicated output, it would be valuable to have a test that counts the occurrences of specific output lines to ensure they appear exactly once, not twice. For example, you could check that "Line 1" appears exactly once in the multiline output test.
- Add a step to verify that lines "Line 1", "Line 2", and "Line 3" each appear exactly once in the multiline output - Fail the workflow if any line is missing or duplicated - Confirm successful validation with a message when no duplicates are found Signed-off-by: appleboy <appleboy.tw@gmail.com>
1 participant
fix #403
fix #397
Signed-off-by: appleboy appleboy.tw@gmail.com