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

Added better exit code checks for CIV execution. #342

Merged
merged 1 commit into from
Sep 27, 2024

Conversation

narmaku
Copy link
Collaborator

@narmaku narmaku commented Sep 6, 2024

This allows the aws.sh script to finish gracefully in the event no test cases were collected because the filters used actually did not select any applicable test (e.g. "pub" marked tests for Image Builder internal CI).

Also added extra checks for infrastructure-related issues such as when CIV exits with code 100 (introduced by us to identify deployment issues).

Copy link
Collaborator

@F-X64 F-X64 left a comment

Choose a reason for hiding this comment

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

Changes look good to me. Added functionality makes sense. Approved!

Side note:
Long term I would suggest to use more functions and more involved conditional statements to make your flows easier to understand and maintain.

E.g. your switch could be rewritten as

run_civ_test() {
    ...
    $CIV_EXIT_CODE=$?
    ...
    case $CIV_EXIT_CODE in
        0)
            greenprint "💚 Success"
            exit 0
            ;;
        5)
            echo "❗ No tests were run"
            exit 0
            ;;
        100)
            redprint "❌ Failed (cloud deployment/destroy issues)"
            exit 1
            ;;
        *)
            redprint "❌ Failed (exit code: ${CIV_EXIT_CODE})"
            exit 1
            ;;
    esac

Just an extra thought.

@ConorC117 ConorC117 self-requested a review September 12, 2024 08:55
Copy link
Collaborator

@ConorC117 ConorC117 left a comment

Choose a reason for hiding this comment

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

LGTM too!

@narmaku narmaku force-pushed the fix-ci-failure-when-no-tests-are-run branch from e5cf6b1 to ca4f694 Compare September 13, 2024 01:18
@narmaku narmaku force-pushed the fix-ci-failure-when-no-tests-are-run branch from ca4f694 to a614a07 Compare September 20, 2024 08:27
@narmaku narmaku enabled auto-merge (rebase) September 20, 2024 08:28
This allows the aws.sh script to finish gracefully in the event no test
cases were collected because the filters used actually did not select any
applicable test (e.g. "pub" marked tests for Image Builder internal CI).

Also added extra checks for infrastructure-related issues such as when
CIV exits with code 100 (introduced by us to identify deployment issues).
@narmaku narmaku force-pushed the fix-ci-failure-when-no-tests-are-run branch from a614a07 to 42fde64 Compare September 27, 2024 06:18
@narmaku narmaku merged commit c7d57fa into osbuild:main Sep 27, 2024
3 checks passed
@narmaku narmaku deleted the fix-ci-failure-when-no-tests-are-run branch September 27, 2024 07:35
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