JuanBindez / pytubefix

Python3 library for downloading YouTube Videos.
https://pytubefix.readthedocs.io
MIT License
722 stars 100 forks source link

playlist tests require working DNS #317

Open benaryorg opened 2 weeks ago

benaryorg commented 2 weeks ago

Describe the bug Two playlist tests are currently failing in the nixpkgs tests (ignore the cipher tests, I opened a second PR to fix those) as they require DNS to resolve which fails in a sandboxed environment (such as the nix build environment). Since all other tests pass just fine and there's mocking in place I figure that these tests specifically are doing some additional requests which somehow bypass the mocking, which would mean that a) tests suites may currently cause requests against the live YouTube API which may have adverse effects on people running the test suite locally (like being treated as bots by YouTube and being unable to watch without an account like many others) and b) the tests may not be pure and fail depending on upstream changes which runs counter to the intention of those tests.


code that was used that resulted in the bug

The logs above contain a full trace, however it seems that the pagination bypasses the mock:

https://github.com/JuanBindez/pytubefix/blob/5303b640c3513b734c9edc9294ff0530dda16602/pytubefix/contrib/playlist.py#L325-L327

And is triggered in these two tests:

FAILED tests/contrib/test_playlist.py::test_playlist_failed_pagination - urllib.error.URLError: <urlopen error [Errno -3] Temporary failure in name ...
FAILED tests/contrib/test_playlist.py::test_playlist_pagination - urllib.error.URLError: <urlopen error [Errno -3] Temporary failure in name ...

Expected behavior Tests should pass even when DNS resolution is not available. Tests should not make requests to the YouTube API.


Desktop (please complete the following information):


Additional context I'd proceed to disable those tests downstream for now, and it's not terribly important, so this is more of a nice to have, and if those tests are made to work without DNS I'd be happy to enable them again.

benaryorg commented 2 weeks ago

Oh my, it seems I've entirely missed that a large chunk of tests have already been disabled as they require network access. My bad.

It'd be cool if they could run without network access, but given that there are already many disabled tests I see no urgency whatsoever.