Take the job's outcome as an argument, not as an assumption
`report_job_log.py` hardcoded the word *failed* into the comment header. Every
caller guards the step with `if: failure()`, so it was true by construction --
until an unguarded probe in weblib-viewer#10 ran it on a job that passed, and
the successful run posted a comment reading as a failure report. Anyone
scrolling that PR would conclude the probe had failed.
The interesting uses of this script are exactly the ones that want
`if: always()`: a probe, or a job whose *output* is the point rather than its
exit code. Those all lied in the header.
`--status` now supplies the outcome. **The default is `failed`**, which is what
`if: failure()` means, so the four consuming repos are untouched -- a change in
required arguments would have broken all of them at once, since they take this
script from `@main`. `JOB_STATUS` in the environment does the same, matching how
every other argument here already reads its default from the Actions
environment.
`${{ job.status }}` yields `success`/`failure`/`cancelled`/`skipped` while a
human writing the flag reaches for `passed`/`failed`, so both spellings are
accepted and the expression can be passed straight through. An **unrecognised**
status goes into the header verbatim rather than being rejected: `argparse`'s
`choices=` would exit 2 on a value the table has not heard of, and the log --
the whole reason this script exists -- would never be posted. A reporter must
not become the thing that reports nothing.
The "no log file" note was status-dependent too; it claimed the step "failed
before the build started" regardless.
## Verified
`test_report_job_log.py`, new here: stdlib only and offline, posting to an
`http.server` on localhost that keeps what it is sent, so each test reads the
comment back. An exit status of 0 proves nothing -- the script deliberately
swallows HTTP errors so a failure to report cannot mask the failure being
reported. 21 tests, 0 skipped, 1.2s. Three of them drive the CLI as a
subprocess with only environment variables set, the way a workflow does.
Each check was shown to fire by injecting the fault and reverting it:
| injected fault | result |
|---|---|
| header hardcodes `failed` again (the original bug) | 10 failures |
| `DEFAULT_STATUS = "passed"` (would break the four callers) | 8 failures |
| unknown status raises, as `choices=` would | 2 errors |
| missing-log note keeps the failure wording | 1 failure |
| a stray `%` in the `--status` help text | 1 failure |
All five reverted; the file's checksum matches the pre-injection copy.
Also drops a tracked `__pycache__/report_job_log.cpython-313.pyc` and adds a
`.gitignore`. It was committed by accident in 061d8b2 and importing the module
from the tests rewrites it, so it would otherwise show up in every future diff
as stale bytecode of a file that had already changed.
Closes #8
Co-authored-by: bit <bit@das-labor.org>
This commit is contained in:
@@ -17,9 +17,16 @@ Usage, from a workflow step guarded by `if: failure()`:
|
||||
|
||||
report_job_log.py /tmp/build.log
|
||||
|
||||
The header says *failed* by default, which is what that guard means. A step
|
||||
guarded by `if: always()` -- a probe, or a job whose output is the point rather
|
||||
than its exit code -- has to say so, or the comment reports a failure that did
|
||||
not happen:
|
||||
|
||||
report_job_log.py /tmp/build.log --status "${{ job.status }}"
|
||||
|
||||
Everything else comes from the Actions environment: `GITHUB_REPOSITORY`,
|
||||
`GITHUB_EVENT_PATH` (for the PR number), `GITEA_TOKEN`/`GITHUB_TOKEN`, and
|
||||
`GITHUB_SERVER_URL`.
|
||||
`GITHUB_EVENT_PATH` (for the PR number), `GITEA_TOKEN`/`GITHUB_TOKEN`,
|
||||
`GITHUB_SERVER_URL`, and `JOB_STATUS` if `--status` is not passed.
|
||||
|
||||
Remove this once the instance is updated and logs can be read directly.
|
||||
"""
|
||||
@@ -41,6 +48,51 @@ DEFAULT_TAIL = 12000
|
||||
# nix colours its output, and the raw escapes are noise in Markdown.
|
||||
ANSI = re.compile(r"\x1b\[[0-9;]*[A-Za-z]")
|
||||
|
||||
# The header used to hardcode "failed". Every caller guards the step with
|
||||
# `if: failure()`, so that was true by construction -- until an `if: always()`
|
||||
# probe in weblib-viewer#10 posted a failure report for a job that had passed.
|
||||
# The default stays `failed` so those guarded callers are unchanged.
|
||||
DEFAULT_STATUS = "failed"
|
||||
|
||||
# `${{ job.status }}` yields success/failure/cancelled/skipped; a human writing
|
||||
# the flag by hand reaches for passed/failed. Accept both spellings so a call
|
||||
# site can pass the expression straight through.
|
||||
STATUS_PHRASES = {
|
||||
"failed": "failed",
|
||||
"failure": "failed",
|
||||
"passed": "passed",
|
||||
"success": "passed",
|
||||
"succeeded": "passed",
|
||||
"cancelled": "was cancelled",
|
||||
"canceled": "was cancelled",
|
||||
"skipped": "was skipped",
|
||||
}
|
||||
|
||||
|
||||
def status_phrase(status):
|
||||
"""The verb for the comment header.
|
||||
|
||||
An unrecognised status is reported verbatim rather than rejected. This
|
||||
script exists because a red job says nothing, so it must not itself become
|
||||
the thing that says nothing: `argparse`'s `choices=` would exit 2 on a
|
||||
status this table has not heard of, and the log would never be posted.
|
||||
"""
|
||||
key = (status or "").strip().lower()
|
||||
if not key:
|
||||
key = DEFAULT_STATUS
|
||||
if key in STATUS_PHRASES:
|
||||
return STATUS_PHRASES[key]
|
||||
return "finished with status `" + status.strip() + "`"
|
||||
|
||||
|
||||
def missing_log_note(path, phrase):
|
||||
"""What to say when the log file the step named is not there."""
|
||||
if phrase == "failed":
|
||||
return (f"The step produced no log file at `{path}` — it failed "
|
||||
"before the build started.")
|
||||
return (f"The step produced no log file at `{path}` — nothing was "
|
||||
"captured.")
|
||||
|
||||
|
||||
def pull_request_number(event_path):
|
||||
"""The PR this job is running for, from the event payload, or None."""
|
||||
@@ -123,6 +175,11 @@ def main(argv=None):
|
||||
ap.add_argument("--pr", type=int, default=None)
|
||||
ap.add_argument("--job", default=os.environ.get("GITHUB_JOB") or "job")
|
||||
ap.add_argument("--tail", type=int, default=DEFAULT_TAIL)
|
||||
ap.add_argument("--status",
|
||||
default=os.environ.get("JOB_STATUS") or DEFAULT_STATUS,
|
||||
help="outcome of the job being reported: failed (the "
|
||||
"default, and what `if: failure()` means), passed, "
|
||||
"cancelled, skipped, or ${{ job.status }} verbatim")
|
||||
args = ap.parse_args(argv)
|
||||
|
||||
if not args.repo or not args.token:
|
||||
@@ -138,14 +195,15 @@ def main(argv=None):
|
||||
print("no pull request for this run: nothing to comment on")
|
||||
return 0
|
||||
|
||||
phrase = status_phrase(args.status)
|
||||
|
||||
if not os.path.exists(args.log):
|
||||
text = ("The step produced no log file at "
|
||||
f"`{args.log}` — it failed before the build started.")
|
||||
text = missing_log_note(args.log, phrase)
|
||||
else:
|
||||
text = "```\n" + tail(args.log, args.tail).rstrip() + "\n```"
|
||||
|
||||
sha = (os.environ.get("GITHUB_SHA") or "")[:8]
|
||||
body = (f"**`{args.job}` failed**{f' on `{sha}`' if sha else ''}.\n\n"
|
||||
body = (f"**`{args.job}` {phrase}**{f' on `{sha}`' if sha else ''}.\n\n"
|
||||
"Job logs return 500 on this Gitea, so here is the tail of the "
|
||||
"build output, posted by [`report_job_log.py`]"
|
||||
"(https://git.chaosbit.de/weblib/weblib-ci).\n\n"
|
||||
|
||||
Reference in New Issue
Block a user