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

[MediaCCC] Fix regressions and improve MediaCCCLiveStreamKioskExtractor #1097

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

TobiGr
Copy link
Member

@TobiGr TobiGr commented Aug 19, 2023

  • I carefully read the contribution guidelines and agree to them.
  • I have tested the API against NewPipe.
  • I agree to create a pull request for NewPipe as soon as possible to make it compatible with the changed API.

Changes

  • Fix wrong ListLinkHandlerFactories for kiosks. Regression introduced in Add support for channel tabs and channel tags #1082. I checked for similar regressions, but did not find any.
  • Live stream kiosk: detect pause "talks" segments. Add and improve tests for MediaCCCLiveStreamKioskExtractor:
    • test stream items if a live stream is running
    • use mock tests to check live talk extraction and testing conferences

To Do

  • Make MediaCCCLiveStreamListExtractorTest.[PreparationTest|LiveConferenceTest] always use mocks.

@TobiGr TobiGr added bug Issue is related to a bug media.ccc.de service, https://media.ccc.de labels Aug 19, 2023
@TobiGr TobiGr marked this pull request as draft August 19, 2023 18:48
@TobiGr TobiGr marked this pull request as ready for review August 19, 2023 19:41
Add and improve tests for MediaCCCLiveStreamKioskExtractor:
- test stream items if a live stream is running
- use mock tests to check live talk extraction and testing conferences
@AudricV AudricV changed the title Fix regressions and improve MediaCCCLiveStreamKioskExtractor [MediaCCC] Fix regressions and improve MediaCCCLiveStreamKioskExtractor Sep 23, 2023
}

@Override
public String getName() throws ParsingException {
return roomInfo.getObject("talks").getObject("current").getString("title");
if (isBreak()) {
return roomInfo.getString("display") + " - Pause";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How MediaCCC's website displays paused rooms? I don't think hardcoding - Pause at the end of a room name is a good idea for localization purposes.

Comment on lines +86 to +88
* <p>Reset cached live stream data.</p>
* This is a temporary method which can be used to reset the cached live stream data until a
* caching policy for {@link #getLiveStreams(Downloader, Localization)} is implemented.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* <p>Reset cached live stream data.</p>
* This is a temporary method which can be used to reset the cached live stream data until a
* caching policy for {@link #getLiveStreams(Downloader, Localization)} is implemented.
* Reset cached live stream data.
*
* <p>
* This is a temporary method which can be used to reset the cached live stream data until a
* caching policy for {@link #getLiveStreams(Downloader, Localization)} is implemented.
* </p>

Comment on lines +8 to +9
* Clears static media.ccc.de states.
* <p>This method needs to be called in every class before running and recording mock tests.</p>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Clears static media.ccc.de states.
* <p>This method needs to be called in every class before running and recording mock tests.</p>
* Clears static MediaCCC states.
*
* <p>
* This method needs to be called in every class before running and recording mock tests.
* </p>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue is related to a bug media.ccc.de service, https://media.ccc.de
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants