Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Testing with Selenium & Sauce labs #3321

Merged
merged 24 commits into from Feb 13, 2018
Merged

Testing with Selenium & Sauce labs #3321

merged 24 commits into from Feb 13, 2018

Conversation

@takluyver
Copy link
Member

@takluyver takluyver commented Feb 9, 2018

This replaces the js/tree section of the tests with Selenium tests run with py.test, and sets up Travis to run it using browsers on Sauce Labs, which has a free open source plan. The tests can also be run locally with a suitable webdriver installed (geckodriver for Firefox, chromedriver for Chrome).

My thinking is that this can form the basis for replacing more of the JS tests with Selenium tests. I think this is a nicer way of writing tests, it's more realistic because it drives a real browser rather than Qt webkit, and hopefully it won't suffer so much from random failures caused by async issues.

Sauce has some nice features - you can browse through screenshots at the different steps, to help diagnose problems:

screenshot from 2018-02-09 14-01-34

This probably won't work on the jupyter repo at the moment, but it's working on my fork. We'll need to create a Jupyter account on Saucelabs, and update the credentials in the file.

@takluyver
Copy link
Member Author

@takluyver takluyver commented Feb 9, 2018

Updating to the latest Firefox re-reveals an issue I was seeing locally but hoped was interference with my main Firefox instance: it randomly seems to go back to the authentication page at times, as if it had lost the authentication cookie. Any ideas?

This seemed to work when it was testing with an older version of Firefox on Linux, so we can push it back to an older version as a workaround if we can't find a better solution. Or maybe a new Firefox release will fix it.

Loading

@takluyver
Copy link
Member Author

@takluyver takluyver commented Feb 9, 2018

I've just realised that the tunnel to Sauce doesn't work for pull requests, to avoid exposing the secret Sauce token.

I'll change this to run a headless browser on Travis. We could set up a Travis cron job to test with a range of different browsers on a regular basis without affecting pull requests.

Loading

@mpacer
Copy link
Member

@mpacer mpacer commented Feb 9, 2018

This is really cool! Could you say a bit more about some of the issues that you’re running into & how you’re debugging them?

Loading

@takluyver
Copy link
Member Author

@takluyver takluyver commented Feb 9, 2018

Thanks! It largely feels like trial and error, but to break down some specific issues:

  • For some reason, passing --NotebookApp.base_url /a@b/ failed on Travis. After talking with Matthias, I tried on a whim passing it as --NotebookApp.base_url=/a@b/, which appears to work.
  • There were some SSL errors connecting to Sauce to drive the browser. After some frustration, I found a SO post saying that you have to use plain http rather than https.
  • Then I wasn't specifying a browser to use. Thankfully Sauce gave me a helpful error message, so that was easy.
  • Excluding a directory from nose proved to be surprisingly hard. Nose has options --exclude and --ignore-files, which are documented as taking a regex, but aren't very specific about what string the regex will be applied to. I finally decided that it's only possible with a plugin called nose-exclude.
  • Now Travis complains that it can't find geckodriver, so I'll copy and update some code to install it.

Loading

@gnestor
Copy link
Contributor

@gnestor gnestor commented Feb 12, 2018

This is epic, @takluyver!! Thank you!

Loading

visited.add(url)
browser.get(url)
wait_for_selector(browser, '.item_link')
assert browser.current_url == url
Copy link
Member Author

@takluyver takluyver Feb 12, 2018

Choose a reason for hiding this comment

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

I've tried to translate the JS test I removed directly to Python to illustrate how the machinery works, but I don't think it's actually a very meaningful test. The next step will be to write some new tests using this; I'm planning to leave that for other PRs, because I think there's already enough to look at in this PR.

Loading

@rgbkrk
Copy link
Member

@rgbkrk rgbkrk commented Feb 12, 2018

Thank you so much for going through this @takluyver, I'll help review.

Loading

@@ -40,6 +39,15 @@ before_install:
if [[ $GROUP == docs ]]; then
pip install -r docs/doc-requirements.txt
fi
- |
if [[ $GROUP == selenium ]]; then
pip install selenium
Copy link
Member

@rgbkrk rgbkrk Feb 12, 2018

Choose a reason for hiding this comment

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

That makes it nice n' easy, this is great.

Loading

rgbkrk
rgbkrk approved these changes Feb 12, 2018
Copy link
Member

@rgbkrk rgbkrk left a comment

This is great as an iteration towards replacing all the tests, so I'm strongly in favor.

Loading

@takluyver
Copy link
Member Author

@takluyver takluyver commented Feb 13, 2018

Is there someone else you'd like to look at this? Or shall we push the green button?

Loading

@gnestor gnestor merged commit aa9c977 into jupyter:master Feb 13, 2018
4 checks passed
Loading
@gnestor
Copy link
Contributor

@gnestor gnestor commented Feb 13, 2018

Pushed! Thanks a lot for taking the initiative on this, @takluyver. Now we should be able to upgrade to React 16 and add the vdom and possibly JSON renderers to notebook 👍

Loading

@takluyver
Copy link
Member Author

@takluyver takluyver commented Feb 13, 2018

Just to be clear, this puts the foundations in place for using Selenium, but we'll still need to convert the rest of the JS tests before the way is clear to using React.

Loading

@takluyver takluyver deleted the selenium branch Feb 13, 2018
@gnestor
Copy link
Contributor

@gnestor gnestor commented Feb 13, 2018

Understood. Replace "now" with "after we rewrite all the tests as Selenium tests" above 😋

Loading

@SimonBiggs
Copy link
Contributor

@SimonBiggs SimonBiggs commented Feb 20, 2018

@takluyver given how fresh this is I thought you might benefit from seeing how I set up protractor to run selenium tests for a notebook server based app:

https://github.com/SimonBiggs/scriptedforms/tree/master/scriptedforms/tests-e2e

Within the app I have a 'spinner' which defines when things are busy. Within the testing scripts I just wait for that 'spinner' to be gone to signify that the kernel has finished and the page has updated with the results from the kernel. It has worked quite well thus far.

Granted, my lil ol package is nothing compared to the mammoth of the Jupyter Notebook project, but I thought you might like to know how my experience was.

Cheers,
Simon

Loading

@SimonBiggs
Copy link
Contributor

@SimonBiggs SimonBiggs commented Feb 20, 2018

To make it work do the following:

Within scriptedforms/scriptedforms run:

yarn
yarn build

Then within scriptedforms run:

pip install -e .

Then within scriptedforms/scriptedforms/tests-e2e run:

yarn
yarn run install
yarn run selenium

Leave that previous terminal running and then within scriptedforms/scriptedforms/tests-e2e run:

yarn run e2e

Loading

@SimonBiggs
Copy link
Contributor

@SimonBiggs SimonBiggs commented Feb 20, 2018

Loading

@takluyver takluyver added this to the 5.5 milestone Apr 24, 2018
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants