LCORE-2916: Slack notification for providers e2e tests#2199
Conversation
WalkthroughThe E2E workflow now captures test output, extracts failing scenarios into per-matrix-job JSON artifacts, consolidates all matrix results, and posts the combined payload to Slack. ChangesE2E reporting workflow
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 7✅ Passed checks (7 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/e2e_tests_providers.yaml:
- Around line 376-403: Update the workflow step to expose matrix.mode and
matrix.environment through environment variables alongside JOB_STATUS, then pass
those variables to jq via the existing --arg options instead of interpolating
GitHub Actions expressions inside the run script. Preserve the current
result.json fields and values.
- Around line 455-460: Update the jq invocation in the payload construction to
request compact JSON output with the -c option. Keep the existing --argjson,
--arg, and transformation logic unchanged so payload contains a single-line JSON
string before the subsequent echo.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f413d7e2-999c-42cc-98f1-30f07f5557ed
📒 Files selected for processing (1)
.github/workflows/e2e_tests_providers.yaml
📜 Review details
⏰ Context from checks skipped due to timeout. (17)
- GitHub Check: build-pr
- GitHub Check: mypy
- GitHub Check: radon
- GitHub Check: Pyright
- GitHub Check: Pylinter
- GitHub Check: unit_tests (3.12)
- GitHub Check: unit_tests (3.13)
- GitHub Check: integration_tests (3.13)
- GitHub Check: integration_tests (3.12)
- GitHub Check: E2E Tests for Lightspeed Evaluation job
- GitHub Check: E2E: server mode / ci / group 2
- GitHub Check: E2E: library mode / ci / group 2
- GitHub Check: E2E: server mode / ci / group 1
- GitHub Check: E2E: library mode / ci / group 3
- GitHub Check: E2E: library mode / ci / group 1
- GitHub Check: E2E: server mode / ci / group 3
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-0-7-on-pull-request
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-05-06T08:35:54.687Z
Learnt from: radofuchs
Repo: lightspeed-core/lightspeed-stack PR: 1690
File: .github/workflows/e2e_tests_providers.yaml:279-285
Timestamp: 2026-05-06T08:35:54.687Z
Learning: In .github/workflows/e2e_tests_providers.yaml and related e2e workflow files, the show_logs step should not use docker compose logs with --tail or --since (i.e., keep logs unbounded). The quick connectivity test runs once immediately after container startup, so the log output is small and a log tail limit is unnecessary. If you adjust this, add a rationale comment in the workflow explaining why unbounded logs are acceptable and ensure CI behavior remains deterministic.
Applied to files:
.github/workflows/e2e_tests_providers.yaml
🪛 zizmor (1.26.1)
.github/workflows/e2e_tests_providers.yaml
[warning] 398-398: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 399-399: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[error] 407-407: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 421-421: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
| env: | ||
| JOB_STATUS: ${{ job.status }} | ||
| run: | | ||
| mkdir -p ./e2e-result | ||
|
|
||
| # Extract the "Failing scenarios:" block that behave prints on failure. | ||
| FAILING_SCENARIOS="" | ||
| if [ -f e2e-test-output.log ]; then | ||
| FAILING_SCENARIOS=$(sed 's/\x1b\[[0-9;]*m//g' e2e-test-output.log \ | ||
| | awk '/^Failing scenarios:/{flag=1; next} /^$/{flag=0} flag' \ | ||
| | sed 's/^[[:space:]]*//') | ||
| fi | ||
|
|
||
| if [ -z "${FAILING_SCENARIOS}" ]; then | ||
| if [ "${JOB_STATUS}" == "success" ]; then | ||
| FAILING_SCENARIOS="None" | ||
| else | ||
| FAILING_SCENARIOS="Job ${JOB_STATUS} - no failing scenarios captured (see run logs)" | ||
| fi | ||
| fi | ||
|
|
||
| jq -n \ | ||
| --arg mode "${{ matrix.mode }}" \ | ||
| --arg environment "${{ matrix.environment }}" \ | ||
| --arg status "${JOB_STATUS}" \ | ||
| --arg failing_scenarios "${FAILING_SCENARIOS}" \ | ||
| '{mode: $mode, environment: $environment, status: $status, failing_scenarios: $failing_scenarios}' \ | ||
| > ./e2e-result/result.json |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Use environment variables for matrix context to prevent shell injection risks.
While the current matrix values are statically defined and safe, it is a security best practice to pass GitHub Actions expressions (like ${{ matrix... }}) into bash scripts via environment variables rather than inline template expansion. This addresses static analysis warnings and prevents potential shell injection vulnerabilities if the matrix is ever updated to use dynamic or external inputs.
♻️ Proposed refactor
env:
JOB_STATUS: ${{ job.status }}
+ MATRIX_MODE: ${{ matrix.mode }}
+ MATRIX_ENVIRONMENT: ${{ matrix.environment }}
run: |
mkdir -p ./e2e-result
# Extract the "Failing scenarios:" block that behave prints on failure.
FAILING_SCENARIOS=""
if [ -f e2e-test-output.log ]; then
FAILING_SCENARIOS=$(sed 's/\x1b\[[0-9;]*m//g' e2e-test-output.log \
| awk '/^Failing scenarios:/{flag=1; next} /^$/{flag=0} flag' \
| sed 's/^[[:space:]]*//')
fi
if [ -z "${FAILING_SCENARIOS}" ]; then
if [ "${JOB_STATUS}" == "success" ]; then
FAILING_SCENARIOS="None"
else
FAILING_SCENARIOS="Job ${JOB_STATUS} - no failing scenarios captured (see run logs)"
fi
fi
jq -n \
- --arg mode "${{ matrix.mode }}" \
- --arg environment "${{ matrix.environment }}" \
+ --arg mode "${MATRIX_MODE}" \
+ --arg environment "${MATRIX_ENVIRONMENT}" \
--arg status "${JOB_STATUS}" \
--arg failing_scenarios "${FAILING_SCENARIOS}" \
'{mode: $mode, environment: $environment, status: $status, failing_scenarios: $failing_scenarios}' \
> ./e2e-result/result.json📝 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.
| env: | |
| JOB_STATUS: ${{ job.status }} | |
| run: | | |
| mkdir -p ./e2e-result | |
| # Extract the "Failing scenarios:" block that behave prints on failure. | |
| FAILING_SCENARIOS="" | |
| if [ -f e2e-test-output.log ]; then | |
| FAILING_SCENARIOS=$(sed 's/\x1b\[[0-9;]*m//g' e2e-test-output.log \ | |
| | awk '/^Failing scenarios:/{flag=1; next} /^$/{flag=0} flag' \ | |
| | sed 's/^[[:space:]]*//') | |
| fi | |
| if [ -z "${FAILING_SCENARIOS}" ]; then | |
| if [ "${JOB_STATUS}" == "success" ]; then | |
| FAILING_SCENARIOS="None" | |
| else | |
| FAILING_SCENARIOS="Job ${JOB_STATUS} - no failing scenarios captured (see run logs)" | |
| fi | |
| fi | |
| jq -n \ | |
| --arg mode "${{ matrix.mode }}" \ | |
| --arg environment "${{ matrix.environment }}" \ | |
| --arg status "${JOB_STATUS}" \ | |
| --arg failing_scenarios "${FAILING_SCENARIOS}" \ | |
| '{mode: $mode, environment: $environment, status: $status, failing_scenarios: $failing_scenarios}' \ | |
| > ./e2e-result/result.json | |
| env: | |
| JOB_STATUS: ${{ job.status }} | |
| MATRIX_MODE: ${{ matrix.mode }} | |
| MATRIX_ENVIRONMENT: ${{ matrix.environment }} | |
| run: | | |
| mkdir -p ./e2e-result | |
| # Extract the "Failing scenarios:" block that behave prints on failure. | |
| FAILING_SCENARIOS="" | |
| if [ -f e2e-test-output.log ]; then | |
| FAILING_SCENARIOS=$(sed 's/\x1b\[[0-9;]*m//g' e2e-test-output.log \ | |
| | awk '/^Failing scenarios:/{flag=1; next} /^$/{flag=0} flag' \ | |
| | sed 's/^[[:space:]]*//') | |
| fi | |
| if [ -z "${FAILING_SCENARIOS}" ]; then | |
| if [ "${JOB_STATUS}" == "success" ]; then | |
| FAILING_SCENARIOS="None" | |
| else | |
| FAILING_SCENARIOS="Job ${JOB_STATUS} - no failing scenarios captured (see run logs)" | |
| fi | |
| fi | |
| jq -n \ | |
| --arg mode "${MATRIX_MODE}" \ | |
| --arg environment "${MATRIX_ENVIRONMENT}" \ | |
| --arg status "${JOB_STATUS}" \ | |
| --arg failing_scenarios "${FAILING_SCENARIOS}" \ | |
| '{mode: $mode, environment: $environment, status: $status, failing_scenarios: $failing_scenarios}' \ | |
| > ./e2e-result/result.json |
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 398-398: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 399-399: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/e2e_tests_providers.yaml around lines 376 - 403, Update
the workflow step to expose matrix.mode and matrix.environment through
environment variables alongside JOB_STATUS, then pass those variables to jq via
the existing --arg options instead of interpolating GitHub Actions expressions
inside the run script. Preserve the current result.json fields and values.
Source: Linters/SAST tools
| payload=$(echo "${ALL_RESULTS}" | jq \ | ||
| --argjson fields "${BASE_FIELDS}" \ | ||
| --arg run "${RUN_URL}" \ | ||
| '{run_url: $run} + (reduce .[] as $r ($fields; . + {(($r.mode + "_" + $r.environment)): $r.failing_scenarios}))') | ||
|
|
||
| echo "Payload: ${payload}" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Use compact JSON output for the payload.
jq pretty-prints JSON by default, which introduces internal newlines into the payload bash variable. While curl generally handles multiline data properly, generating a compact JSON string using the -c flag is safer for bash variables and produces cleaner, more readable log output on the subsequent echo command.
♻️ Proposed refactor
# Build one field per "<mode>_<environment>" key holding that job's failing scenarios.
- payload=$(echo "${ALL_RESULTS}" | jq \
+ payload=$(echo "${ALL_RESULTS}" | jq -c \
--argjson fields "${BASE_FIELDS}" \
--arg run "${RUN_URL}" \
'{run_url: $run} + (reduce .[] as $r ($fields; . + {(($r.mode + "_" + $r.environment)): $r.failing_scenarios}))')
echo "Payload: ${payload}"📝 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.
| payload=$(echo "${ALL_RESULTS}" | jq \ | |
| --argjson fields "${BASE_FIELDS}" \ | |
| --arg run "${RUN_URL}" \ | |
| '{run_url: $run} + (reduce .[] as $r ($fields; . + {(($r.mode + "_" + $r.environment)): $r.failing_scenarios}))') | |
| echo "Payload: ${payload}" | |
| payload=$(echo "${ALL_RESULTS}" | jq -c \ | |
| --argjson fields "${BASE_FIELDS}" \ | |
| --arg run "${RUN_URL}" \ | |
| '{run_url: $run} + (reduce .[] as $r ($fields; . + {(($r.mode + "_" + $r.environment)): $r.failing_scenarios}))') | |
| echo "Payload: ${payload}" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/e2e_tests_providers.yaml around lines 455 - 460, Update
the jq invocation in the payload construction to request compact JSON output
with the -c option. Keep the existing --argjson, --arg, and transformation logic
unchanged so payload contains a single-line JSON string before the subsequent
echo.
1 participant
Description
Adds a consolidated Slack notification to the provider e2e test workflow so failures are reported per mode/environment combination instead of just an overall pass/fail. Each
e2e_testsmatrix job (2 modes × 4 environments) now captures its ownbehaveoutput, extracts theFailing scenarios:block on failure, and uploads it as a result artifact. A newnotifyjob downloads all artifacts and posts a single Slack webhook payload with one field per<mode>_<environment>key (e.g.server_azure,library_watsonx) containing that job's failing scenarios (or"None"on success,"N/A"if a job never ran), plus arun_urlfield linking back to the workflow run. This makes it much faster to see exactly which provider/mode combinations broke and why, straight from Slack, without digging through 8 separate CI job logs.Type of change
Tools used to create PR
Identify any AI code assistants used in this PR (for transparency and review context)
Related Tickets & Documents
Checklist before requesting a review
Testing
jqpayload-construction logic locally with sample data to confirm it produces the expected{run_url, server_azure, server_vertexai, server_watsonx, server_bedrock, library_azure, library_vertexai, library_watsonx, library_bedrock}shape, including correct escaping of multi-line failing-scenario strings.awk/sedextraction correctly pulls out theFailing scenarios:block from a samplebehave-style log, including stripping ANSI color codes.workflow_dispatchonce merged to confirm the actual Slack message renders as expected against the real Slack workflow trigger's input variables.Summary by CodeRabbit