Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Test] Improve download test #29944

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open

Conversation

dirkf
Copy link
Contributor

@dirkf dirkf commented Sep 11, 2021

Boilerplate: own code/improvementPlease follow the guide below

Before submitting a pull request make sure you have:

In order to be accepted and merged into youtube-dl each piece of code must be in public domain or released under Unlicense. Check one of the following options:

  • I am the original author of this code and I am willing to release it under Unlicense
  • I am not the original author of this code but it is in public domain or released under Unlicense (provide reliable evidence)

What is the purpose of your pull request?

  • Bug fix
  • Improvement
  • New extractor
  • New feature

Description of your pull request and other information

This PR makes two small and one larger improvement to test/test_download.py, in 3 separate commits.

Calculating the file hash

External downloaders may not support limiting the download exactly to a specified size. When running a test download, only a short initial portion of the file is supposed to be downloaded, and the hash is calculated from that portion. If the portion is longer than requested the hash will be incorrect. The PR explicitly uses the requested length, if specified, when calculating the file hash, so that the same value should be calculated regardless of downloader.

Header-related params in test cases

Header-related params (here: --user-agent, --referer, --add-header) are parsed into std_headers rather than the params dict in the main yt-dl program. In download tests, only params that are set into the params dict were supported, and header-related params in test cases were ignored. This PR changes the params processing for test downloads to match what happens in the main program.

As an example, the change makes it possible to specify a test case with a specific UA, such as this for an Invidious site fronted by Cloudflare

        ...
        'params': {
            # Cloudflare breaks HTTP if Chrome is mentioned in the UA (2021-08)
            'user_agent': 'Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/6533.18.5 (KHTML, like Gecko) Safari/6533.18.5',
        },                                       
        ...

Add TestSuite test_..._all with all downloads for each extractor

When testing downloads for an extractor XXX, a set of test cases is created but it's not obvious which item in the extractor's _TESTS list corresponds to which TestDownload.test_XXX... test case. If there are many test cases one might fail to run some of the later tests, or run past the highest numbered case.

To facilitate running the download tests for an extractor in the same way that they run in the CI servers, the PR adds a further download test case with name suffix _all for each extractor class that has test case(s). The new test case is a unittest.TestSuite that runs all the other download tests for that extractor. Counts of the possible test outcomes (error, failure, etc) are printed.

Thus to test YoutubeIE:

python test/test_download.py TestDownload.test_Youtube_all

Supersedes, closes #27461.

External downloaders may not support limiting the download to a specified size
Header-related params are parsed into `std_headers` rather than
the `params` dict, but this wasn't respected for test cases.
So TestDownload.test_XXX_all runs all the download test cases for
the XXX extractor.
test/test_download.py Outdated Show resolved Hide resolved
test/test_download.py Outdated Show resolved Hide resolved
@dirkf
Copy link
Contributor Author

dirkf commented Oct 18, 2022

Note to self:

  • also handle case where a fragment is downloaded that is smaller than expected by the original code.

Co-authored-by: Simon Sawicki <accounts@grub4k.xyz>
test/test_download.py Show resolved Hide resolved
test/test_download.py Outdated Show resolved Hide resolved
test/test_download.py Outdated Show resolved Hide resolved
test/test_download.py Outdated Show resolved Hide resolved
test/test_download.py Outdated Show resolved Hide resolved
test/test_download.py Outdated Show resolved Hide resolved
test/test_download.py Outdated Show resolved Hide resolved
* make fake `report_warning()` method signatures correct (per 640d39f)
* support single warning to expect as well as sequence
* don't colour text to be matched
* use `expected_warnings()` function throughout
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.

None yet

2 participants