Split notification tests into multiple files. #2094
Conversation
WalkthroughThis PR reorganizes unit tests for the notify.sh script by extracting notification channel-specific test suites from a monolithic file into separate dedicated test files. Tests for email, GitHub, Jira, New Relic, Slack, and webhook notifications are moved to individual BAT test files, while the original file retains only general structure tests. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
.vortex/tests/bats/unit/notify-email.bats(1 hunks).vortex/tests/bats/unit/notify-github.bats(1 hunks).vortex/tests/bats/unit/notify-jira.bats(1 hunks).vortex/tests/bats/unit/notify-newrelic.bats(1 hunks).vortex/tests/bats/unit/notify-slack.bats(1 hunks).vortex/tests/bats/unit/notify-webhook.bats(1 hunks).vortex/tests/bats/unit/notify.bats(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-08T12:02:24.652Z
Learnt from: AlexSkrypnyk
Repo: drevops/vortex PR: 1896
File: .vortex/tests/bats/unit/download-db-lagoon.bats:24-25
Timestamp: 2025-08-08T12:02:24.652Z
Learning: In .vortex/tests/bats/unit Bats tests using ../_helper.bash (run_steps), prefixing a STEPS entry with "- " denotes a negative assertion (the substring must NOT appear in output). Unprefixed entries are positive assertions. Example: "- Database dump refresh requested. Will create a new dump." asserts absence; "Database dump refresh requested. Will create a new dump." asserts presence.
Applied to files:
.vortex/tests/bats/unit/notify-slack.bats.vortex/tests/bats/unit/notify.bats.vortex/tests/bats/unit/notify-jira.bats.vortex/tests/bats/unit/notify-email.bats.vortex/tests/bats/unit/notify-newrelic.bats.vortex/tests/bats/unit/notify-webhook.bats.vortex/tests/bats/unit/notify-github.bats
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: build (0)
- GitHub Check: build (1)
- GitHub Check: vortex-test-workflow (4)
- GitHub Check: vortex-test-workflow (1)
- GitHub Check: vortex-test-workflow (3)
- GitHub Check: vortex-test-workflow (2)
- GitHub Check: vortex-test-common
- GitHub Check: vortex-test-workflow (0)
- GitHub Check: vortex-test-installer (8.4)
- GitHub Check: vortex-test-installer (8.3)
- GitHub Check: vortex-test-docs
🔇 Additional comments (6)
.vortex/tests/bats/unit/notify.bats (1)
1-56: LGTM! Excellent test refactoring.The reorganization successfully reduces this file to focus on general notification dispatch behavior while moving channel-specific tests to dedicated files. The updated comments clearly document where to find the split-out tests, making the codebase more maintainable and navigable.
.vortex/tests/bats/unit/notify-webhook.bats (1)
1-59: LGTM! Comprehensive webhook test coverage.The tests properly cover both success and failure scenarios for webhook notifications. The use of mocked curl responses (200 for success, 400 for failure) effectively validates error handling without requiring external dependencies.
.vortex/tests/bats/unit/notify-email.bats (1)
1-62: LGTM! Email notification tests are well-structured.The tests effectively distinguish between branch and PR deployment scenarios, properly validating recipient parsing (including the pipe-delimited format with optional names) and ensuring the correct messaging appears in the output.
.vortex/tests/bats/unit/notify-slack.bats (1)
1-185: Excellent Slack notification test coverage.The test suite comprehensively covers branch deployments, PR deployments, pre-deployment events, error conditions (missing webhook, API failures), and configuration variations (custom channel, username, icon, commit SHA). The variety of scenarios provides strong confidence in the Slack notification functionality.
.vortex/tests/bats/unit/notify-jira.bats (1)
1-55: LGTM! Thorough Jira integration test.This test comprehensively validates the complete Jira notification workflow: issue extraction from branch name, API authentication, comment posting with inline cards, issue transitions, and assignee updates. The step-by-step validation approach with mocked curl commands ensures the exact API interactions are correct, including payload structure and authentication headers.
.vortex/tests/bats/unit/notify-github.bats (1)
1-243: LGTM! Comprehensive GitHub deployments API test coverage.The test suite thoroughly validates both pre-deployment and post-deployment notification flows, including:
- Different deployment ID lengths (verifying regex patterns work correctly)
- PR-specific environment naming (PR-123 vs PR)
- Multiple failure scenarios (invalid branch, missing deployment ID, failed status updates)
The use of negative assertions (prefixed with "-") correctly validates that certain messages don't appear in failure scenarios, aligning with the project's test patterns.
| #!/usr/bin/env bats | ||
| ## | ||
| # Unit tests for NewRelic notifications (notify.sh). | ||
| # | ||
| #shellcheck disable=SC2030,SC2031,SC2034 | ||
|
|
||
| load ../_helper.bash | ||
|
|
||
| @test "Notify: newrelic" { | ||
| pushd "${LOCAL_REPO_DIR}" >/dev/null || exit 1 | ||
|
|
||
| app_id="9876543210" | ||
| mock_curl=$(mock_command "curl") | ||
|
|
||
| mock_set_output "${mock_curl}" "12345678910-1234567890-${app_id}-12345" 1 | ||
| mock_set_output "${mock_curl}" "201" 2 | ||
|
|
||
| export VORTEX_NOTIFY_CHANNELS="newrelic" | ||
| export VORTEX_NOTIFY_PROJECT="testproject" | ||
| export VORTEX_NOTIFY_NEWRELIC_APIKEY="key1234" | ||
| export VORTEX_NOTIFY_EMAIL_RECIPIENTS="[email protected]|John Doe,[email protected]|Jane Doe" | ||
| export VORTEX_NOTIFY_REF="develop" | ||
| export VORTEX_NOTIFY_SHA="123456" | ||
|
|
||
| run ./scripts/vortex/notify.sh | ||
| assert_success | ||
|
|
||
| assert_output_contains "Started dispatching notifications." | ||
|
|
||
| assert_output_contains "Started New Relic notification." | ||
| assert_output_contains "Discovering APP id by name if it was not provided." | ||
| assert_output_contains "Checking if the application ID is valid." | ||
| assert_output_contains "Creating a deployment notification for application testproject-develop with ID 9876543210." | ||
|
|
||
| assert_equal "-s -X GET https://api.newrelic.com/v2/applications.json -H Api-Key:key1234 -s -G -d filter[name]=testproject-develop&exclude_links=true" "$(mock_get_call_args "${mock_curl}" 1)" | ||
| assert_equal '-X POST https://api.newrelic.com/v2/applications/9876543210/deployments.json -L -s -o /dev/null -w %{http_code} -H Api-Key:key1234 -H Content-Type: application/json -d { | ||
| "deployment": { | ||
| "revision": "123456", | ||
| "changelog": "develop deployed", | ||
| "description": "develop deployed", | ||
| "user": "Deployment robot" | ||
| } | ||
| }' "$(mock_get_call_args "${mock_curl}" 2)" | ||
|
|
||
| assert_output_contains "Finished New Relic notification." | ||
|
|
||
| assert_output_contains "Finished dispatching notifications." | ||
|
|
||
| popd >/dev/null || exit 1 | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
LGTM! New Relic notification test validates the happy path effectively.
The test properly validates the complete New Relic deployment notification workflow, including app ID discovery by name and deployment creation with the correct JSON payload structure. The use of mock_get_call_args to assert exact API call parameters (including filter parameters and JSON structure) provides strong validation of the integration.
Consider adding failure scenario tests (invalid API key, app not found, API errors) in a future iteration for more comprehensive coverage. Would you like me to suggest additional test cases?
🤖 Prompt for AI Agents
.vortex/tests/bats/unit/notify-newrelic.bats around lines 1-50: no code change
required for the happy-path test (LGTM); if you want to extend coverage, add
separate tests for failure scenarios: (1) invalid API key returning 401/403 and
asserting error handling and exit code, (2) app not found returning empty
applications list or 404 and asserting the correct discovery/error message and
fallback behavior, and (3) API errors (5xx or non-201 on deploy) asserting
retry/error logging and non-zero exit status.
| @test "Notify: slack, pre_deployment" { | ||
| pushd "${LOCAL_REPO_DIR}" >/dev/null || exit 1 | ||
|
|
||
| mock_curl=$(mock_command "curl") | ||
| mock_set_output "${mock_curl}" "200" 1 | ||
|
|
||
| export VORTEX_NOTIFY_CHANNELS="slack" | ||
| export VORTEX_NOTIFY_EVENT="pre_deployment" | ||
| export VORTEX_NOTIFY_PROJECT="testproject" | ||
| export VORTEX_NOTIFY_REF="develop" | ||
| export VORTEX_NOTIFY_ENVIRONMENT_URL="https://develop.testproject.com" | ||
| export VORTEX_NOTIFY_SLACK_WEBHOOK="https://hooks.slack.com/services/T00000000/B00000000/XXXXXXXXXXXXXXXXXXXX" | ||
|
|
||
| # Pre-deployment forces github channel, but we're testing slack directly | ||
| export VORTEX_NOTIFY_CHANNELS="github" | ||
|
|
||
| # Test Slack script directly for pre-deployment | ||
| export VORTEX_NOTIFY_CHANNELS="slack" | ||
| run ./scripts/vortex/notify-slack.sh | ||
| assert_success | ||
|
|
||
| assert_output_contains "Started Slack notification." | ||
| assert_output_contains "Notification sent to Slack." | ||
| assert_output_contains "Finished Slack notification." | ||
|
|
||
| popd >/dev/null || exit 1 | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Remove unnecessary assignment on Line 78.
The assignment VORTEX_NOTIFY_CHANNELS="github" on Line 78 is immediately overwritten by the same variable being set to "slack" on Line 81. Since the test directly invokes notify-slack.sh (bypassing the channel routing logic), the first assignment serves no purpose and may confuse future readers.
Apply this diff to remove the redundant assignment:
export VORTEX_NOTIFY_SLACK_WEBHOOK="https://hooks.slack.com/services/T00000000/B00000000/XXXXXXXXXXXXXXXXXXXX"
- # Pre-deployment forces github channel, but we're testing slack directly
- export VORTEX_NOTIFY_CHANNELS="github"
-
# Test Slack script directly for pre-deployment
export VORTEX_NOTIFY_CHANNELS="slack"
run ./scripts/vortex/notify-slack.sh📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @test "Notify: slack, pre_deployment" { | |
| pushd "${LOCAL_REPO_DIR}" >/dev/null || exit 1 | |
| mock_curl=$(mock_command "curl") | |
| mock_set_output "${mock_curl}" "200" 1 | |
| export VORTEX_NOTIFY_CHANNELS="slack" | |
| export VORTEX_NOTIFY_EVENT="pre_deployment" | |
| export VORTEX_NOTIFY_PROJECT="testproject" | |
| export VORTEX_NOTIFY_REF="develop" | |
| export VORTEX_NOTIFY_ENVIRONMENT_URL="https://develop.testproject.com" | |
| export VORTEX_NOTIFY_SLACK_WEBHOOK="https://hooks.slack.com/services/T00000000/B00000000/XXXXXXXXXXXXXXXXXXXX" | |
| # Pre-deployment forces github channel, but we're testing slack directly | |
| export VORTEX_NOTIFY_CHANNELS="github" | |
| # Test Slack script directly for pre-deployment | |
| export VORTEX_NOTIFY_CHANNELS="slack" | |
| run ./scripts/vortex/notify-slack.sh | |
| assert_success | |
| assert_output_contains "Started Slack notification." | |
| assert_output_contains "Notification sent to Slack." | |
| assert_output_contains "Finished Slack notification." | |
| popd >/dev/null || exit 1 | |
| } | |
| @test "Notify: slack, pre_deployment" { | |
| pushd "${LOCAL_REPO_DIR}" >/dev/null || exit 1 | |
| mock_curl=$(mock_command "curl") | |
| mock_set_output "${mock_curl}" "200" 1 | |
| export VORTEX_NOTIFY_CHANNELS="slack" | |
| export VORTEX_NOTIFY_EVENT="pre_deployment" | |
| export VORTEX_NOTIFY_PROJECT="testproject" | |
| export VORTEX_NOTIFY_REF="develop" | |
| export VORTEX_NOTIFY_ENVIRONMENT_URL="https://develop.testproject.com" | |
| export VORTEX_NOTIFY_SLACK_WEBHOOK="https://hooks.slack.com/services/T00000000/B00000000/XXXXXXXXXXXXXXXXXXXX" | |
| # Test Slack script directly for pre-deployment | |
| export VORTEX_NOTIFY_CHANNELS="slack" | |
| run ./scripts/vortex/notify-slack.sh | |
| assert_success | |
| assert_output_contains "Started Slack notification." | |
| assert_output_contains "Notification sent to Slack." | |
| assert_output_contains "Finished Slack notification." | |
| popd >/dev/null || exit 1 | |
| } |
🤖 Prompt for AI Agents
In .vortex/tests/bats/unit/notify-slack.bats around lines 64 to 90, remove the
redundant assignment VORTEX_NOTIFY_CHANNELS="github" (line 78) which is
immediately overwritten by VORTEX_NOTIFY_CHANNELS="slack" on line 81; simply
delete the unnecessary line so the test sets channels to "slack" only once
before invoking ./scripts/vortex/notify-slack.sh and leave the rest of the test
assertions unchanged.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #2094 +/- ##
========================================
Coverage 68.04% 68.04%
========================================
Files 96 96
Lines 4731 4731
Branches 44 44
========================================
Hits 3219 3219
Misses 1512 1512 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary by CodeRabbit