Fix #2437 - Upgrades python ua-parser version to 0.8 #2444
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Travis tests have failedHey @karlcow, 1st Buildnpm run test:js -- --reporters="runner" --firefoxBinary=`which firefox` |
|
@karlcow can you investigate the chrome test failure? That's an instance where we do use UA sniffing to know which addon link to give. |
miketaylr
reviewed
May 9, 2018
|
ok let's see manually at least. This is working.
And I'm not sure why it would fail, because this is done on the JS side there's no request to the UA parsing in python. The test took 10+ s. I would bet on unresponsive page. The |
|
@karlcow do you see a "retrigger build" button from https://travis-ci.org/webcompat/webcompat.com/builds/376706871?utm_source=github_status&utm_medium=notification when you log in? Normally it's there for me... but I think since I don't have write access to your fork, only can do it. Might be good to see if it's just being slow like you suggest. |
|
In the process but I tried locally and it fails too at the same place with a long delay too.
What is strange is the test before is the front page load and it is blazing fast. :) |
|
Same consistent failure. So weird. |
|
ah it is used here webcompat.com/webcompat/views.py Lines 150 to 151 in 7661d49 |
|
Locally and manually, it displays: chrome using chrome Canary. Let's test a couple of things. |
|
Ah wait one of the changes, that 0.8 brought is We have a couple of http://python-future.org/compatible_idioms.html#basestring but that should fail for other tests then. What else: webcompat.com/webcompat/templates/nav/addon-links.html Lines 35 to 45 in 7661d49 The test is always expected to work whichever browser we used. And there's no test to test that the addon link is not there for an unknown browser. I think that's the issue. |
|
Ah! uap-core added headlesschrome parsing one year ago on Mar 15, 2017 https://github.com/ua-parser/uap-python/releases
So we have the combination of a weak functional test and a better UA detection. ok now I need to fix the functional test :) |
:) We could just search for "chrome" in the string, rather than expect an exact match... (in the template, that is). |
|
@miketaylr I think these addons link test could be on the unit testing side, as they are not dependent of the JS interaction at all but about the content. |
karlcow
added a commit
to karlcow/webcompat.com
that referenced
this pull request
May 9, 2018
This does two things: - Adds unittest tests to check that the right partial template for addons is sent depending on the user agent string. - Adds a test for an unknown browser - Fixes the template for addon where some white spaces where left over here and there. - Removes the previous weaker functional tests which was very dependent on the testing browser. It was triggered by the upgrade of ua-parser in python which took into account the headlesschrome user agent string, then not detected by our template. It was making the functional test fail. See webcompat#2444 for the background.
|
ok let's see what Travis think about |
karlcow
added a commit
to karlcow/webcompat.com
that referenced
this pull request
May 10, 2018
This does two things: - Adds unittest tests to check that the right partial template for addons is sent depending on the user agent string. - Adds a test for an unknown browser - Fixes the template for addon where some white spaces where left over here and there. - Removes the previous weaker functional tests which was very dependent on the testing browser. It was triggered by the upgrade of ua-parser in python which took into account the headlesschrome user agent string, then not detected by our template. It was making the functional test fail. See webcompat#2444 for the background.
|
oopsie some syntax issues. commit, force update, repeat and rinse. |
|
@miketaylr Yoohoo all checks have passed. Pfew… |
miketaylr
approved these changes
May 10, 2018
| @@ -26,16 +26,6 @@ registerSuite("Index", { | |||
| .end(); | |||
| }, | |||
|
|
|||
| "reporter addon link is shown"() { | |||
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
tests/unit/test_rendering.py
Outdated
| CHROME_UA = 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_4) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/68.0.3425.0 Safari/537.36' # noqa | ||
| FIREFOX_UA = 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:61.0) Gecko/20100101 Firefox/61.0' # noqa | ||
| FIREFOX_AND_UA = 'Mozilla/5.0 (Android; Mobile; rv:61.0) Gecko/61.0 Firefox/61.0' # noqa | ||
| SAFARI_UA = 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_4) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/11.2 Safari/605.1.15' # noqa |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
|
Feel free to merge if you want to remove that safari string (or just merge and leave it in, not a huge issue inside test code). |
This does two things: - Adds unittest tests to check that the right partial template for addons is sent depending on the user agent string. - Adds a test for an unknown browser - Fixes the template for addon where some white spaces where left over here and there. - Removes the previous weaker functional tests which was very dependent on the testing browser. It was triggered by the upgrade of ua-parser in python which took into account the headlesschrome user agent string, then not detected by our template. It was making the functional test fail. See webcompat#2444 for the background.
|
wooot… |
|
Let's kick it again again… 😭 |
|
pfew. all green. 💚 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
r? @miketaylr
see comments in #2437 for details.