Skip to content

Fix order-dependent test failures caused by leaking dump_into global - #100

Merged
idlesign merged 1 commit into
idlesign:masterfrom
GriffTanen:fix/test-dump-leak
Aug 31, 2026
Merged

idlesign merged 1 commit into
idlesign:masterfrom
GriffTanen:fix/test-dump-leak

Conversation

@GriffTanen

Copy link
Copy Markdown
Contributor

On current master four tests fail when the suite is run as a whole:

FAILED tests/test_main.py::test_fullcycle - FileNotFoundError
FAILED tests/trackers/test_eniahd.py::test_get_torrent - AssertionError: Not all requests have been executed
FAILED tests/trackers/test_kinozal.py::test_get_torrent - AssertionError: Not all requests have been executed
FAILED tests/trackers/test_nnmclub.py::test_get_torrent - AssertionError: Not all requests have been executed

Each of those files passes on its own, which is what makes it confusing.

Cause. --dump stores the target directory in GlobalParam, which is backed by
a thread local and never reset. The CLI smoke test runs ['walk', '-f', '--dump', '/tmp/'],
so from that point on dump_contents() is enabled for every test running later in the
same thread. Subsequent tests then try to write scraped pages into /tmp/ — which fails
outright on Windows (FileNotFoundError), and on any platform makes tracker tests
misbehave.

Fix. Reset the global in the existing autouse fixture, next to the config reset
that is already there.

With this change: 58 passed, 1 skipped, and every test file still passes individually.

🤖 Generated with Claude Code

`--dump` sets a thread local global that is never reset, so the CLI smoke test
running `walk -f --dump /tmp/` leaves dumping enabled for every test that runs
after it in the same thread, making unrelated tests fail depending on order.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@coveralls

Copy link
Copy Markdown

Coverage Report for CI Build 33317750056

Coverage decreased (-0.2%) to 67.794%

Details

  • Coverage decreased (-0.2%) from the base build.
  • Patch coverage: No coverable lines changed in this PR.
  • 3 coverage regressions across 1 file.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

3 previously-covered lines in 1 file lost coverage.

File Lines Losing Coverage Coverage
src/torrt/utils.py 3 91.42%

Coverage Stats

Coverage Status
Relevant Lines: 1686
Covered Lines: 1143
Line Coverage: 67.79%
Coverage Strength: 4.06 hits per line

💛 - Coveralls

@GriffTanen

Copy link
Copy Markdown
Contributor Author

A note on the red Coveralls check, since it is a direct consequence of the fix rather than a regression.

The three lines that "lost coverage" are the body of dump_contents() that actually writes a file:

filename = filename % {'ts': time()}

with (Path(dump_into) / filename).open('wb') as f:
    f.write(contents)

Previously they were executed by every test that made a request after the CLI smoke test ran walk -f --dump /tmp/, because the leaked global kept dumping enabled for the rest of the session. That incidental execution is exactly what this PR removes, so those lines are now exercised only by the test that genuinely asks for a dump — hence -0.2%.

The build jobs themselves pass on all six matrix entries, which they did not before this change.

Happy to add a small dedicated test for dump_contents() if you would rather keep the coverage number where it was.

@idlesign idlesign added the bug label Aug 31, 2026
@idlesign
idlesign merged commit 0acbe74 into idlesign:master Aug 31, 2026
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants