search

Automattic / a8c-ci-toolkit-buildkite-plugin

A caching plugin that can be invoked from your build script.
22 stars 5 forks source link

Send Slack notification for Test Failures #60

Closed jostnes closed 1 year ago

jostnes commented 1 year ago

Description

This PR updates annotate_test_failures with the ability to send Slack notification when there are test failures. It is an optional feature and is used by passing a second argument when calling it from project repos (I just realized via this PR when updating to latest trunk that Tumblr also uses this and they might not want the Slack notification to be sent which was the reason I added the second argument).

The changes made for this to happen:

  1. Update update_annotation to return a JSON object when a failure happen
  2. Create send_slack_notification to read the JSON object, parse and form the message payload, and send a POST call to the Slack Webhook. For now, it's set specifically to #build-and-ship channel

In this change, the message displays the repo (to know which app) and step name (to know if it's Unit, UI iPhone or UI iPad, etc), the number of failing test cases, and the names + test classes of failing tests.

Some notes:

  1. Failure notification is only sent for merges to trunk (failures on other branches will not as we are trying to get an idea how noisy this will be for trunk first)
  2. When there are no failures, a notification is not sent (no notification for flaky tests)
  3. If the app crashes before the JUnit XML file is created, a notification is not sent (similar to how annotation on Buildkite is not displayed when this happens currently)

Testing

Testing PR: https://github.com/woocommerce/woocommerce-ios/pull/9798

  1. When there are multiple failures (build): image

Slack: p1685000034603039-slack-C03Q9FTCQA0

  1. When there is 1 failure (build): image

Slack: p1685001564964369-slack-C03Q9FTCQA0

  1. When all test pass (with flaky tests) or one without flaky test here: image

  2. When failure not on trunk branch: image

  3. When should_send_slack_notification is "false": image

AliSoftware commented 1 year ago

@jostnes FYI, given https://github.com/Automattic/a8c-ci-toolkit-buildkite-plugin/pull/58 was recently merged and also updated annotate_test_failures script, you might want to update your branch to make sure your changes fit nicely on top of those (I don't forsee any conflict, but just to make things clean and avoid any surprise) 🙃

image
jostnes commented 1 year ago

thanks for the review @AliSoftware ! i made the suggested changes and tested with this CI build and locally, it's ready for another review.

though i did notice one thing - the main branches name are different on Tumblr repos. i made this update https://github.com/Automattic/a8c-ci-toolkit-buildkite-plugin/pull/60/commits/f89ed065dfb7e31cd51d5713fa1555ddc55fdc32 to also check if the build is on develop or master. wdyt?

jostnes commented 1 year ago

Merging this, the latest changes were tested using this WCiOS build (notification sent, notification not sent) and works as expected.