-
Notifications
You must be signed in to change notification settings - Fork 110
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.
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
ci: fix slack matrix notifications #2496
Conversation
WalkthroughThe GitHub Actions workflows have been updated to improve Slack notifications. A new step was added to the E2E workflow to send detailed Slack messages with test results, while the reusable E2E workflow had its Slack notification step removed. These changes streamline how test run statuses are communicated. Changes
Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant E2EWorkflow
participant SlackWebhook
GitHubActions->>E2EWorkflow: Run E2E tests
E2EWorkflow->>E2EWorkflow: Process test results
E2EWorkflow->>SlackWebhook: Send Slack message with test results
SlackWebhook->>E2EWorkflow: Acknowledge message
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 (
|
f3458cc
to
8af9bc3
Compare
ef200ab
to
c80aff0
Compare
c80aff0
to
08117b6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (2)
- .github/workflows/e2e.yml (1 hunks)
- .github/workflows/reusable-e2e.yml (1 hunks)
Additional comments not posted (8)
.github/workflows/reusable-e2e.yml (4)
Line range hint
1-50
: Workflow Review: General ConfigurationThe general configuration of the workflow, including triggers and job settings, appears to be correctly defined. The removal of the Slack notification step does not seem to affect these configurations.
Line range hint
51-100
: Workflow Review: Docker and Cache ConfigurationsThe steps for Docker login, cache restoration, and cache injection are well-defined. These steps are crucial for optimizing build times and ensuring that the environment is correctly set up for the tests.
Line range hint
101-150
: Workflow Review: Test Execution and LoggingThe steps for building the Docker image, starting the test, and handling logs are appropriately configured. The use of
docker logs -f
for log watching and the conditional steps for log dumping and uploading on failure are good practices.
Line range hint
151-200
: Workflow Review: Cleanup OperationsThe steps for stopping the network and cleaning up the workspace are essential for maintaining a clean test environment. These steps are executed regardless of the job's success or failure, which is a good practice.
.github/workflows/e2e.yml (4)
Line range hint
1-50
: Workflow Review: General Configuration and TriggersThe configuration for triggers such as push, pull_request, and schedule is correctly set up. The use of
workflow_dispatch
with multiple test options provides flexibility for manual triggers.
Line range hint
51-100
: Workflow Review: Job Matrix and ConditionalsThe use of a job matrix based on test labels and conditions is a robust way to handle different test scenarios. The script for setting outputs based on the context and event type is well-implemented.
Line range hint
101-150
: Workflow Review: E2E Job ConfigurationThe configuration for the E2E jobs using a reusable workflow is correctly set up. The matrix strategy and the conditional inclusion of jobs based on the outputs from the
matrix-conditionals
step are well thought out.
Line range hint
151-209
: Workflow Review: Slack Notification StepThe new Slack notification step is complex and involves fetching job details, processing them, and sending a formatted message to Slack. The conditions for sending the message (on schedule or failed push events) are appropriate. However, the script should be reviewed to ensure that the message formatting and the logic for determining the overall result are correct.
Description
The slack e2e failure webhooks haven't been working quite right since #2422. Move the alert step from the reusable workflow to the top level e2e workflow and manually format and send the message. Now we'll only see one message per matrix run and the URL will actually work.
How Has This Been Tested?
See this slack channel for example
Summary by CodeRabbit