Skip to content

Conversation

@bastimeyer
Copy link
Member

Fixes #3451

Changes are split into two commits:

  1. Replace static reference to the HLSStreamReader in HLSStream.open()
  2. Fix HLS writer race conditions (see commit message)
    This explicitly awaits all writer calls in filtered HLS tests, like the Twitch HLS tests for example.

I've been running all HLS tests in a loop for almost an hour now without any failures, so I think this should be good.

while pytest -q tests/plugins/test_twitch.py tests/streams/test_hls.py tests/streams/test_hls_filtered.py; do true; done

- immediately set read_once event in HLSStreamReadThread
- rename _TestSubjectHLSStreamWriter to EventedHLSStreamWriter
- move EventedHLSStreamWriter and await_write
  from test_hls_filtered to stream_hls test mixin
- add write_calls and timeout parameters to await_write
- await all writer calls in TestTwitchHLSStream
@bastimeyer bastimeyer added bug tests WIP Work in process stream: HLS and removed WIP Work in process labels Dec 26, 2020
@bastimeyer
Copy link
Member Author

This should be good to go I think. I've tagged the WIP tag earlier today because I was checking something related to requests_mock in the HLS tests, but it's not worth the effort and the current changes are good enough. As said, the tests should now be reliable.

@back-to back-to merged commit ae0ec17 into streamlink:master Dec 27, 2020
Billy2011 added a commit to Billy2011/streamlink-27 that referenced this pull request Dec 28, 2020
Billy2011 added a commit to Billy2011/streamlink-27 that referenced this pull request Dec 28, 2020
- immediately set read_once event in HLSStreamReadThread
- rename _TestSubjectHLSStreamWriter to EventedHLSStreamWriter
- move EventedHLSStreamWriter and await_write
  from test_hls_filtered to stream_hls test mixin
- add write_calls and timeout parameters to await_write
- await all writer calls in TestTwitchHLSStream

streamlink#3454

patches for test_hls_filtered not yet applied
@bastimeyer bastimeyer deleted the tests/hls-await-writer branch January 19, 2021 23:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Test is flaky: TestTwitchHLSStream.test_hls_disable_ads_has_midstream

2 participants