fix: restrict markdown tests to ubuntu runner #3317
Conversation
anshgoyalevil
requested review from
derberg,
akshatnema,
magicmatatjahu,
Mayaleeeee and
asyncapi-bot-eve
as code owners
WalkthroughThe pull request modifies the Node.js workflow file for pull request testing by changing the event trigger from Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
| @@ -76,7 +76,7 @@ jobs: | |||
| run: npm run generate:assets --if-present | |||
|
|
|||
| # Run the test:md script and capture output | |||
| - if: steps.packagejson.outputs.exists == 'true' | |||
| - if: ${{ steps.packagejson.outputs.exists == 'true' && matrix.os == 'ubuntu-latest' }} | |||
There was a problem hiding this comment.
Why are we restricting the tests to ubuntu os only?
There was a problem hiding this comment.
Because we are using the commands like sed to parse the errors, windows bash doesn't have that support
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
.github/workflows/if-nodejs-pr-testing.yml (1)
Line range hint
79-87: Consider adding error handling for the test:md script.While the implementation works, it might silently fail if the
test:mdscript exits with an error.Consider adding error handling:
- if: ${{ steps.packagejson.outputs.exists == 'true' && matrix.os == 'ubuntu-latest' }} name: Run markdown checks id: markdown_check run: | + set -eo pipefail ERRORS=$(npm run test:md | sed -n '/Errors in file/,$p') echo "markdown_output<<EOF" >> $GITHUB_OUTPUT echo "$ERRORS" >> $GITHUB_OUTPUT echo "EOF" >> $GITHUB_OUTPUT🧰 Tools
🪛 actionlint
82-82: shellcheck reported issue in this script: SC2129:style:2:1: Consider using { cmd1; cmd2; } >> file instead of individual redirects
(shellcheck)
82-82: shellcheck reported issue in this script: SC2086:info:2:32: Double quote to prevent globbing and word splitting
(shellcheck)
82-82: shellcheck reported issue in this script: SC2086:info:3:19: Double quote to prevent globbing and word splitting
(shellcheck)
82-82: shellcheck reported issue in this script: SC2086:info:4:15: Double quote to prevent globbing and word splitting
(shellcheck)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- .github/workflows/if-nodejs-pr-testing.yml (3 hunks)
🧰 Additional context used
🔇 Additional comments (2)
.github/workflows/if-nodejs-pr-testing.yml (2)
Line range hint
79-87: LGTM! Markdown checks are now properly restricted to Ubuntu.The implementation correctly restricts the markdown checks to Ubuntu, addressing the
sedcommand compatibility issue. The error capture and output handling are well implemented.🧰 Tools
🪛 actionlint
82-82: shellcheck reported issue in this script: SC2129:style:2:1: Consider using { cmd1; cmd2; } >> file instead of individual redirects
(shellcheck)
82-82: shellcheck reported issue in this script: SC2086:info:2:32: Double quote to prevent globbing and word splitting
(shellcheck)
82-82: shellcheck reported issue in this script: SC2086:info:3:19: Double quote to prevent globbing and word splitting
(shellcheck)
82-82: shellcheck reported issue in this script: SC2086:info:4:15: Double quote to prevent globbing and word splitting
(shellcheck)
Line range hint
90-110: LGTM! PR comment handling is properly implemented.The implementation correctly handles both the creation and deletion of PR comments based on markdown check results, and is properly restricted to Ubuntu runner.
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-3317--asyncapi-website.netlify.app/ |
This PR restricts markdown tests to Ubuntu Runner since we cannot make use of commands like
sedin Windows bash.Summary by CodeRabbit