Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions .github/workflows/create-release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,11 @@ jobs:
released="$(mktemp -d)"
git show "${from}:${tiers}" > "${released}/python-versions.toml"
git show "${from}:util/python_versions.py" > "${released}/python_versions.py"
# #2047: the reader imports util/_actions.py from beside it, from the
# commit that has it.
if git cat-file -e "${from}:util/_actions.py" 2> /dev/null; then
git show "${from}:util/_actions.py" > "${released}/_actions.py"
fi
wheels="$(python3 "${released}/python_versions.py" --file "${released}/python-versions.toml" pypi-wheels)"
builds="$(python3 "${released}/python_versions.py" --file "${released}/python-versions.toml" conda-builds)"
fi
Expand Down
4 changes: 3 additions & 1 deletion .github/workflows/deploy-pages.yml
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,8 @@ jobs:
# `deploy-pages` ran or `head`'s gate failed, so never on a pull request's
# run or a verdict for another commit,
# which skip `deploy-pages` deliberately. It reads no artifact, and checks
# out the script alone. The job and its step are `continue-on-error`;
# out the script and util/_actions.py, which it imports (#2047), alone. The
# job and its step are `continue-on-error`;
# util/discussion_report.py writes a warning rather than failing.
discussion:
name: Post the Pages report
Expand All @@ -218,6 +219,7 @@ jobs:
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
with:
sparse-checkout: |
util/_actions.py
util/discussion_report.py
sparse-checkout-cone-mode: false
persist-credentials: false
Expand Down
2 changes: 2 additions & 0 deletions .github/workflows/experimental.yml
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ jobs:
with:
sparse-checkout: |
.github/python-versions.toml
util/_actions.py
util/python_versions.py
util/superseded.py
sparse-checkout-cone-mode: false
Expand Down Expand Up @@ -100,6 +101,7 @@ jobs:
with:
sparse-checkout: |
.github/python-versions.toml
util/_actions.py
util/python_versions.py
sparse-checkout-cone-mode: false
persist-credentials: false
Expand Down
16 changes: 10 additions & 6 deletions .github/workflows/project-status.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,10 @@ name: Project Status
# the classic "pwn request". The job below:
#
# * never checks out the repository, at any ref, and has no `uses:` step at all;
# * fetches util/project_status.py through the API at `github.sha`, which under
# * fetches util/project_status.py, and util/_actions.py, which it imports
# (#2047), through the API at `github.sha`, which under
# `pull_request_target` is the tip of the *base* branch, never the PR head --
# so editing the script in a pull request changes nothing about its own run;
# so editing either in a pull request changes nothing about its own run;
# * takes nothing from the event payload but the item number, passed through
# `env:` rather than interpolated into the script, and re-reads labels and
# state from the API.
Expand Down Expand Up @@ -84,10 +85,13 @@ jobs:
exit 0
fi

# The base branch's copy of the script -- see the security note above.
gh api -H 'Accept: application/vnd.github.raw' \
"repos/${GITHUB_REPOSITORY}/contents/util/project_status.py?ref=${SCRIPT_REF}" \
> "${RUNNER_TEMP}/project_status.py"
# The base branch's copy of the script, and of util/_actions.py, which
# it imports from beside it (#2047) -- see the security note above.
for script in project_status.py _actions.py; do
gh api -H 'Accept: application/vnd.github.raw' \
"repos/${GITHUB_REPOSITORY}/contents/util/${script}?ref=${SCRIPT_REF}" \
> "${RUNNER_TEMP}/${script}"
done

if [ -n "$ITEM" ]; then
python3 "${RUNNER_TEMP}/project_status.py" --item "$ITEM"
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/python-compatibility.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ jobs:
with:
sparse-checkout: |
.github/python-versions.toml
util/_actions.py
util/python_versions.py
sparse-checkout-cone-mode: false
persist-credentials: false
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/unit-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ jobs:
with:
sparse-checkout: |
.github/python-versions.toml
util/_actions.py
util/python_versions.py
sparse-checkout-cone-mode: false
persist-credentials: false
Expand Down
9 changes: 6 additions & 3 deletions docs/source/contributing/workflows.rst
Original file line number Diff line number Diff line change
Expand Up @@ -503,8 +503,10 @@ release waits for the next Vendor Update on ``main``.
that runs with a secret on an event a fork can cause: it writes the project board's
*Status* field with ``PROJECT_TOKEN``, which a fork's ``pull_request`` run would not
receive. It therefore never checks out the repository and has no ``uses:`` step. It
fetches :file:`util/project_status.py` through the API at ``github.sha`` -- the base
branch's tip under that event -- and takes only the item number from the payload.
fetches :file:`util/project_status.py`, and :file:`util/_actions.py`, which it imports
(`#2047 <https://github.com/JarryShaw/PyPCAPKit/issues/2047>`__), through the API at
``github.sha`` -- the base branch's tip under that event -- and takes only the item
number from the payload.
``tests/project/test_project_status.py`` asserts this over the file. Without the
secret it skips with a ``::notice::``. The Status mapping it applies is documented in
:doc:`conventions/process`.
Expand Down Expand Up @@ -1365,7 +1367,8 @@ does while its findings stand, and ``failure`` where actionlint failed the job.
``cron-vendor.yml``'s rules: ``push`` on its ``workflow_run``, ``scheduled`` on its
``schedule``, and never on a pull request or a verdict for another commit. Its
report is the results of ``head``, ``deploy-pages`` and ``publish-pages``, so it
uploads no artifact, and its ``discussion`` job checks out the script alone. The
uploads no artifact, and its ``discussion`` job checks out the script and
:file:`util/_actions.py`, which it imports, alone. The
``--status`` is the publish's result after a green build, the build's where the
build did not pass, and the gate's where the gate skipped the build.

Expand Down
54 changes: 50 additions & 4 deletions tests/project/test_actions_helper_2047.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
function's exact output, GitHub's escaping included.
* :class:`TestScriptsImportIt` loads each script that imports the helper in the
ways the steps and the tests do, and runs it as the steps do.
* :class:`TestTheStepsCheckItOut` holds every sparse checkout of such a script
to listing the helper too.

"""

Expand All @@ -29,10 +31,11 @@
from typing import TYPE_CHECKING
from unittest import mock

from tests._workflows import WORKFLOWS, load
from util import _actions

if TYPE_CHECKING:
from typing import Iterator, Optional
from typing import Any, Iterator, Optional

#: Test tier (#1958): reads and runs the tree's own scripts, a question about the tree.
__tier__ = 'LATEST'
Expand All @@ -41,9 +44,10 @@
HELPER = ROOT / 'util' / '_actions.py'
#: The scripts that import the helper.
SCRIPTS = ('apt_packages', 'check_suite_report', 'ci_timing', 'classify_changes', 'codeql_report',
'coverage_report', 'leg_summary', 'pip_install', 'regenerate_samples', 'release_status', 'release_tags',
'release_upload', 'release_vendor_update', 'update_python_versions', 'vendor_code_changed',
'vendor_test_gate')
'coverage_report', 'discussion_report', 'leg_summary', 'pip_install', 'project_status',
'python_versions', 'regenerate_samples', 'release_status', 'release_tags', 'release_upload',
'release_vendor_update', 'superseded', 'update_python_versions', 'vendor_code_changed',
'vendor_test_gate', 'verdict_covers_checkout')
#: The top-level modules the helper may import, each in Python 3.7's standard
#: library: the running interpreter's :data:`sys.stdlib_module_names` would
#: pass ``tomllib``, which 3.7 lacks.
Expand All @@ -56,16 +60,21 @@
'classify_changes': ([], {'OWN_PR': 'false'}),
'codeql_report': (['--help'], {}),
'coverage_report': (['--help'], {}),
'discussion_report': (['--help'], {}),
'leg_summary': (['ktlo'], {'LEG': 'Python 3.8 (KTLO, non-blocking)', 'STEPS': '{}'}),
'pip_install': (['--help'], {}),
'project_status': (['--help'], {}),
'python_versions': (['latest'], {}),
'regenerate_samples': (['--help'], {}),
'release_status': (['--help'], {}),
'release_tags': (['--help'], {}),
'release_upload': (['--help'], {}),
'release_vendor_update': (['--help'], {}),
'superseded': (['--help'], {}),
'update_python_versions': (['--help'], {}),
'vendor_code_changed': (['--help'], {}),
'vendor_test_gate': (['--help'], {}),
'verdict_covers_checkout': (['--help'], {}),
}
#: ``-X importtime``'s line for a module, by its name.
IMPORT_TIME = re.compile(r'^import time:.*\|\s*(\S+)\s*$', re.M)
Expand Down Expand Up @@ -330,5 +339,42 @@ def test_run_as_the_steps_run_it(self) -> None:
self.assertEqual(output.read_text(encoding='utf-8'), 'code=true\n')


def sparse_checkouts_missing_the_helper(spec: 'Any') -> 'list[tuple[str, list[str]]]':
"""Each job of the workflow ``spec`` whose sparse checkout lists a script of :data:`SCRIPTS` but not the helper."""
found = []
for job_id, job in (spec.get('jobs') or {}).items():
for step in job.get('steps') or []:
listed = str((step.get('with') or {}).get('sparse-checkout') or '').split()
scripts = [path for path in listed if path.startswith('util/') and path.endswith('.py')
and path[len('util/'):-len('.py')] in SCRIPTS]
if scripts and 'util/_actions.py' not in listed:
found.append((job_id, scripts))
return found


class TestTheStepsCheckItOut(unittest.TestCase):
"""A sparse checkout of a script that imports the helper holds the helper too, or the import fails."""

def test_every_sparse_checkout_of_a_script_lists_the_helper(self) -> None:
checked = 0
for path in sorted(WORKFLOWS.glob('*.yml')):
with self.subTest(workflow=path.name):
spec = load(path.name)
self.assertEqual(sparse_checkouts_missing_the_helper(spec), [])
checked += sum('sparse-checkout' in (step.get('with') or {}) for job in spec['jobs'].values()
for step in job.get('steps') or [])
self.assertGreater(checked, 0, 'no sparse checkout was found to check')

def test_the_check_sees_a_missing_helper(self) -> None:
def spec(*listed: 'str') -> 'Any':
return {'jobs': {'j': {'steps': [{'with': {'sparse-checkout': ''.join(f'{p}\n' for p in listed)}}]}}}

self.assertEqual(sparse_checkouts_missing_the_helper(spec('.github/python-versions.toml',
'util/python_versions.py')),
[('j', ['util/python_versions.py'])])
self.assertEqual(sparse_checkouts_missing_the_helper(spec('util/_actions.py', 'util/python_versions.py')), [])
self.assertEqual(sparse_checkouts_missing_the_helper(spec('util/no_helper_here.py')), [])


if __name__ == '__main__':
unittest.main()
60 changes: 60 additions & 0 deletions tests/project/test_discussion_report_2030.py
Original file line number Diff line number Diff line change
Expand Up @@ -438,6 +438,66 @@ def test_a_bad_command_line_exits_2(self) -> None:
self.assertEqual(self.calls(), [])


class TestTheWorkflowCommands(_FakeGitHub):
"""#2047: the whole of what the script prints and appends to the summary, byte for byte.

The tests above read the warning with ``assertIn``; these pin each run's whole
standard output and summary file, with text each escape changes, so moving the
writes onto util/_actions.py can be shown to change nothing.

"""

def test_a_post_prints_two_lines_and_appends_one(self) -> None:
title = 'ci-timing: 50% [odd], title'
self.summary.write_bytes(b'before\n')
result = self.script(title=title)
url = f'https://github.com/{REPO}/discussions/100'
self.assertEqual((result.returncode, result.stdout, result.stderr),
(0, f"Opened the thread {title!r}: {url}\nPosted the report: {url}#discussioncomment-1\n", ''))
self.assertEqual(self.summary.read_bytes(),
f'before\nThe report was posted to [{title}]({url}#discussioncomment-1).\n'.encode())
# A second post finds the thread, and appends a second line.
result = self.script(title=title)
self.assertEqual(result.stdout, f'Posted the report: {url}#discussioncomment-2\n')
self.assertEqual(self.summary.read_bytes().count(b'\n'), 3)

def test_no_summary_file_or_one_that_will_not_open(self) -> None:
"""Unset or empty, nothing is written; a directory, the post still succeeds, silently."""
url = f'https://github.com/{REPO}/discussions/100'
for name, value in (('unset', None), ('empty', ''), ('a directory', str(self.tmp))):
with self.subTest(summary=name):
self.update(discussions=[])
env = self.env()
if value is None:
del env['GITHUB_STEP_SUMMARY']
else:
env['GITHUB_STEP_SUMMARY'] = value
result = self.script(env=env)
self.assertEqual((result.returncode, result.stdout, result.stderr),
(0, f"Opened the thread {TITLE!r}: {url}\nPosted the report: {url}"
'#discussioncomment-1\n', ''))
self.assertFalse(self.summary.exists())

def test_each_warning_is_one_escaped_line(self) -> None:
"""``%`` and LF in the message escape; the title, which holds neither ``:`` nor ``,``, is as written.

The stand-in's standard error reaches the script through universal
newlines, so a CR arrives as LF; the helper's own tests pin CR.

"""
self.update(fail={'op': 'discussionCategories', 'message': 'a 50%\nb: c, d'})
self.summary.write_bytes(b'before\n')
result = self.script()
self.assertEqual((result.returncode, result.stdout),
(0, '::warning title=CI report not posted::gh api graphql exited 1: gh: a 50%25%0Ab: '
'c, d. The report was not posted.\n'))
self.assertEqual(self.summary.read_bytes(), b'before\n')
missing = self.tmp / '50%.md'
result = self.script(report=missing)
self.assertEqual(result.stdout, f"::warning title=CI report not posted::the report '{self.tmp}/50%25.md' "
'could not be read (No such file or directory), so nothing was posted.\n')


#: ``(workflow, reporting job, the thread's title, the artifact, the file in it)``.
POSTERS = (
('ci-timing.yml', 'report', 'ci-timing: weekly CI timing report', 'ci-timing-report', 'report.md'),
Expand Down
5 changes: 3 additions & 2 deletions tests/project/test_lint_pages_reports_2030.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,12 +159,13 @@ def test_lint_hands_its_report_on(self) -> None:
self.assertEqual(download['with'], {'name': 'lint-report', 'path': 'report'})
self.assertIs(download['continue-on-error'], True)

def test_pages_reads_no_artifact_and_checks_out_the_script_alone(self) -> None:
def test_pages_reads_no_artifact_and_checks_out_the_script_and_its_helper(self) -> None:
job = load(PAGES)['jobs']['discussion']
uses = [entry['uses'] for entry in job['steps'] if 'uses' in entry]
self.assertEqual(len(uses), 1)
self.assertTrue(pinned(uses[0], 'actions/checkout'))
self.assertEqual(job['steps'][0]['with'], {'sparse-checkout': 'util/discussion_report.py\n',
# The script imports util/_actions.py (#2047).
self.assertEqual(job['steps'][0]['with'], {'sparse-checkout': 'util/_actions.py\nutil/discussion_report.py\n',
'sparse-checkout-cone-mode': False,
'persist-credentials': False})

Expand Down
Loading
Loading