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
12 changes: 7 additions & 5 deletions tests/_tiers.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,7 @@
* :func:`is_unit_tier` -- which tier does this module belong to?
* :func:`committed_captures` -- which captures does *git* track? Asked of git
rather than hardcoded, because a hardcoded list of names silently rots the
moment somebody commits another capture (there are six today, not the two
the rule started with).
moment somebody commits another capture, or stops committing one.
* :func:`audit_module` -- does this module read a generated capture without
handling its absence?
* :func:`check_unit_tier_read` -- may this particular
Expand Down Expand Up @@ -64,9 +63,12 @@
are entitled to, e.g. :file:`tests/protocols/test_option_coverage_runtime.py`,
which imports :data:`SAMPLE_ROOT` from here and joins names onto it -- legal,
because that tier runs only once the fixtures exist. The one unit-tier module that
touches :data:`SAMPLE_ROOT` at all, :file:`tests/test_tier_guard.py`, only stats
:file:`in.pcap`, which is committed. But nothing stops the next unit-tier module
from opening a *generated* capture that way, and the shape stays invisible here.
touches :data:`SAMPLE_ROOT` at all, :file:`tests/test_tier_guard.py`, stats
:file:`in.pcap` directly -- which is committed -- and separately loops over every
name :func:`committed_captures` reports, each committed by construction, since that
is git's own tracked listing rather than a name the test made up. But nothing stops
the next unit-tier module from opening a *generated* capture that way, and the shape
stays invisible here.

Flagging it in general was considered and rejected, with a measurement behind the
decision. A rule as broad as "any string literal ending in ``.pcap``" matches 18
Expand Down
4 changes: 2 additions & 2 deletions tests/integration/_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ class EndToEndTestCase(unittest.TestCase):

Gives every test a private temporary directory in :attr:`tmp_path` and an
:meth:`extract` wrapper that closes the input stream on teardown. Captures
under :file:`examples/captures/` are fixtures -- four of them are committed
-- so nothing here ever writes outside :attr:`tmp_path`.
under :file:`examples/captures/` are fixtures -- some of them committed --
so nothing here ever writes outside :attr:`tmp_path`.

"""

Expand Down
70 changes: 67 additions & 3 deletions tests/test_tier_guard.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
* the tier rule here still matches the one CI runs
(:class:`TierClassificationTests`, :class:`WorkflowAgreementTests`);
* committedness comes from git rather than from a list of names that goes stale
the moment a seventh capture is committed (:class:`CommittedCaptureTests`);
the moment another capture is committed (:class:`CommittedCaptureTests`);
* a violation is caught and explained, the legitimate reads next to it are not,
and several violations in one module are listed in the order those violations
appear in the file rather than in the order a tree walk happened to reach them
Expand All @@ -28,6 +28,7 @@

import pathlib
import re
import subprocess
import tempfile
import textwrap
import unittest
Expand Down Expand Up @@ -131,17 +132,80 @@ def test_committed_set_comes_from_the_index(self) -> None:
self.assertNotIn('test.pcap', tracked)

def test_every_tracked_name_exists_and_matches_git(self) -> None:
"""The set is the index's answer verbatim, prefix stripped."""
"""The set is the index's answer verbatim, prefix stripped.

"Verbatim" is checked by re-deriving the index here, independently of
:func:`~tests._tiers.committed_captures` -- a second call to that same
function would only prove it agrees with itself, not that it agrees
with git. A hardcoded, stale, or otherwise wrong literal set would fail
the comparison below; it could only have passed the old body, which
asserted nothing but non-emptiness and the absence of a ``/``.

"""
tracked = _tiers.committed_captures()
assert tracked is not None
self.assertTrue(tracked, 'git tracks no capture at all, which cannot be right')

relative_root = _tiers.SAMPLE_ROOT.relative_to(_tiers.ROOT).as_posix()
# Shell out directly rather than through `_tiers._git` -- going through
# the implementation's own helper would only show that
# `committed_captures()` agrees with itself, not with git. The failure
# tolerance mirrors `_git`'s for the same reason it exists there: no
# executable, no repository, or a non-zero exit is "cannot tell", not
# "the sets disagree", and a source tarball's test run should not fail
# for a rule it cannot possibly break.
try:
completed = subprocess.run(
('git', 'ls-files', '-z', '--', relative_root),
cwd=str(_tiers.ROOT), stdout=subprocess.PIPE, stderr=subprocess.DEVNULL,
timeout=30, check=False,
)
except (OSError, subprocess.SubprocessError) as exc:
self.skipTest(f'git ls-files could not be run independently: {exc}')
if completed.returncode != 0:
self.skipTest('git ls-files exited non-zero on a fresh, independent re-derivation')

listing = completed.stdout.decode('utf-8', 'surrogateescape')
prefix = relative_root + '/'
expected = {
entry[len(prefix):] for entry in listing.split('\0')
if entry and entry.startswith(prefix)
}
self.assertEqual(
tracked, expected,
'committed_captures() disagrees with a freshly re-derived `git ls-files`'
)

for name in tracked:
with self.subTest(capture=name):
self.assertNotIn('/', name, 'names are relative to examples/captures/')
self.assertTrue(
(_tiers.SAMPLE_ROOT / name).is_file(),
f'git tracks examples/captures/{name} but it is not on disk'
)

def test_capture_suggestions_are_captures(self) -> None:
"""The replacement suggestion offers captures, not reference outputs."""
"""The suggestion is the tracked set filtered to captures and sorted.

Equality against a set built independently of
:func:`~tests._tiers.committed_capture_names` is the point, but on an
empty tracked set it holds vacuously -- both sides would be ``()`` --
so the ``assertTrue`` guard below comes first, the same guard its
sibling ``test_every_tracked_name_exists_and_matches_git`` carries for
the same reason. Only past that guard do the two example-name
assertions add anything: they are satisfied by any list that happens
to contain ``in.pcap`` and omit ``out.txt``, hardcoded or not, so they
stay a readable sanity check on top of the real one rather than the
thing holding the test up.

"""
tracked = _tiers.committed_captures()
assert tracked is not None
self.assertTrue(tracked, 'git tracks no capture at all, which cannot be right')
expected = tuple(sorted(name for name in tracked if name.endswith(_tiers.CAPTURE_SUFFIXES)))

suggestions = _tiers.committed_capture_names()
self.assertEqual(suggestions, expected)
self.assertIn('in.pcap', suggestions)
self.assertNotIn('out.txt', suggestions)

Expand Down
Loading