Fix tests and cleanup other stuff #1277

litetex · 2025-02-12T21:21:38Z

I carefully read the contribution guidelines and agree to them.

Overview of the changes:

Implemented Ratelimiting for REAL and RECORDING downloader
- Currently YT and Bandcamp require this
- Using a fork of guava's Ratelimiter
- Removed old "rate-limiter" from RECORDING
Use a more modern TLS configuration
- This fixes Bandcamp tests not working on Windows 10 with Java 17+. Their CDN (Fastly) is likely blocking the outdated offered ciphers.
Fixed tests
Disabled some broken tests that currently have no fixes
- for example: YT's age-restriced videos
Re-enabled a few disabled tests
Updated test dependencies
Removed YT channel mixes as these no longer exist
Use new Bandcamp autocomplete API
Removed unused visitorData generation code
YT now has 100% mock test coverage. This should be further improved with Mock downloader for everything #1276 in the future
Cleaned up a ton of smaller code/code style problems
Updated gradle to latest version (8.12.1) to fix compatibility with newer Java Versions

⚠️ Note:
YT still sometimes randomly fails on testMoreItems when doing real test (I suspect a new A/B test being rolled out).
I also observed this behavior inside the app but with a very low probability (1:20+)

It looks like YT removed this around autumn 2024 Other mentions: https://www.reddit.com/r/youtube/comments/1dhuvyo/channel_mixes_disappeared/

TeamNewPipe#1273

TeamNewPipe#1274

This fixes the following problem: Bandcamp/their Fastly CDN fails with 403 responses when running tests * on Windows * with Java 17+ * and OkHttp's APPROVED_CIPHER_SUITES (default) are used

…ral way

* Bandcamp also has rate-limiting (but a lot more req are allowed than with YT) * Shorten default warm up time * Fix typo

Use ``UncheckedIOException`` instead This no longer forces one to always write ``throws IOException`` in tests

Stypox

Thank you for all of these changes! Maybe some of the stuff could have been broken up in multiple PRs but I guess it would have taken too much time. The commits were well separated, I reviewed them one by one and they look good to me. :-)

Small criticism: please put [YouTube]/[BandCamp]/... at the beginning of service-specific commit messages.

Maybe rate-limiting should be added to NewPipe too, since we currently already do some rate limiting e.g. in the feed manager.

...g/schabi/newpipe/extractor/services/youtube/linkHandler/YoutubeStreamLinkHandlerFactory.java

...chabi/newpipe/extractor/services/youtube/extractors/YoutubeMusicArtistInfoItemExtractor.java

AudricV · 2025-02-13T18:35:35Z

extractor/src/main/java/org/schabi/newpipe/extractor/playlist/PlaylistInfo.java

-        /**
-         * A mix made only of streams from (or related to) the same channel, for example YouTube
-         * channel mixes
-         */
-        MIX_CHANNEL,
-


Are you really sure that channel mixes are gone? Even if they are gone from tests, maybe they aren't on the whole platform. This concept may be applicable to other services too.

This is a breaking change, this should precised in the PR description, so you're also required to add if you tested the changes in NewPipe and if you want to make a PR in the app repo to fix breaking changes, if applicable (from the PR template).

Yeah it might make sense to keep the code, you never know if YouTube might readd them

I just now noticed that this is inside org.schabi.newpipe.extractor.playlist and not in inside youtube. Thought it was in youtube (because these mix type only exist for YT and nothing else).

I will reintroduce the enum but deprecate it.

Are you really sure that channel mixes are gone?

Yes for around 7-8 months.
See description of 451bb13

Yeah it might make sense to keep the code, you never know if YouTube might readd them

I think it has no sense to keep unused code around as it only creates a maintenance burden.
One can also still get it from the git history if really needed ;)

extractor/src/main/java/org/schabi/newpipe/extractor/utils/ProtoBuilder.java

I expected this class to be located in ``youtube`` as it mostly contains YT specific playlist types, but this assumption was wrong. See also: https://github.com/TeamNewPipe/NewPipeExtractor/pull/1277/files/d9caa90d239571dfa394923ab7b7c35004454a06#r1955161108

litetex · 2025-02-15T13:37:57Z

@Stypox @AudricV
I think if there are no further objections this can be merged so that we can go on and fix other stuff :)

Stypox

I can add this as a comment to the code if you wish ;)

Yes please :-)

> > I think you can use YoutubeParsingHelper.getTextFromObject() > That was used there before however I did not choose to use it again. > > ``runs`` returns an array with 3 items containing: > 0: ``<artistname>`` > 1: ``•`` > 2: ``<subscriberCount>`` > > There is 2 problems with that: > 1. It's inefficient for extracting the subscription count > 2. If for some reason the artist now has a name like "2k" the extraction malfunctions See also: TeamNewPipe#1277 (comment)

Solved

* Implement rate-limiting * Disabled age restricted test as it's currently not working * Fix unexpected error due to malformed url * Channel has a new name * YouTube channel mixes no longer exist It looks like YT removed this around autumn 2024 Other mentions: https://www.reddit.com/r/youtube/comments/1dhuvyo/channel_mixes_disappeared/ * Disable YT Shorts UI TeamNewPipe/NewPipeExtractor#1273 * Add missing override annotations * Disable irrelevant test * Fix code style * MetaInfo Search is was removed or there is none active anymore TeamNewPipe/NewPipeExtractor#1274 * Remove outdated comment * Move into dedicated package * Fix PeerTube tests * Backport/Sync code from NewPipe implementation * Update gradle * Enforce modern TLS1.2+ This fixes the following problem: Bandcamp/their Fastly CDN fails with 403 responses when running tests * on Windows * with Java 17+ * and OkHttp's APPROVED_CIPHER_SUITES (default) are used * Update test dependencies * Remove recording retry/throttle as this is now handled in a more general way * No random errors * Use new Bandcamp autocomplete api * Fix duplicated ; * Re-Enable test as it's no longer broken * YT: Suggestion test is working again * Fix YT artist getSubscriberCount extraction * Add rate limiter with default cold factor * Improve rate limiting * Bandcamp also has rate-limiting (but a lot more req are allowed than with YT) * Shorten default warm up time * Fix typo * Make all YT tests in MOCK mode use mock data * Downloader: Don't force IOException Use ``UncheckedIOException`` instead This no longer forces one to always write ``throws IOException`` in tests * YouTube 100% Mock coverage * Use camelCase * Add recorded mock data * Removed unused code * Fix okhttp deprecation * Revert potentially breaking change I expected this class to be located in ``youtube`` as it mostly contains YT specific playlist types, but this assumption was wrong. See also: https://github.com/TeamNewPipe/NewPipeExtractor/pull/1277/files/d9caa90d239571dfa394923ab7b7c35004454a06#r1955161108 * Only include ``jsr305`` and not complete spotbugs Only ``javax.annotation.Nonull`` or ``javax.annotation.Nullable`` are currently in used which means that spotbugs can and should be removed. See TeamNewPipe/NewPipeExtractor#1278 for details * Improve documentation > > I think you can use YoutubeParsingHelper.getTextFromObject() > That was used there before however I did not choose to use it again. > > ``runs`` returns an array with 3 items containing: > 0: ``<artistname>`` > 1: ``•`` > 2: ``<subscriberCount>`` > > There is 2 problems with that: > 1. It's inefficient for extracting the subscription count > 2. If for some reason the artist now has a name like "2k" the extraction malfunctions See also: TeamNewPipe/NewPipeExtractor#1277 (comment) * Update gradle-wrapper to 8.12.1 * Improve github workflow * Cache gradle wrapper distributions * Don't always show gradle welcome message * Deduplicated code * Make it possible to select downloader and manually execute a run * Bump org.jsoup:jsoup from 1.17.2 to 1.18.3 Bumps [org.jsoup:jsoup](https://github.com/jhy/jsoup) from 1.17.2 to 1.18.3. - [Release notes](https://github.com/jhy/jsoup/releases) - [Changelog](https://github.com/jhy/jsoup/blob/master/CHANGES.md) - [Commits](jhy/jsoup@jsoup-1.17.2...jsoup-1.18.3) --- updated-dependencies: - dependency-name: org.jsoup:jsoup dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> * Fix tests Upstream changes are described here: jhy/jsoup#1278 They are effectively irrelevant as YT removes the ``"``/attributes anyway. * Bump org.mozilla:rhino from 1.7.15 to 1.8.0 Bumps [org.mozilla:rhino](https://github.com/mozilla/rhino) from 1.7.15 to 1.8.0. - [Release notes](https://github.com/mozilla/rhino/releases) - [Changelog](https://github.com/mozilla/rhino/blob/master/RELEASE-NOTES.md) - [Commits](https://github.com/mozilla/rhino/commits) --- updated-dependencies: - dependency-name: org.mozilla:rhino dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> * Improve tests * Also use rhino engine * Fix compile problems in JavaScript class * Cleanup test * Fix compile problems and optimize TokenStream * Fix typo * Rework TokenStream * Consolidate Rhino Version * Use Files methods in tests. * Fix compilation + Cleanup * Bump org.junit:junit-bom from 5.11.4 to 5.12.0 Bumps [org.junit:junit-bom](https://github.com/junit-team/junit5) from 5.11.4 to 5.12.0. - [Release notes](https://github.com/junit-team/junit5/releases) - [Commits](junit-team/junit-framework@r5.11.4...r5.12.0) --- updated-dependencies: - dependency-name: org.junit:junit-bom dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> * Fix tests execution See gradle/gradle#32534 (comment) * Bump org.jsoup:jsoup from 1.18.3 to 1.19.1 Bumps [org.jsoup:jsoup](https://github.com/jhy/jsoup) from 1.18.3 to 1.19.1. - [Release notes](https://github.com/jhy/jsoup/releases) - [Changelog](https://github.com/jhy/jsoup/blob/master/CHANGES.md) - [Commits](jhy/jsoup@jsoup-1.18.3...jsoup-1.19.1) --- updated-dependencies: - dependency-name: org.jsoup:jsoup dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> * removed use of reflection in PatternsManager.java. For this the Code to generate the Localization files was extracted into its own submodule. The generator now also creates a class that Holds a Map of all Localizations for the timeago-parser. * removed obsolete proguard rules * Use org.schabi.newpipe.timeago_generator as package name * getPattern final parameter * [YouTube] Fix crash on SABR-only player responses, do not use WEB client * Release v0.24.6 --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: litetex <[email protected]> Co-authored-by: Stypox <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Isira Seneviratne <[email protected]> Co-authored-by: TobiGr <[email protected]> Co-authored-by: Thilo Kogge <[email protected]> Co-authored-by: thigg <[email protected]> Co-authored-by: AudricV <[email protected]>

litetex added 30 commits February 9, 2025 19:57

Implement rate-limiting

f0ed76c

Disabled age restricted test as it's currently not working

6e82a80

Fix unexpected error due to malformed url

ddc7f3e

Channel has a new name

ee7d8b0

YouTube channel mixes no longer exist

451bb13

It looks like YT removed this around autumn 2024 Other mentions: https://www.reddit.com/r/youtube/comments/1dhuvyo/channel_mixes_disappeared/

Disable YT Shorts UI

51586c0

TeamNewPipe#1273

Add missing override annotations

271a465

Disable irrelevant test

d746d1b

Fix code style

61ef169

MetaInfo Search is was removed or there is none active anymore

1a9df85

TeamNewPipe#1274

Remove outdated comment

f1b9098

Move into dedicated package

e1b94c4

Fix PeerTube tests

c892cfa

Backport/Sync code from NewPipe implementation

ad5ca4e

Update gradle

12e2145

Enforce modern TLS1.2+

351174a

This fixes the following problem: Bandcamp/their Fastly CDN fails with 403 responses when running tests * on Windows * with Java 17+ * and OkHttp's APPROVED_CIPHER_SUITES (default) are used

Update test dependencies

460e0f7

Remove recording retry/throttle as this is now handled in a more gene…

9f94a29

…ral way

No random errors

59a2f1a

Use new Bandcamp autocomplete api

1966ad1

Fix duplicated ;

fa99003

Re-Enable test as it's no longer broken

ee6bc84

YT: Suggestion test is working again

25e4a6f

Fix YT artist getSubscriberCount extraction

4d7df14

Add rate limiter with default cold factor

287123b

Improve rate limiting

cf5df3f

* Bandcamp also has rate-limiting (but a lot more req are allowed than with YT) * Shorten default warm up time * Fix typo

Make all YT tests in MOCK mode use mock data

1772624

Downloader: Don't force IOException

3b34b82

Use ``UncheckedIOException`` instead This no longer forces one to always write ``throws IOException`` in tests

YouTube 100% Mock coverage

69d5bf2

Use camelCase

daac243

litetex added 3 commits February 12, 2025 22:01

Add recorded mock data

2f2e2df

Removed unused code

4c3918a

Fix okhttp deprecation

d9caa90

litetex force-pushed the fix-tests branch from 8c3368a to d9caa90 Compare February 12, 2025 21:29

Stypox requested changes Feb 13, 2025

View reviewed changes

...g/schabi/newpipe/extractor/services/youtube/linkHandler/YoutubeStreamLinkHandlerFactory.java Show resolved Hide resolved

...chabi/newpipe/extractor/services/youtube/extractors/YoutubeMusicArtistInfoItemExtractor.java Show resolved Hide resolved

AudricV previously requested changes Feb 13, 2025

View reviewed changes

ShareASmile added codequality Improvements to the codebase to improve the code quality tests Issues and PR related to unit tests labels Feb 14, 2025

litetex mentioned this pull request Feb 15, 2025

Bump com.google.code.gson:gson from 2.11.0 to 2.12.1 #1270

Closed

Stypox approved these changes Feb 15, 2025

View reviewed changes

Stypox merged commit b1ecb68 into TeamNewPipe:dev Feb 15, 2025
4 checks passed

litetex deleted the fix-tests branch February 15, 2025 14:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Fix tests and cleanup other stuff #1277

Fix tests and cleanup other stuff #1277

Uh oh!

litetex commented Feb 12, 2025 •

edited

Loading

Uh oh!

Stypox left a comment

Uh oh!

Uh oh!

Uh oh!

AudricV Feb 13, 2025

Uh oh!

Stypox Feb 13, 2025

Uh oh!

litetex Feb 13, 2025

Uh oh!

litetex Feb 13, 2025

Uh oh!

Uh oh!

litetex commented Feb 15, 2025

Uh oh!

Stypox left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Fix tests and cleanup other stuff #1277

Fix tests and cleanup other stuff #1277

Uh oh!

Conversation

litetex commented Feb 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Stypox left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

AudricV Feb 13, 2025

Choose a reason for hiding this comment

Uh oh!

Stypox Feb 13, 2025

Choose a reason for hiding this comment

Uh oh!

litetex Feb 13, 2025

Choose a reason for hiding this comment

Uh oh!

litetex Feb 13, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

litetex commented Feb 15, 2025

Uh oh!

Stypox left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

litetex commented Feb 12, 2025 •

edited

Loading