Skip to content

stream.hls_filtered: fix tests #3194

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

Merged

Conversation

bastimeyer
Copy link
Member

@bastimeyer bastimeyer commented Sep 22, 2020

  • Refactor test setup and properly tear down threads
  • Re-order tests
  • Change mocked request URLs to temporarily work around bleeding threads
    from previously run HLSStream tests which are sometimes still making
    requests on the same URLs and causing the mocked playlist URL to
    incorrectly advance one call and breaking the current stream worker

Resolves #3187
See #3187 (comment)

As mentioned in the linked comment, test_hls.py and test_twitch.py need to be fixed at some point, so that they gracefully tear down their spawned HLSStream threads, so that the same mocked request URLs don't get queried in tests which are run (directly) afterwards. Using unique mocked request URLs in each test would make this extra safe.


All tests successfully ran 50 times in a row on all CI runners on my test-account's fork:
https://github.com/bastimeyer-test/streamlink-test/actions/runs/266975807

- Refactor test setup and properly tear down threads
- Re-order tests
- Change mocked request URLs to temporarily work around bleeding threads
  from previously run HLSStream tests which are sometimes still making
  requests on the same URLs and causing the mocked playlist URL to
  incorrectly advance one call and breaking the current stream worker
@bastimeyer bastimeyer added the bug label Sep 22, 2020
@bastimeyer bastimeyer mentioned this pull request Sep 22, 2020
@codecov
Copy link

codecov bot commented Sep 22, 2020

Codecov Report

Merging #3194 into master will increase coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3194      +/-   ##
==========================================
+ Coverage   52.65%   52.67%   +0.01%     
==========================================
  Files         244      244              
  Lines       15687    15687              
==========================================
+ Hits         8260     8263       +3     
+ Misses       7427     7424       -3     

@bastimeyer
Copy link
Member Author

Btw, I believe the slightly incorrect coverage results in some PRs are a result of the other HLSStream related tests not tearing down properly. The code lines which are run sometimes and sometimes not are these:

@gravyboat
Copy link
Member

Nice this looks good, thanks @bastimeyer.

@gravyboat gravyboat merged commit 09ec66b into streamlink:master Sep 22, 2020
@bastimeyer bastimeyer deleted the stream/hls_filtered/fix-test branch October 9, 2020 06:52
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.

2 participants