Skip to content

test: cover geth fixture cleanup on errors#3851

Open
vuyua9 wants to merge 1 commit into
ApeWorX:mainfrom
vuyua9:devb/geth-fixture-cleanup-test
Open

test: cover geth fixture cleanup on errors#3851
vuyua9 wants to merge 1 commit into
ApeWorX:mainfrom
vuyua9:devb/geth-fixture-cleanup-test

Conversation

@vuyua9
Copy link
Copy Markdown

@vuyua9 vuyua9 commented May 24, 2026

What was wrong?

start_geth_process_and_yield_port now uses a finally block to stop the geth process when a test fails after the fixture yields, but that cleanup path was not covered by a focused regression test.

Related to the go-ethereum integration test fixture cleanup path.

How was it fixed?

Added a small test that drives the fixture generator directly with monkeypatched geth process helpers. The test throws a consumer exception into the generator and asserts that:

  • kill_proc_gracefully() is called
  • proc.communicate(timeout=5) is called
  • the process is killed before communicate() is invoked

Todo:

  • Clean up commit history
  • Add or update documentation related to these changes: not needed, test-only regression coverage
  • Add entry to the release notes: added newsfragments/3851.internal.rst

Tests

  • git diff --check
  • .venv/bin/python -m pytest tests/integration/go_ethereum/test_conftest_cleanup.py -q

Note: uv pip install -e '.[test]' hit a local safe-pysha3 build issue in this macOS CommandLineTools environment, so I installed the package plus the focused dependencies needed for this test and ran the targeted pytest above.

Cute Animal Picture

red panda

@vuyua9 vuyua9 force-pushed the devb/geth-fixture-cleanup-test branch from 165f155 to 0b94e82 Compare May 24, 2026 15:31
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