Skip to content
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

DRAFT [QA-TICKET] Slack test results notification fails to report failed E2E test #4833

Open
wants to merge 9 commits into
base: dev
Choose a base branch
from

Conversation

aweng98
Copy link
Contributor

@aweng98 aweng98 commented May 15, 2024

hypothesis, need testing.

Jira Ticket: https://broadworkbench.atlassian.net/browse/[Ticket #]

Summary of changes:

What

terra-ui integration tests Slack notification were not reporting failed tests in the notifications sometimes. Seeing in this job and job.

Why

I think it's a timing issue, occurs when the data update time in CircleCI varies. While CircleCI is in the middle of updating job data from remaining CIRCLE_NODE_INDEX: 0 node, the execution of notify-circleci-test-results.js has prematurely gathered incomplete test results and finished slack notification.

either test-summary-0.json does't show failed test because it was the "final" or this file could be missing and not read at all.

Testing strategy

@aweng98 aweng98 changed the title log err in bash script [QA-TICKET] Slack test results notification fails to report failed E2E test May 15, 2024
@aweng98 aweng98 marked this pull request as ready for review May 15, 2024 23:56
@aweng98 aweng98 force-pushed the alex-ci-testing branch from e8d520c to 504755a Compare May 16, 2024 01:45
Comment on lines 20 to 22
runs=$(echo "$job_detail" | jq -r '.parallel_runs[]')
running_vm=$(echo "$runs" | jq -r 'select(.status=="running")')
count=$(echo "$running_vm" | grep -c -e "running" || test $? = 1;)
Copy link
Contributor Author

@aweng98 aweng98 May 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Break down into smaller jq statements instead one very long statement. Easier to read and debugging.
Removed select(.index != '"$CIRCLE_NODE_INDEX"')' condition. Is this going to be a problem for the while loop?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed select(.index != '"$CIRCLE_NODE_INDEX"')' condition. Is this going to be a problem for the while loop?

Yes, I think that's going to be a problem. This script can't wait for the runner that it is running on to finish because then the script will be waiting on itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense! It has been too long since last time i looked at this file.

Comment on lines 23 to 29
if [ "$count" -eq 0 ]; then
echo "Parallel runs have finished. Exiting wait-for-job-finish script."
echo "$runs"
exit 0
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additional logs for debugging or manual inspection.

Screenshot 2024-05-15 at 9 40 38 PM

@aweng98 aweng98 changed the title [QA-TICKET] Slack test results notification fails to report failed E2E test DRAFT [QA-TICKET] Slack test results notification fails to report failed E2E test May 16, 2024
@aweng98 aweng98 force-pushed the alex-ci-testing branch from 4d48839 to 0e17970 Compare June 3, 2024 22:45
Copy link

sonarqubecloud bot commented Jun 4, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants