Repository navigation
Fix order-dependent test failures caused by leaking dump_into global - #100
Conversation
`--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>
Coverage Report for CI Build 33317750056Coverage decreased (-0.2%) to 67.794%Details
Uncovered ChangesNo uncovered changes found. Coverage Regressions3 previously-covered lines in 1 file lost coverage.
Coverage Stats
💛 - Coveralls |
|
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 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 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 |
On current
masterfour tests fail when the suite is run as a whole:Each of those files passes on its own, which is what makes it confusing.
Cause.
--dumpstores the target directory inGlobalParam, which is backed bya 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 thesame thread. Subsequent tests then try to write scraped pages into
/tmp/— which failsoutright on Windows (
FileNotFoundError), and on any platform makes tracker testsmisbehave.
Fix. Reset the global in the existing
autousefixture, next to the config resetthat is already there.
With this change:
58 passed, 1 skipped, and every test file still passes individually.🤖 Generated with Claude Code