ci: fix shellcheck findings and trim comments in unit-tests.yml (#1967) by JarryShaw · Pull Request #2037 · JarryShaw/PyPCAPKit

JarryShaw · 2026-10-22T08:22:35Z

Searched for similar pull requests
Followed the coding style (make pylint, make mypy, make isort)
make test passes, and a test case covers the change
Added a changelog entry — N/A — centralised in docs(changelog): shared 1.5.0 changelog — long-lived, merges last (#610, #616, #617, #618, #620) #657

What is the purpose of your pull request?

ci — workflows or build tooling

Description of your pull request and other information

#1967 step 5, part 2, covering unit-tests.yml. Stacked on #2034, so review only the top commit.

Fixes the last actionlint finding, SC2086, in pypcap-parity's "Run full test suite" (its KTLO twin picks this up via the alias). The selection is split with read -ra and passed as "${selection[@]}". integration's and project-tests' versions get the same treatment, which drops two # shellcheck disable=SC2086 lines.
Trims comments using ci: fix shellcheck findings and trim workflow comments (#1967) #2023's rules: drop history narration, run ids, timing tables and restatements of the YAML, and keep each non-obvious why with its issue ref. Every comment that a test pins or that another workflow points at stays.
Adds tests/project/test_unit_tests_shellcheck_1967.py, which shellchecks every run: block the way actionlint 1.7.12 does and skips when shellcheck is absent.

Parsed before and after, the workflow is identical apart from run: strings. Only six of those change: five carry the fix above and one is comment text in the engine cell script. actionlint 1.7.12 now reports nothing on any workflow. The new test fails on #2034's file (2 failed) and passes here. tests/project plus every file reading unit-tests.yml: 1657 passed, 71 skipped.

Follow-up: lint.yml still says actionlint stays advisory while findings remain, and that is no longer true.

JarryShaw · 2026-10-22T08:32:54Z

GOOD TO GO at 7ad513be5 (Sonnet cross-review of 1fe996ee9).

Parsed before and after, the workflow differs only in six run: strings.
The read -ra arrays give the same argv for empty, multi-space and plain-path selections. No selection holds a glob.
Each rationale and its issue ref survive in condensed form. The refs that went with it (toolkit: pypcapfile adapter reads dotted-decimal addresses as packed bytes, so 7 of 10 gated tests fail when enabled #743, engines(pypcapfile): _decode() feeds hexlified frame bytes straight to pypcapfile's decoders #746, test(vendor): a stale warning-class generation makes the snapshot-restore assertion order-dependent #985, feat(protocols)!: move OSPF and RARP to the application layer (#719) #1031, ci(unittest-ordering): shard the protocols leg into internet and the rest (#1029) #1032) were history or run-id tables.
The shellcheck test's flags match actionlint 1.7.12's rule_shellcheck.go exactly, including --norc, -x and the exclude list.

Since review I fixed the comment the review flagged in lint.yml and the test's docstring, which both still said findings remain. Known risk: the test uses the runner image's shellcheck, so a newer shellcheck could flag unchanged code.

github-actions · 2026-10-22T08:56:18Z

Coverage: 91.54% (unit tier, Python 3.15, 77368b669, Unit Tests run success)

Package	Statements	Missed	Branches	Partial	Cover
`pcapkit (top level)`	104	4	20	2	95.16%
`pcapkit/const`	19597	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 · 2026-10-22T09:04:10Z

Ordering check on 7ad513be5, which contains #2031 and #2034: python util/run_unittest_leg.py project on this tree gave Ran 1758 tests, OK (skipped=71). KTLO failing test ids: 0 across every KTLO leg. Ready to merge after #2034.

JarryShaw · 2026-10-22T14:55:04Z

Rebased onto #2034's new head as 77368b669. The tree is byte-identical and patch-id matches. Still stacked on #2034.

JarryShaw · 2026-10-22T17:14:26Z

resolve conflicts

- lint.yml: drop the actionlint step's `continue-on-error`, so a finding fails Lint; move it below the advisory steps and run it on `!cancelled()` so a finding skips none of them and a failed setup does not skip it. Version and sha256 pin unchanged. - lint.yml: the step's comment says it blocks, why that is safe now (#2023, #2037, #2038), and that the image's shellcheck can drift. - workflows.rst: Workflow Lint no longer calls the step advisory. - test_actionlint_in_lint_1967: TestItBlocks pins the blocking shape; test_unit_tests_shellcheck_1967's docstring no longer says advisory.

JarryShaw · 2026-10-22T17:22:21Z

Rebase only, no new changes: 77368b669 → 1f1921a1e. This rebase has stacked on #2034, moved onto its new head. No conflicts, and the patch-id is identical.

CI on the previous head 77368b669: 58 checks green, 0 failing; the KTLO legs fail 0 tests. The GOOD TO GO verdict carries over. You can bypass checks and merge.

- lint.yml: drop the actionlint step's `continue-on-error`, so a finding fails Lint; move it below the advisory steps and run it on `!cancelled()` so a finding skips none of them and a failed setup does not skip it. Version and sha256 pin unchanged. - lint.yml: the step's comment says it blocks, why that is safe now (#2023, #2037, #2038), and that the image's shellcheck can drift. - workflows.rst: Workflow Lint no longer calls the step advisory. - test_actionlint_in_lint_1967: TestItBlocks pins the blocking shape; test_unit_tests_shellcheck_1967's docstring no longer says advisory.

JarryShaw · 2026-10-22T18:15:00Z

Rebase only, no new changes: 1f1921a1e → ebffa0000. This is stacked on #2034 and moved onto its new head. No conflicts, and the patch-id is identical.

The head 77368b669, with the same patch, was fully green, with 0 failing KTLO tests. The GOOD TO GO verdict carries over. You can bypass checks and merge after #2034.

Step 5, part 2 of #1967, over unit-tests.yml: - Fix the SC2086 in pypcap-parity's "Run full test suite": split the selection with `read -ra` into an array, and do the same in integration's and project-tests' pytest steps, dropping their `shellcheck disable=SC2086`. Same words reach pytest, with no pathname expansion. - Trim comments: drop restatements of the YAML, history narration, measurements already in the issues, and rationale repeated across jobs; keep each non-obvious why, its issue ref, and the phrases tests pin. - Add test_unit_tests_shellcheck_1967.py, which runs shellcheck over every bash run: block as actionlint 1.7.12 does, skipping without shellcheck. actionlint 1.7.12 reports nothing on unit-tests.yml; the workflow tests pass.

- lint.yml: drop the actionlint step's `continue-on-error`, so a finding fails Lint; move it below the advisory steps and run it on `!cancelled()` so a finding skips none of them and a failed setup does not skip it. Version and sha256 pin unchanged. - lint.yml: the step's comment says it blocks, why that is safe now (#2023, #2037, #2038), and that the image's shellcheck can drift. - workflows.rst: Workflow Lint no longer calls the step advisory. - test_actionlint_in_lint_1967: TestItBlocks pins the blocking shape; test_unit_tests_shellcheck_1967's docstring no longer says advisory.

JarryShaw · 2026-10-22T18:19:46Z

Rebase only, no new changes: ebffa0000 → 894aebce4, moved onto main after #2034 merged. There were no conflicts, and the patch-id is identical.

The head 77368b669 carried the same patch and was fully green, with 0 failing KTLO tests. The GOOD TO GO verdict carries over. You can bypass checks and merge.

- lint.yml: drop the actionlint step's `continue-on-error`, so a finding fails Lint; move it below the advisory steps and run it on `!cancelled()` so a finding skips none of them and a failed setup does not skip it. Version and sha256 pin unchanged. - lint.yml: the step's comment says it blocks, why that is safe now (#2023, #2037, #2038), and that the image's shellcheck can drift. - workflows.rst: Workflow Lint no longer calls the step advisory. - test_actionlint_in_lint_1967: TestItBlocks pins the blocking shape; test_unit_tests_shellcheck_1967's docstring no longer says advisory.

JarryShaw added ci Pull requests that change CI or workflow configuration (ci: subject prefix) review: running A cross-review is in flight against the current head - no verdict yet labels Oct 22, 2026

JarryShaw added this to PyPCAPKit Oct 22, 2026

JarryShaw moved this to In review in PyPCAPKit Oct 22, 2026

JarryShaw force-pushed the ci/1967-step5-unit-tests branch 2 times, most recently from 335f24a to 7ad513b Compare October 22, 2026 08:32

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 mentioned this pull request Oct 22, 2026

ci: divide and simplify the workflows into scripts, shared heads and smaller files #1967

Closed

JarryShaw mentioned this pull request Oct 22, 2026

ci: trim comments in experimental.yml (#1967) #2038

Merged

4 of 5 tasks

JarryShaw force-pushed the ci/1967-step5-unit-tests branch from 7ad513b to 77368b6 Compare October 22, 2026 14:49

JarryShaw mentioned this pull request Oct 22, 2026

ci: make Lint's actionlint step blocking (#1967) #2043

Merged

4 of 5 tasks

JarryShaw force-pushed the ci/1967-step5-unit-tests branch from 77368b6 to 1f1921a Compare October 22, 2026 17:21

JarryShaw force-pushed the ci/1967-step5-unit-tests branch from 1f1921a to ebffa00 Compare October 22, 2026 18:14

JarryShaw force-pushed the ci/1967-step5-unit-tests branch from ebffa00 to 894aebc Compare October 22, 2026 18:19

JarryShaw merged commit c45fb6d into main Oct 22, 2026
57 checks passed

JarryShaw deleted the ci/1967-step5-unit-tests branch October 22, 2026 18:54

github-project-automation Bot moved this from In review to Done in PyPCAPKit Oct 22, 2026

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 this to the PyPCAPKit v1.5 milestone Oct 26, 2026

Navigation Menu

Repository navigation

ci: fix shellcheck findings and trim comments in unit-tests.yml (#1967) - #2037

ci: fix shellcheck findings and trim comments in unit-tests.yml (#1967)#2037
JarryShaw merged 1 commit into
mainfrom
ci/1967-step5-unit-tests

JarryShaw commented Oct 22, 2026

Uh oh!

JarryShaw commented Oct 22, 2026

Uh oh!

github-actions Bot commented Oct 22, 2026 •

edited

Loading

Uh oh!

JarryShaw commented Oct 22, 2026

Uh oh!

JarryShaw commented Oct 22, 2026

Uh oh!

JarryShaw commented Oct 22, 2026

Uh oh!

JarryShaw commented Oct 22, 2026

Uh oh!

JarryShaw commented Oct 22, 2026

Uh oh!

JarryShaw commented Oct 22, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

JarryShaw commented Oct 22, 2026

What is the purpose of your pull request?

Description of your pull request and other information

Uh oh!

JarryShaw commented Oct 22, 2026

Uh oh!

github-actions Bot commented Oct 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JarryShaw commented Oct 22, 2026

Uh oh!

JarryShaw commented Oct 22, 2026

Uh oh!

JarryShaw commented Oct 22, 2026

Uh oh!

JarryShaw commented Oct 22, 2026

Uh oh!

JarryShaw commented Oct 22, 2026

Uh oh!

JarryShaw commented Oct 22, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

github-actions Bot commented Oct 22, 2026 •

edited

Loading