Skip to content
This repository has been archived by the owner on Feb 8, 2018. It is now read-only.

Unclog ttw tests #4391

Merged
merged 4 commits into from
Mar 30, 2017
Merged

Unclog ttw tests #4391

merged 4 commits into from
Mar 30, 2017

Conversation

chadwhitacre
Copy link
Contributor

@chadwhitacre chadwhitacre commented Mar 24, 2017

Fix #4359.

@chadwhitacre
Copy link
Contributor Author

❗️

$ phantomjs --version
1.9.8

@chadwhitacre
Copy link
Contributor Author

travis-ci/travis-ci#3225 💃

@chadwhitacre
Copy link
Contributor Author

chadwhitacre commented Mar 24, 2017

It looks like Travis is already using 2.1.1 by default on the dist: trusty instances. I'm having the intermittent PhantomJS crashes with 2.1.1 as well.

travis-ci/travis-ci#3225 (comment) (an hour ago :o)

@chadwhitacre
Copy link
Contributor Author

chadwhitacre commented Mar 24, 2017

Could also potentially switch to Firefox (post-#4322 :o).

@chadwhitacre
Copy link
Contributor Author

The failures w/ 2.0.0 I think I'm also seeing locally w/ 2.1.1. Looking into it ...

@chadwhitacre
Copy link
Contributor Author

Didn't I already post this somewhere?

Traceback (most recent call last):
  File "/Users/whit537/personal/gratipay/gratipay.com/tests/ttw/test_trim.py", line 14, in test_trim_strips_all_unicode
    assert self.trim('Úaø¶') == 'a'
  File "/Users/whit537/personal/gratipay/gratipay.com/tests/ttw/test_trim.py", line 10, in trim
    return self.js("window.Gratipay.trim('{}')".format(val))
  File "/Users/whit537/personal/gratipay/gratipay.com/gratipay/testing/browser.py", line 73, in js
    return self.evaluate_script(code)
  File "/Users/whit537/personal/gratipay/gratipay.com/env/lib/python2.7/site-packages/splinter/driver/webdriver/__init__.py", line 199, in evaluate_script
    return self.driver.execute_script("return %s" % script)
  File "/Users/whit537/personal/gratipay/gratipay.com/env/lib/python2.7/site-packages/selenium/webdriver/remote/webdriver.py", line 465, in execute_script
    'args': converted_args})['value']
  File "/Users/whit537/personal/gratipay/gratipay.com/env/lib/python2.7/site-packages/selenium/webdriver/remote/webdriver.py", line 236, in execute
    self.error_handler.check_response(response)
  File "/Users/whit537/personal/gratipay/gratipay.com/env/lib/python2.7/site-packages/selenium/webdriver/remote/errorhandler.py", line 192, in check_response
    raise exception_class(message, screen, stacktrace)
WebDriverException: Message: {"errorMessage":"Refused to evaluate a string as JavaScript because 'unsafe-eval' is not an allowed source of script in the following Content Security Policy directive: \"script-src 'self' assets.gratipay.com 'unsafe-inline'\".\n","request":{"headers":{"Accept":"application/json","Accept-Encoding":"identity","Connection":"close","Content-Length":"129","Content-Type":"application/json;charset=UTF-8","Host":"127.0.0.1:64827","User-Agent":"Python-urllib/2.7"},"httpVersion":"1.1","method":"POST","post":"{\"sessionId\": \"f9a82430-10c4-11e7-90b0-2db0b140720e\", \"args\": [], \"script\": \"return window.Gratipay.trim('\\u02daa\\u00f8\\u00b6')\"}","url":"/execute","urlParsed":{"anchor":"","query":"","file":"execute","directory":"/","path":"/execute","relative":"/execute","port":"","host":"","password":"","user":"","userInfo":"","authority":"","protocol":"","source":"/execute","queryKey":{},"chunks":["execute"]},"urlOriginal":"/session/f9a82430-10c4-11e7-90b0-2db0b140720e/execute"}}
Screenshot: available via screen

================================= 13 failed, 7 passed in 45.89 seconds ==================================
[gratipay] $

@chadwhitacre
Copy link
Contributor Author

I believe our ttw tests function by injecting scripts into the page, and that is running afoul of our CSP.

@chadwhitacre
Copy link
Contributor Author

Looks like 2.1.1 chokes on the CSP as 2.0.0 does, and then also on cross-domain cookies.

[gratipay] $ grep unsafe-eval log|wc -l
       4
[gratipay] $ grep 'Can only set Cookies for the current domain' log|wc -l
       9
[gratipay] $ 

@chadwhitacre
Copy link
Contributor Author

Best suggestion in ariya/phantomjs#11337 is to run through a proxy and strip the CSP header. ariya/phantomjs#13114 is longer ...

@chadwhitacre
Copy link
Contributor Author

I'm going to explore the Firefox option.

@chadwhitacre
Copy link
Contributor Author

(Best workaround to date in ariya/phantomjs#13114 is also to use a proxy.)

@chadwhitacre
Copy link
Contributor Author

Having pretty good luck with Firefox, though I'm having to upgrade Selenium to duck a "profile not found" error, and the latest Selenium requires this geckodriver to be installed—I'm pretty sure you need that and Firefox itself. Then I believe I'm seeing some timing bugs that I may have been seeing occasionally under Chrome anyway.

@chadwhitacre
Copy link
Contributor Author

Alright, pushed dd833f2. I'm expecting some timing failures in test_team_distribution.py.

@chadwhitacre
Copy link
Contributor Author

Will we need this xvfb hack?

@chadwhitacre
Copy link
Contributor Author

Getting an ancient version of Firefox (38). Addon?

@chadwhitacre
Copy link
Contributor Author

That's better. 👍

$ firefox --version
Mozilla Firefox 52.0.1

@chadwhitacre
Copy link
Contributor Author

@chadwhitacre
Copy link
Contributor Author

I'm expecting some timing failures in test_team_distribution.py.

https://travis-ci.org/gratipay/gratipay.com/builds/214804197 💃

@chadwhitacre
Copy link
Contributor Author

Can we use xvfb-run instead?

Yes!

travis-ci/docs-travis-ci-com#1060

@chadwhitacre
Copy link
Contributor Author

chadwhitacre commented Mar 24, 2017

I've got a plan to implement #4314 (comment), removing all time.sleep calls from the ttw tests. That should do away once and for all with the bugs cropping up here as well as #4118#4129#4290#4314#4362.

@chadwhitacre
Copy link
Contributor Author

chadwhitacre commented Mar 25, 2017

Squashed Travis flailing into 0e360f2, had been 5417051.

@chadwhitacre
Copy link
Contributor Author

With 27202ac I'm expecting Travis to be green! 💃

@chadwhitacre
Copy link
Contributor Author

Ready for review and merge! Note that this includes #4390.

This should squash #4362. Also:

- adapt to Firefox (`Keys.ENTER` vs `\n`)
- fix js memory leak in notifications (`delete $dialog`)
- only open one browser; reuse between classes
- git ignore geckodriver.log
@chadwhitacre
Copy link
Contributor Author

Rebased now that #4390 is in. Was b680cf4.

@chadwhitacre
Copy link
Contributor Author

Ready for review @rohitpaulk @nobodxbodon @JessaWitzel @kaguillera et al.

@@ -20,5 +20,8 @@ docs/gratipay.rst
_vimrc_local.vim
.transifexrc
npm-debug.log

# ttw tests
Copy link
Contributor

Choose a reason for hiding this comment

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

Whoopsy doopsy

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh it's a valid comment :D I thought it was a line used for testing :P

while time.time() < end_time:
if not self.has_element(selector):
return
raise NeverLeft()
Copy link
Contributor

@rohitpaulk rohitpaulk Mar 28, 2017

Choose a reason for hiding this comment

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

I think this could make it a bit difficult to debug failures - (What never left?). Maybe we could make it better by adding a message saying that The element specified by #{selector} did not disappear

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks in 3c1e47e like passing in the selector is sufficient to get it out again in a traceback.

@chadwhitacre
Copy link
Contributor Author

You good here, @rohitpaulk?

@rohitpaulk
Copy link
Contributor

Looks good to me!

@rohitpaulk rohitpaulk merged commit f9eb6df into master Mar 30, 2017
@chadwhitacre
Copy link
Contributor Author

!m @rohitpaulk 💃

@chadwhitacre chadwhitacre mentioned this pull request Apr 21, 2017
@chadwhitacre chadwhitacre deleted the unclog-ttw-tests branch June 15, 2017 19:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants