manu-chroma / username-availability-checker

Live at : https://username-check.herokuapp.com/
95 stars 37 forks source link

Medium test fails because of difference between avatar url #79

Closed seeeturtle closed 6 years ago

seeeturtle commented 6 years ago

https://travis-ci.org/manu-chroma/username-availability-checker/builds/347315141?utm_source=github_status&utm_medium=notification

https://travis-ci.org/manu-chroma/username-availability-checker/builds/347339603?utm_source=github_status&utm_medium=notification

From this two builds above, we can know that both fails because of medium's avatar url difference. It's curious that one fails and other doesn't.

p.s. There is occasional failure in first build.

seeeturtle commented 6 years ago

It's really curious that I can't find miro.medium.... in the page. And why doesn't @MediumStaff fail?

manu-chroma commented 6 years ago

I think I will be able to find time to debug this one tomorrow

Meanwhile, why don't you try inspect the source you receive from res.get rather than from the browser source.

seeeturtle commented 6 years ago

I'll try it.

seeeturtle commented 6 years ago

This is tested with python 3.6.3

>>> import username_api
>>> app = username_api.app.test_client()
>>> res = app.get("/check/medium/Medium")
>>> for x in res.response: print(x)
...
b'{\n  "avatar": "https://cdn-images-1.medium.com/max/1200/1*6_fgYnisCa9V21mymySIvA.png",\n  "possible": true,\n  "status": 200,\n  "url": "https://medium.com/@Medium",\n  "usable": true\n}'

It seems it also returns "cdn..." not "miro...".

@manu-chroma

manu-chroma commented 6 years ago

interesting. I'll also check

manu-chroma commented 6 years ago

@This issue seems to plague travis only.

Here's a build log from codeship: https://app.codeship.com/projects/274884/builds/32516987 and it passes successfully. Locally also, the test passes

On manual testing also, I got "cdn.." avatar link in case of both expected and actual response. Maybe we can just remove MediumStaff from the test list because that is the only username that fails. All other medium tests work as expected.

manu-chroma commented 6 years ago

In latest master branch travis rebuild: https://travis-ci.org/manu-chroma/username-availability-checker/builds/343095785, it return None for avatar..

seeeturtle commented 6 years ago

Maybe we can just remove MediumStaff from the test list because that is the only username that fails. All other medium tests work as expected.

Yeah, I think that is the only solution we can do for now. I'll modify it now.

@manu-chroma

seeeturtle commented 6 years ago

@manu-chroma, Can you test at your local before merging the PR? It's also None in my local test. It's not a problem only for Travis

seeeturtle commented 6 years ago

Hmm.... It's "very" irregular :( Sometime it passes in my local test and sometime "Medium" fails and sometimes "MediumStaff" fails... It sometime passes at CI and sometime doesn't pass...

What is the problem? @manu-chroma

seeeturtle commented 6 years ago
>>> count = 0
>>> for _ in range(100):
...     res = username_api.get_avatar('medium', 'Medium')
...     if res is None:
...             count += 1
...
>>> count/100
0.14
>>> count
14

Test from python 3.6.3

It seems 14 out of 100 are None. I think this is the reason why the result of CI is irregular

manu-chroma commented 6 years ago

is it just for Medium and MediumStaff username or any random username on Medium website?

seeeturtle commented 6 years ago

You can see that it's only for Medium. I'll try it for MediumStaff too. but it'll take some time

manu-chroma commented 6 years ago

sure! thanks

seeeturtle commented 6 years ago
>>> count = 0
>>> for _ in range(100):
...     res = username_api.get_avatar('medium', 'MediumStaff')
...     if res is None:
...             count += 1
...
>>> count/100
0.07
>>> count
7

Test for MediumStaff