chore: Ability to run PR automations every time PR updates#35476
chore: Ability to run PR automations every time PR updates#35476
Conversation
WalkthroughThe recent modifications to the GitHub Actions workflow enhance control flow by introducing a new job called Changes
Poem
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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
github-actions
bot
added
the
skip-changelog
hetunandu
added
ok-to-test
github-actions
bot
added
the
skip-changelog
![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
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- .github/workflows/pr-automation.yml (3 hunks)
Additional comments not posted (9)
.github/workflows/pr-automation.yml (9)
11-12: Good job defining the job runner.The
check-labeljob is correctly set to run onubuntu-latest. This is a common and reliable choice.
14-15: Checkout code step is essential.Ensuring the code is checked out is a necessary step for any subsequent operations.
17-31: Label check script is well-structured.The script correctly checks for the "ok-to-test" label and sets the output accordingly. This ensures that the workflow only proceeds if the label is present.
33-35: Correct dependency and conditional check.The
parse-tagsjob correctly depends on thecheck-labeljob and includes a conditional check to ensure it only runs if the "ok-to-test" label is present.
51-51: Essential step to checkout the head commit.Ensuring the head commit of the branch is checked out is crucial for the subsequent steps to operate on the correct codebase.
Line range hint
54-59: Reading tags from PR description is well-implemented.Using a script to read tags from the PR description ensures that the workflow can dynamically adjust based on the tags present.
Line range hint
62-72: Dynamic matrix allocation based on tags.The step correctly checks for the
@tag.Alltag and adjusts the matrix size accordingly. This ensures efficient resource allocation.
Line range hint
75-88: Informative test response step.Updating the PR with a link to the workflow run provides clear feedback to the developers about the test status.
96-96: Correct dependency and conditional check for Cypress tests.The
perform-testjob correctly depends on theparse-tagsjob and includes a conditional check to ensure it only runs if the previous steps are successful.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- .github/workflows/pr-automation.yml (3 hunks)
Additional comments not posted (6)
.github/workflows/pr-automation.yml (6)
11-14: LGTM! The job structure is well-defined.The
check-labeljob is well-structured with appropriate steps and outputs.
36-38: LGTM! The job modifications are well-implemented.The
parse-tagsjob modifications ensure efficient workflow execution based on label verification.
Line range hint
99-104:
LGTM! The job structure is well-defined.The
perform-testjob is well-structured with appropriate dependencies and steps for running Cypress tests.
Line range hint
99-104:
Verify the dependency and Cypress test execution.Ensure that the dependency on
parse-tagsis correctly implemented and that the Cypress tests run as expected.
38-38: Verify the conditional execution logic.Ensure that the conditional execution logic correctly evaluates the output from the
check-labeljob.
19-34: Verify the correctness of the script logic.The script correctly checks for the "ok-to-test" label and sets the output variable. Ensure that the label checking logic works as expected.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- .github/workflows/pr-automation.yml (4 hunks)
Files skipped from review as they are similar to previous changes (1)
- .github/workflows/pr-automation.yml
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- .github/workflows/pr-automation.yml (3 hunks)
Additional comments not posted (3)
.github/workflows/pr-automation.yml (3)
11-35: Great job defining thecheckTestLabeljob!The job correctly checks for the "ok-to-test" label and sets the
ok_to_testoutput variable based on its presence. The logic and syntax are correct.
37-57: Well-structuredmark-stalejob!The job correctly depends on the
checkTestLabeljob and runs only if theok_to_testoutput isfalse. The script for marking the Cypress status as stale is used appropriately.
Line range hint
59-121:
Excellentparse-tagsjob definition!The job correctly depends on the
checkTestLabeljob and runs only if theok_to_testoutput istrue. The steps for checking out the code, reading tags from the PR description, and updating the PR with run details are well-structured and logically sound.
|
Isn't this wasteful of CI resources @hetunandu ? We want to encourage developers to commit to remote as often as possible, even with broken builds. Why are we opting to occupy our runners unnecessarily? Perhaps we can think of a CLI option to mock the label removal and addition instead? |
|
@nidhi-nair this PR would only reduce the steps to test in my opinion. It solves these problems
I am not sure how a CLI process to remove and add back the label would be any better? |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- .github/workflows/pr-automation.yml (3 hunks)
Additional comments not posted (3)
.github/workflows/pr-automation.yml (3)
15-37: Great job on implementing thecheckTestLabeljob!This job effectively checks for the "ok-to-test" label and sets the output correctly. This is a smart way to control the workflow based on PR labels.
39-60: Well done on themark-stalejob!This job appropriately marks the Cypress status as stale when the "ok-to-test" label is missing. The use of a warning message keeps the PR status clear.
63-64: Nice update to theparse-tagsjob!The dependency on
checkTestLabelensures that this job only runs when the PR is ready for testing. This is a good use of conditional logic to streamline the workflow.
hetunandu
added
ok-to-test
3 participants
Description
By changing the workflow trigger and the way to check for correct labels, we are trying to allow the PR integrations tests to run every time the PR is updated. That means, if you push changes, and the PR already has ok-to-test label, it will rerun the tests based on new changes
Automation
/ok-to-test tags="@tag.Sanity"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/10296910575
Commit: c59583b
Cypress dashboard.
Tags:
@tag.SanitySpec:
Thu, 08 Aug 2024 06:32:48 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
checkTestLabel, to verify the presence of the "ok-to-test" label on pull requests.mark-stale, to mark the Cypress status as stale when the label is absent.parse-tagsjob to run only after thecheckTestLabeljob, enhancing workflow control.parse-tagsjob based on the output of thecheckTestLabeljob.