Skip to content

Comments

Quick fix for failing test#210

Closed
mauriciocolli wants to merge 1 commit intoTeamNewPipe:devfrom
mauriciocolli:quick-fix-test
Closed

Quick fix for failing test#210
mauriciocolli wants to merge 1 commit intoTeamNewPipe:devfrom
mauriciocolli:quick-fix-test

Conversation

@mauriciocolli
Copy link
Contributor

Tests should be refactored as soon as possible. I mean look at their state, it's sad, really.

For example, from time to time (read often) we get rate limited and receive a 500 status from SoundCloud, we should implement retry and back off in the downloader used in tests. They are very brittle in the current state, and yes, we should expect them to be because we don't control the source, but we can at least improve the situation.

This is just a temporary quick fix.

@Stypox
Copy link
Member

Stypox commented Oct 23, 2019

I tried fixing tests in #180, but I needed some feedback to continue... can you provide some, please? ;-)

// Only test if subscribers count is available in the search page
if (subscriberCount != -1) {
assertTrue("Count does not fit: " + Long.toString(subscriberCount),
69043316 < subscriberCount && subscriberCount < 103043316);
Copy link
Member

Choose a reason for hiding this comment

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

This is going to start failing because PewDiePie will soon have more that 103,043,316 subscribers (currently he has 102M).

Using this we should be safe for the next 10 years: ;-)

Suggested change
69043316 < subscriberCount && subscriberCount < 103043316);
100000000 < subscriberCount && subscriberCount < 300000000);

@Stypox
Copy link
Member

Stypox commented Jan 20, 2020

This pr should be closed in favour of #234, where the entire YoutubeSearchCountTest class is removed

@Stypox Stypox closed this Jan 21, 2020
@mauriciocolli mauriciocolli deleted the quick-fix-test branch March 7, 2020 20:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants