Skip to content

fix(tests): return actual download count in run_cli to catch missing output files - #156

Closed
snushev wants to merge 1 commit into
LeyckerS:mainfrom
snushev:fix-test-no-chrome
Closed

fix(tests): return actual download count in run_cli to catch missing output files#156
snushev wants to merge 1 commit into
LeyckerS:mainfrom
snushev:fix-test-no-chrome

Conversation

@snushev

@snushev snushev commented Aug 7, 2026

Copy link
Copy Markdown

Closes #155. Removed the fallback that forced ok count to len(urls) when done == 0, preventing false positive test passes when no files are actually written.

@snushev

snushev commented Aug 7, 2026

Copy link
Copy Markdown
Author

My bad, noticed the mocked behavior

@snushev snushev closed this Aug 7, 2026
@LeyckerS

LeyckerS commented Aug 7, 2026

Copy link
Copy Markdown
Owner

@snushev — you spotted it yourself and closed it before anyone had to say a word, which is the right instinct and I would rather have that than a defended patch. No harm done at all.

For the record, since it is a genuinely tricky one: the stubs write no files, so len(os.listdir(out)) is 0 and run_cli substitutes len(urls). Removing the fallback alone leaves ok == 0 against len(urls) == 4, which turns a vacuous pass into a false failure. And it turns out there is a further wrinkle neither of us had: the same helper is used by the engine tests, where ok comes from engine.snapshot() and is real — so the assertion is dead through two callers and live through two others. I had that wrong in the issue too and have corrected it there.

#155 was already reserved for @XEDAB, who reported it — that is on my listings for not making it obvious enough, not on you.

If you would like something that is genuinely free right now, #83 is the most interesting one open: the interface offers one "streams" number for both providers while datanodes is hard-capped at 8 lanes, so the number on screen is not the number in effect — measured at roughly 40 of 48 slots busy for one provider against 4 for the other. #151 is smaller and more self-contained if you would rather. Say the word on either and it is yours.

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.

test_no_chrome.py reports success when the run produced no files, so the ok assertion cannot fail

2 participants