Skip to content

refactor(util): share one stdlib-only Actions helper across the CI scripts (#2047) - #2050

Merged
JarryShaw merged 1 commit into
mainfrom
refactor/2047-actions-helper
Oct 22, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
refactor/2047-actions-helper

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

What is the purpose of your pull request?

  • refactor — restructures code without changing behaviour

Description of your pull request and other information

The first PR for #2047 (owner's ruling: option (a)).

  • New util/_actions.py: standard library only and 3.7-syntax. It provides annotation/annotate, group, set_output (with the heredoc form for multiline values), summary/open_summary, say/emit, exit_status and run. The tee-to-log that pip_install and regenerate_samples both copied becomes run. Escaping follows GitHub's workflow-commands reference.
  • Scripts moved onto it: apt_packages, classify_changes, coverage_report, leg_summary, pip_install, regenerate_samples and vendor_code_changed. Each keeps its exact output, with escape=False where a script escapes its own way. On /usr/bin/python3.7, main's and this branch's copies gave byte-identical stdout, stderr, exit code, outputs file and summary file on 10 cases.
  • Not moved yet:
    • The annotations in apt_packages, leg_summary and coverage_report stay literal (marked #2047). ci: a failed KTLO leg turns main and pull requests red; report it as a warning and exit 0 #1672's KTLO guard (commands() in test_ktlo_legs_stay_green_1672.py) reads literal print('::level text, and routing them through the helper would blind it. Teaching the guard to see _actions.annotate comes next.
    • verdict_covers_checkout.py, superseded.py and project_status.py are left out: each runs from a single-file checkout or download.
  • Tests: the new tests/project/test_actions_helper_2047.py checks the stdlib-only and 3.7-parse rules, the exact output and escaping, and that every migrated script loads both as util.x and from its file path. It errors without the helper and passes here (21 passed). Every existing test of the migrated scripts passes unchanged, and none was edited.

@JarryShaw JarryShaw added ci Pull requests that change CI or workflow configuration (ci: subject prefix) refactor Restructuring for its own sake — neither a fix nor a new capability (refactor: prefix) review: running A cross-review is in flight against the current head - no verdict yet labels Oct 22, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

GOOD TO GO at 17c9c8692 (Sonnet cross-review). The reviewer confirmed:

  • Behaviour unchanged: byte-identical exit code, stdout, stderr, outputs file and summary file on 43 adversarial cases each under 3.7 and 3.14. These covered unset and empty env vars, a signal-killed subprocess, non-UTF-8 bytes, %, :, , and \n in leg names, and LC_ALL=C.
  • Escaping: matches escapeData/escapeProperty in @actions/core.
  • Imports: every caller runs the script from a full checkout as python util/x.py.
  • KTLO guard: ci: a failed KTLO leg turns main and pull requests red; report it as a warning and exit 0 #1672's guard loses nothing on any script a KTLO step runs.

Being tightened before ready: the new test's 3.7 check lets f"{x=}" through, and its stdlib check uses the running interpreter's module list, so tomllib passes. It also never runs a script as python util/x.py. The worker is fixing all three, and this will get a delta re-review.

@JarryShaw JarryShaw moved this to In review in PyPCAPKit Oct 22, 2026
…ripts (#2047)

- util/_actions.py: annotation()/annotate() with GitHub's escaping (and
  escape=False for callers that keep their own), group(), set_output()
  (multiline for a \n or \r), summary()/open_summary(), emit()/say(),
  exit_status() and run(), the tee of a command to stdout and a log.
  Standard library only, 3.7 syntax.
- apt_packages, classify_changes, coverage_report, leg_summary, pip_install,
  regenerate_samples, vendor_code_changed: use it for outputs, summaries,
  byte writes, exit statuses and the logged run; output is unchanged.
  Annotations that #1672's scan of KTLO steps reads stay literal.
- tests/project/test_actions_helper_2047.py pins the helper's output and
  imports to 3.7, and runs each script as the steps do; workflows.rst
  lists the helper.
@JarryShaw
JarryShaw force-pushed the refactor/2047-actions-helper branch from 17c9c86 to 967d907 Compare October 22, 2026 19:39
@JarryShaw

Copy link
Copy Markdown
Owner Author

Pushed 967d90722 (was 17c9c8692). This tightens the new test, which now checks for real that the helper is safe on 3.7 and standard library only:

  • Imports: the helper's imports are checked against a fixed list of modules that exist on 3.7.
  • f-strings: a tokenize check rejects f"{x=}".
  • Scripts run for real: each migrated script is run as python util/<script>.py and must import _actions from util/.
  • Output: set_output takes the multiline form for \r as well as \n.

Each check fails on a planted violation; I re-planted tomllib and f"{os=}" myself, and both failed. Delta re-review in progress.

@JarryShaw

Copy link
Copy Markdown
Owner Author

GOOD TO GO at 967d90722 (Sonnet delta re-review). The reviewer seeded one violation for each new check and saw each one fail:

  • Import allowlist: a function-local import tomllib fails the import check.
  • f-string check: f"{x=}" fails the tokenize check, which flags the = forms and spares ==, walrus and :=^5. Its skip below 3.12 only affects experimental legs, since LATEST runs on the newest CPython.
  • Subprocess test: forcing from util import _actions fails it, so it tells the two import branches apart.

No set_output caller can produce \r, so no output changes. With the KTLO guard and apt tests: 83 passed.

@JarryShaw JarryShaw added review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: running A cross-review is in flight against the current head - no verdict yet labels Oct 22, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

should we separate action helper scripts from other project maintenance scripts in the util folder? or everything is actually already action helpers?

@JarryShaw

Copy link
Copy Markdown
Owner Author

Almost everything in util/ already is an Actions helper. Of the scripts on main, every one but pyshark_encap_map.py is invoked by a workflow. They fall into two groups:

  • Actions plumbing, reading or writing GITHUB_*: apt_packages, ci_timing, classify_changes, coverage_report, discussion_report, leg_summary, project_status, python_versions, superseded, update_python_versions, vendor_code_changed, vendor_test_gate, verdict_covers_checkout.
  • Dual-use: run by workflows, but also run by hand. bump_version, cap_file_size, changelog_md, conda-build, conda_vendor, pip_install, pytest_shards, regenerate_samples, run_unittest_leg, wheel_rename. For example, run_unittest_leg is how the ordering check runs locally.

So the line between the groups would be fuzzy, and a move costs churn everywhere: every run: python util/x.py, every util.x import in tests, the sparse checkouts and the single-file download in project-status.yml, plus _actions' import path.

Needs you:

This PR is ready to merge either way.

@JarryShaw JarryShaw added the needs: decision Waiting on the maintainer to decide — not blocked by other work label Oct 22, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

take (a)

@JarryShaw

Copy link
Copy Markdown
Owner Author

Recorded: (a). util/ stays flat. The scripts table in workflows.rst gains a column saying whether each script is CI-only or also run by hand. This PR adds its own row to that table, so I will make the column change as a docs-only follow-up once this merges, rather than move this head.

@JarryShaw JarryShaw removed the needs: decision Waiting on the maintainer to decide — not blocked by other work label Oct 22, 2026
@JarryShaw JarryShaw moved this from In review to Needs decision in PyPCAPKit Oct 22, 2026
@JarryShaw JarryShaw moved this from Needs decision to In review in PyPCAPKit Oct 22, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Coverage: 91.54% (unit tier, Python 3.15, 967d90722, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 2 95.16%
pcapkit/const 19599 881 2618 867 92.11%
pcapkit/corekit 2567 71 816 37 96.57%
pcapkit/dumpkit 282 3 102 3 98.44%
pcapkit/foundation 2930 23 1022 25 98.79%
pcapkit/interface 138 4 52 3 96.32%
pcapkit/protocols 17436 219 4582 185 98.15%
pcapkit/toolkit 872 36 298 10 95.21%
pcapkit/utilities 529 7 160 5 98.26%
pcapkit/vendor 4431 1995 1018 167 51.73%

Per-file detail: the coverage-html artifact of this run.

@JarryShaw

Copy link
Copy Markdown
Owner Author

Ordering check on 967d90722: the project leg (util/run_unittest_leg.py project) ran 1867 tests, OK (71 skipped). CI: 61 checks green, 0 failing. KTLO 3.8, 3.9 and 3.10 legs: 0 failing tests. Ready to merge. The workflows.rst column from your ruling follows as a docs-only change once this lands.

@JarryShaw
JarryShaw merged commit 0ab1e95 into main Oct 22, 2026
63 checks passed
@github-project-automation github-project-automation Bot moved this from In review to Done in PyPCAPKit Oct 22, 2026
@JarryShaw
JarryShaw deleted the refactor/2047-actions-helper branch October 22, 2026 20:48
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 22, 2026
JarryShaw added a commit that referenced this pull request Oct 23, 2026
The scripts table gains a "By hand" column, and a second table lists the
scripts a person runs, with the command: bump_version, changelog_md,
run_unittest_leg, conda_vendor --check, ci_timing and pyshark_encap_map.
Every other util/ script is CI-only, as ruled on #2050 (keep util/ flat).
JarryShaw added a commit that referenced this pull request Oct 25, 2026
…R roles (#1103)

- Cite issues and pull requests by :issue:/:pr: in rendered text: the
  hand-written GitHub links in workflows.rst, releasing.rst and pep.rst,
  the bare #NNN in workflows.rst, and the #NNN in pcapkit docstrings and
  #: comments. #2050, linked as an issue, is a pull request.
- Add tests/project/test_docs_issue_pr_roles_1103_unit.py, which fails on
  a hand-written link or bare #NNN in .rst pages, docstrings and #:
  comments; its allowlist holds only the references other tests pin.
- Record the rule on the documentation conventions page.
- Name the external action functions releasing.rst and workflows.rst cited
  by line, where no test pins the line.
- Fix stale claims: the shared changelog pull request has merged, the
  Windows leg runs leg_summary.py and pip_install.py, Lint expects pylint's
  import-error rather than tracking a count, pypcapfile fails from 3.12 up
  rather than working only on 3.10 and 3.11, cryptography in the all extra
  is no pure-Python wheel; drop "today", "this change" and "current".

Docs build: no warnings, and the same 116 under -n as origin/main.
@JarryShaw JarryShaw added this to the PyPCAPKit v1.5 milestone Oct 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci Pull requests that change CI or workflow configuration (ci: subject prefix) refactor Restructuring for its own sake — neither a fix nor a new capability (refactor: prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant