Skip to content

Fixes #939 - Handle incomplete CI results - #940

Open
jakduch wants to merge 1 commit into
theforeman:masterfrom
jakduch:fix/ci-required-job-results
Open

jakduch wants to merge 1 commit into
theforeman:masterfrom
jakduch:fix/ci-required-job-results

Conversation

@jakduch

@jakduch jakduch commented Oct 15, 2026

Copy link
Copy Markdown
Contributor

Why are you introducing these changes? (Problem description, related links)

Fixes #939.

In PR #861, Ansible Lint never acquired a hosted runner. GitHub reported its result as abandoned, and the final Test suite job raised KeyError: 'abandoned' instead of reporting the unfinished lint check.

An unfinished job must still make CI fail. Foremanctl requires every job to succeed and does not use the allow-lists offered by alls-green, so a direct result check is sufficient.

What are the changes introduced in this pull request?

  • Print every required job and its result
  • Use jq --exit-status to accept only a non-empty result object where every job reports success
  • Pass the needs context through an environment variable, without interpolating its contents into shell code
  • Exercise the workflow's actual commands in regression tests and document the VM-free local test command

How to test this pull request

With the development virtual environment active and bash and jq on PATH:

python -m pytest --confcutdir=tests/unit tests/unit/ci_results_test.py
ruff check tests/unit/ci_results_test.py
git diff --check

All 12 regression cases pass locally. They cover success, failure, cancellation, skipped and abandoned jobs, future unknown statuses, null or missing results, empty and malformed input. No VMs were deployed or additional dependencies installed for these checks. The full deployment matrix remains for CI.

Checklist

  • Tests added/updated (if applicable)
  • Documentation updated (if applicable)

AI assistance: Codex. Project signature: GPT_5.6 Sol.

Check required job results directly instead of crashing while rendering an
unexpected result such as abandoned. Keep the check failing unless every
required job succeeds, and print each result to identify the unfinished job.

Test the workflow's actual shell commands for successful, failed, skipped,
cancelled, abandoned, unknown, null, missing, empty and malformed inputs.

AI assistance: Codex. Project signature: GPT_5.6 Sol.
@coderabbitai

coderabbitai Bot commented Oct 15, 2026

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 59 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 016f87bb-9f60-48ac-b0b7-549f54af991e
📥 Commits

Reviewing files that changed from the base of the PR and between 172b57b and 6ca237e.

📒 Files selected for processing: 3
  • .github/workflows/test.yml
  • docs/developer/testing.md
  • tests/unit/ci_results_test.py
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

name: Test suite
steps:
- name: Decide whether the needed jobs succeeded or failed
uses: re-actors/alls-green@release/v1

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Did you consider if this has configuration to match what you were seeing? Or could this github action project be improved instead of us abandoning it?

@jakduch

jakduch commented Oct 26, 2026

Copy link
Copy Markdown
Contributor Author

@ehelms I checked the configuration and reproduced this in alls-green itself. The job was already considered unsuccessful; rendering the abandoned result was what crashed, so changing the allow-lists would not fix it.

I opened re-actors/alls-green#45 with a small fix and regression tests. tox -e py passed all 45 tests with 100% coverage locally.

I agree we should keep alls-green rather than maintain our own replacement. I'll leave this PR open while the upstream fix is reviewed and released, but the jq alternative here is not intended to be merged in the meantime.

@jakduch
jakduch requested a review from ehelms October 27, 2026 17:42
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.

Handle abandoned jobs in the final CI check

2 participants