Skip to content

🐛 Use allowed-failures and skips equally #24

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.

Sign up for GitHub

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

Open
webknjaz wants to merge 2 commits into
base: unstable/v1
Choose a base branch
from
Open

🐛 Use allowed-failures and skips equally #24

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7e63d4b
🐛 Use allowed-failures and skips equally
webknjaz Aug 25, 2023
646d949
debug! prints
webknjaz Jul 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 32 additions & 6 deletions src/normalize_needed_jobs_status.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,9 @@ def main(argv: list[str]) -> int:
jobs = inputs['jobs'] or {}
jobs_allowed_to_fail = set(inputs['allowed_failures'] or [])
jobs_allowed_to_be_skipped = set(inputs['allowed_skips'] or [])
print(f'{jobs=}')
print(f'{jobs_allowed_to_fail=}')
print(f'{jobs_allowed_to_be_skipped=}')

Comment on lines +169 to 172
Copy link

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

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

The added debug print(f'{jobs=}') (and the following prints) can leak potentially sensitive data into GitHub Actions logs. jobs includes each needed job’s outputs, which may contain secrets depending on workflow usage. Please remove these prints or gate them behind an explicit debug flag and ensure any logged data excludes job outputs.

Suggested change
print(f'{jobs=}')
print(f'{jobs_allowed_to_fail=}')
print(f'{jobs_allowed_to_be_skipped=}')
# Optional debug logging: enable by setting DEBUG_NORMALIZE_NEEDED_JOBS=1.
# Only non-sensitive information (job names and results) is logged; job outputs are excluded.
if os.getenv('DEBUG_NORMALIZE_NEEDED_JOBS') == '1':
jobs_debug_view = {name: {'result': job['result']} for name, job in jobs.items()}
print(f'{jobs_debug_view=}')
print(f'{jobs_allowed_to_fail=}')
print(f'{jobs_allowed_to_be_skipped=}')

Copilot uses AI. Check for mistakes.
if not jobs:
with summary_file_path.open( # type: ignore[misc]
Expand All @@ -185,28 +188,51 @@ def main(argv: list[str]) -> int:
)
return 1

allowed_outcome_map = {}
for job_name in jobs:
allowed_outcome_map[job_name] = {'success'}
if job_name in jobs_allowed_to_be_skipped:
allowed_outcome_map[job_name].add('skipped')
if job_name in jobs_allowed_to_fail:
allowed_outcome_map[job_name].add('failure')
Copy link

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

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

allowed_outcome_map currently allows only {success, failure} for jobs in allowed_failures. This changes existing behavior and breaks the documented/tested semantics where an allowed-failure job may also be skipped without affecting the overall outcome (see tests like success-despite-failure-and-skip). Consider adding 'skipped' (and potentially 'cancelled' to match “don’t succeed” wording) to the allowed outcomes for allowed_failures, while keeping allowed_skips limited to allowing 'skipped' only.

Suggested change
allowed_outcome_map[job_name].add('failure')
allowed_outcome_map[job_name].update({'failure', 'skipped', 'cancelled'})

Copilot uses AI. Check for mistakes.

print(f'{allowed_outcome_map=}')
print(
f"""{
[
job['result'] == 'success' for name, job in jobs.items()
if name not in (jobs_allowed_to_fail | jobs_allowed_to_be_skipped)
]=
}""",
)
print(
f"""{
[
(name, job['result'] in {'skipped', 'success'}) for name, job in jobs.items()
if name in jobs_allowed_to_be_skipped
]=
}""",
)
job_matrix_succeeded = all(
job['result'] == 'success'
job['result'] in allowed_outcome_map[name]
for name, job in jobs.items()
if name not in (jobs_allowed_to_fail | jobs_allowed_to_be_skipped)
) and all(
job['result'] in {'skipped', 'success'}
for name, job in jobs.items()
if name in jobs_allowed_to_be_skipped
)
print(f'{job_matrix_succeeded=}')
Comment on lines +199 to +220
Copy link

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

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

There are multiple additional debug prints here (including printing computed lists and job_matrix_succeeded) that will add noisy log output for every action run. Please remove them or use a structured/conditional debug logger controlled by an input/env var so normal runs stay clean.

Copilot uses AI. Check for mistakes.
set_final_result_outputs(job_matrix_succeeded)

allowed_to_fail_jobs_succeeded = all(
job['result'] == 'success'
for name, job in jobs.items()
if name in jobs_allowed_to_fail
)
print(f'{allowed_to_fail_jobs_succeeded=}')

allowed_to_be_skipped_jobs_succeeded = all(
job['result'] == 'success'
for name, job in jobs.items()
if name in jobs_allowed_to_be_skipped
)
print(f'{allowed_to_be_skipped_jobs_succeeded=}')

with summary_file_path.open( # type: ignore[misc]
mode=FILE_APPEND_MODE,
Expand Down
Loading