Skip to content

test(build-context): guard the second FIFO case on mkfifo availability - #502

Open
kevin9327 wants to merge 1 commit into
NVIDIA:mainfrom
kevin9327:fix/build-context-fifo-skip-guard
Open

test(build-context): guard the second FIFO case on mkfifo availability#502
kevin9327 wants to merge 1 commit into
NVIDIA:mainfrom
kevin9327:fix/build-context-fifo-skip-guard

Conversation

@kevin9327

Copy link
Copy Markdown

Problem

tests/nodes/test_build_context.py has two adjacent cases that build a named pipe to prove the
ledger keeps non-regular entries. One guards the primitive, the other does not:

def test_build_context_records_non_regular_files_in_the_ledger(tmp_path: Path) -> None:
    """Named pipes are inventoried so the cache phase can report their failure."""
    if not hasattr(os, "mkfifo"):
        pytest.skip("named pipes are unavailable on this platform")
    pipe = tmp_path / "events.pipe"
    os.mkfifo(pipe)
def test_build_context_records_non_regular_entries_in_the_ledger(tmp_path: Path) -> None:
    """A discovered FIFO is retained as failed ledger evidence, never silently skipped."""
    fifo = tmp_path / "inspection.pipe"
    os.mkfifo(fifo)          # no guard

os.mkfifo does not exist on Windows, so the second case raises AttributeError during setup.
Its assertion — that a discovered FIFO is retained as failed ledger evidence rather than
silently dropped — is never evaluated, and the result is indistinguishable from a real
regression in that ledger behaviour.

Fix

Apply the guard the sibling case already uses, with the same wording. Two lines; no change on
any platform that has os.mkfifo, so CI (ubuntu-latest) continues to execute the case
exactly as before.

Reproduction

Windows 11, Python 3.12.10, against unmodified main (704bc95). Both cases selected together
so the asymmetry is visible in one run:

$ python -m pytest -p no:randomly tests/nodes/test_build_context.py -k "records_non_regular" \
    -q --no-header -rs
sF                                                                       [100%]
================================== FAILURES ===================================
________ test_build_context_records_non_regular_entries_in_the_ledger _________

    def test_build_context_records_non_regular_entries_in_the_ledger(tmp_path: Path) -> None:
        """A discovered FIFO is retained as failed ledger evidence, never silently skipped."""
        fifo = tmp_path / "inspection.pipe"
>       os.mkfifo(fifo)
        ^^^^^^^^^
E       AttributeError: module 'os' has no attribute 'mkfifo'

tests\nodes\test_build_context.py:1058: AttributeError
=========================== short test summary info ===========================
SKIPPED [1] tests\nodes\test_build_context.py:1004: named pipes are unavailable on this platform
1 failed, 1 skipped, 78 deselected in 4.57s

With this change, same command:

ss                                                                       [100%]
=========================== short test summary info ===========================
SKIPPED [1] tests\nodes\test_build_context.py:1004: named pipes are unavailable on this platform
SKIPPED [1] tests\nodes\test_build_context.py:1058: named pipes are unavailable on this platform
2 skipped, 78 deselected in 3.06s

What must still hold

tests/nodes/test_build_context.py as a whole — exactly one case changes state, and the passing
set is untouched:

failed passed skipped
before 12 66 2
after 11 66 3

The eleven that remain are unrelated to named pipes and are not touched here.

ruff check tests/All checks passed!
ruff format --check tests/126 files already formatted

Diff is 1 file changed, 2 insertions(+).

test_build_context_records_non_regular_files_in_the_ledger checks
hasattr(os, "mkfifo") before creating a named pipe. Its neighbour
test_build_context_records_non_regular_entries_in_the_ledger calls
os.mkfifo directly, so on a platform without named pipes it raises
AttributeError during setup instead of reporting that it cannot run.

Apply the guard the sibling case already uses.

Signed-off-by: kevin9327 <kevin9327@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant