Fix all tests and make everything work offline/with mocks#1332
Fix all tests and make everything work offline/with mocks#1332TobiGr merged 36 commits intoTeamNewPipe:devfrom
Conversation
|
Thank you. I was really needed to get more reliable tests and better detect flaws in PRs.
Isn't it just called |
...r/src/test/java/org/schabi/newpipe/extractor/services/DefaultSimpleUntypedExtractorTest.java
Outdated
Show resolved
Hide resolved
...src/test/java/org/schabi/newpipe/extractor/services/youtube/YoutubeChannelExtractorTest.java
Outdated
Show resolved
Hide resolved
...rc/test/java/org/schabi/newpipe/extractor/services/youtube/YoutubeCommentsExtractorTest.java
Outdated
Show resolved
Hide resolved
...test/java/org/schabi/newpipe/extractor/services/youtube/YoutubeMixPlaylistExtractorTest.java
Outdated
Show resolved
Hide resolved
...rc/test/java/org/schabi/newpipe/extractor/services/youtube/YoutubePlaylistExtractorTest.java
Show resolved
Hide resolved
...actor/src/test/java/org/schabi/newpipe/extractor/services/youtube/YoutubeSignaturesTest.java
Show resolved
Hide resolved
.../java/org/schabi/newpipe/extractor/services/youtube/YoutubeStreamLinkHandlerFactoryTest.java
Show resolved
Hide resolved
Oh I didn't notice that. Update: Nope, that's not the Top 50 anymore they seems to have completely reworked how charts work. There is also no more https://soundcloud.com/charts/top or https://soundcloud.com/charts/new just https://soundcloud.com/charts which then has links to various sets (aka playlists). https://api-v2.soundcloud.com/charts?genre=soundcloud:genres:all-music&client_id=MoLbAg35TuqjYwWVtNIKyRPFScQGMOBY&kind=top (used for |
Thanks for taking a closer look and sorry for causing confusion :) |
No problem :)
Rebased, should be gone now ^^ |
8f2c322 to
1be3bc7
Compare
There was a problem hiding this comment.
Thank you! The commits are a bit messy but I guess there was no way to organize them better, given the huge amount of changes. I reviewed everything now and it seems ok. If any of my comments do not apply to the latest code, please ignore them, I could only review commits instead of the whole code as the browser would start lagging 😅
extractor/src/test/java/org/schabi/newpipe/extractor/GeneralNewPipeTest.java
Outdated
Show resolved
Hide resolved
extractor/src/test/java/org/schabi/newpipe/downloader/DownloaderFactory.java
Outdated
Show resolved
Hide resolved
...r/src/test/java/org/schabi/newpipe/extractor/services/DefaultSimpleUntypedExtractorTest.java
Show resolved
Hide resolved
...rc/test/java/org/schabi/newpipe/extractor/services/bandcamp/BandcampSearchExtractorTest.java
Outdated
Show resolved
Hide resolved
...st/java/org/schabi/newpipe/extractor/services/media_ccc/MediaCCCRecentListExtractorTest.java
Outdated
Show resolved
Hide resolved
.../test/java/org/schabi/newpipe/extractor/services/peertube/PeertubeCommentsExtractorTest.java
Outdated
Show resolved
Hide resolved
...src/test/java/org/schabi/newpipe/extractor/services/youtube/YoutubeChannelExtractorTest.java
Outdated
Show resolved
Hide resolved
...actor/src/test/java/org/schabi/newpipe/extractor/services/youtube/YoutubeSignaturesTest.java
Outdated
Show resolved
Hide resolved
I'm not quite sure what happend there but I somehow got corrupted mocks, it didn't happen in subsequent runs so I think it might have been some sort of very weird race condition or YT just didn't like me xD
Belongs to #1335 ;) |
|
@Stypox A few tests are currently failing due to the rebase I will have a look at them and after that (when the CI is green) we can probably merge/SQUASH if there is nothing else Update: 2197 test passed, 108 ignored - We are good to go |
"showing results for ..." doesn't seem to be returned by the backend anymore, however the code is still present in the JS frontend.
From review: * Fix format * Use ``extractorUrl`` name * YoutubeStreamLinkHandlerFactoryTest setup with no downloader Co-Authored-By: Tobi <[email protected]>
They no longer exists and the API returns nothing for them. Overall SoundCloud seems to use a new system.
We are not C / C++ devs: * https://medium.com/@muqsithirfan/java-null-comparison-fcfbcb55ac67
Also removed irrelevant methods See TeamNewPipe#1308
Co-Authored-By: Stypox <[email protected]>
Co-Authored-By: Stypox <[email protected]>
Co-Authored-By: Stypox <[email protected]>
Co-Authored-By: Stypox <[email protected]>
Co-Authored-By: Stypox <[email protected]>
Co-Authored-By: Stypox <[email protected]>
These are currently not working, but the tests said OK because it was not checked what happens if the returned parameter is empty! -> TeamNewPipe#1339
|
There is currently a problem with n-Parameter deobfuscation, which is not working at all. |
| public void setUp() throws Exception { | ||
| super.setUp(); | ||
|
|
||
| extractor(); // Initialize |
There was a problem hiding this comment.
What I meant here is to put extractor() directly in super.setUp() so we don't have to ever worry about the extractor not being initialized, or would that create issues? There are various other places where I saw // Init comments
There was a problem hiding this comment.
What I meant here is to put extractor() directly in super.setUp() so we don't have to ever worry about the extractor not being initialized, or would that create issues?
Yes because there is no setUp method that creates an extractor when the class derives from InitYoutubeTest.
Also regarding "so we don't have to ever worry about the extractor not being initialized" there is a reason why this is lazily initialized:
We had a bunch of tests where there was a extractor used exactly in this way but it was never used.
When initializing the extractor during setup all tests mark it as used despite maybe never doing so.
There are various other places where I saw // Init comments
These are all places where this is used:

I now also refactored both classes to use dedicated methods which initialize the extractor.
|
This requires app changes / a migration when Top 50 kiosk is set as main page. Otherwise, you'll get this crash when starting the app: Stacktrace |
Fix all tests and make everything work offline/with mocks. Remove old mocks and generate new ones with new structure. Remove SoundCloud "Top 50" kiosk.
Fix all tests and make everything work offline/with mocks. Remove old mocks and generate new ones with new structure. Remove SoundCloud "Top 50" kiosk.
|
Note: SoundCloud's |
Based on/Required before merging:
List of fixes:
Made all tests mockable
Fixes #1276
-Ddownloader=MOCKand everything will workRECalias forRECORDINGdownloader (so that I have to type less)InitNewPipeTest(orInitYoutubeTestfor YT) so that the downloader is setup correctlyDefaultSimpleExtractorTestandDefaultSimpleUntypedExtractorTestregioncomments that are notregioncommentsYT
Never Gonna Give You Upvideo having a different title (YoutubeMixPlaylistExtractorTest)YoutubeMusicSearchExtractorTest- "showing results for ..." doesn't seem to be returned by the backend anymore, however the code is still present in the JS frontend.YoutubeMusicSearchExtractorto not do the same work multiple timesYoutubeStreamExtractorRelatedMixTestrandomly failing:Mix Videos can also display non-mix playlists in the related items:
Regenerated all YT mocksSoundcloud
Top 50list no longer exists (not present at https://soundcloud.com/charts) → RemovedSoundcloudChannelTabExtractorTest#Playlistsno longer exists → Replaced with another useronFetchPageand not always whengetInitialPageis calledAlso:
Fixes #1283