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

Convert JS tests to Selenium #3335

Closed
takluyver opened this issue Feb 13, 2018 · 65 comments
Closed

Convert JS tests to Selenium #3335

takluyver opened this issue Feb 13, 2018 · 65 comments

Comments

@takluyver
Copy link
Member

We have a number of Javascript tests:

We would like to convert these to use Selenium, in this folder. To run selenium tests, you will need the Selenium Python package and a 'driver' (a backend for it to talk to your browser). Once you have those, you can run:

py.test -v notebook/tests/selenium

This is quite a big task, and it probably won't be done in one pull request, but rather several smaller ones converting individual bits of the tests.

I'm marking this 'good first issue', though it's a bit borderline. It requires a decent understanding of Javascript and Python, but you don't need to know much about Jupyter to translate the tests. It should get easier as more of the tests are translated, because there will be more examples to refer to.

@parkerduckworth
Copy link

I would like to take this on. Getting started on it now

@parkerduckworth
Copy link

Hmm. It appears that the Selenium IDE is no longer being supported in Firefox:
https://addons.mozilla.org/en-US/firefox/addon/selenium-ide/

There is a chrome extension, but it is still under development and has a lot of issues. Have we considered Endtest? Are there any other testing frameworks we would consider using?

@takluyver
Copy link
Member Author

The Selenium IDE isn't important to us - I haven't used it at all. We're using Selenium Webdriver, and the Firefox backend is still supported.

@parkerduckworth
Copy link

Ah, ok! Can I be assigned this issue?

@varshneydevansh
Copy link

I want to contribute to this good first issue.
I am proficient in Python, C++ and Good at JS but need help to get started.

@takluyver
Copy link
Member Author

Github only lets us assign issues to people with push access to the repo, so just commenting that you plan to work on it is sufficient - especially since this is one a large task which multiple people can work on.

You can see the existing JS tests, for example this file. By comparison, here's the initial Selenium test. We want to add a Selenium (Python) version of each JS test, check that it passes, and then delete the JS test.

@takluyver
Copy link
Member Author

PhantomJS has suspended development, providing some extra impetus to make this switch.

@shubhsherl
Copy link

I see this is a good first issue.
Can you guide me how can I begin with this?

@sheshtawy
Copy link
Contributor

If no one is working in this issue I'd be happy to help

@takluyver
Copy link
Member Author

This is an issue that can be split up into small pieces, so multiple people can work on it at once. However, as I describe in the first message, it's not the easiest issue. You need to look at the Javascript tests (see the links in the first message), pick a test and understand what it's doing. Then look at the Selenium tests (also linked in the first message), and understand how they work. Then translate the Javascript test you chose to Selenium.

This should get easier as we convert more of the tests, because there will be more examples to look at. @mpacer and I are working on converting some of them, so if you're not sure what to do, I can hopefully point you to an example pull request soon.

@sheshtawy
Copy link
Contributor

From the example selenium test you posted in the description I got an initial good idea. I'm familiar with selenium and webdrivers and I'm willing to invest time understanding the phantomjs tests so I can start working on https://github.com/jupyter/notebook/tree/master/notebook/tests/notebook if that's okay. And once your pull request is ready I can use it as a guide to make sure my code is consistent with yours and also in case I got stuck or something.

But before I do that I have a question, you would want all tests from the three directories you linked above in the same tests/selenium directory? or you want them separated as they are now? so that the selenium directory is something like:

selenium/base/
selenium/notebook/

Let me know if it's okay to start tackling the notebook directory and what structure do you prefer inside the new tests/selenium directory and I can start working on immediately.

@takluyver
Copy link
Member Author

For now, let's keep the tests/selenium directory flat, placing files directly in there. We can easily rearrange it later if we need to divide them up, but I'm not convinced that the current way the JS tests are divided is that useful.

@sheshtawy
Copy link
Contributor

Okay great! I'll start tackling the test files in the tests/notebook then, okay?

@takluyver
Copy link
Member Author

Sounds good. @mpacer was looking at converting markdown.js from that folder, so if you want to start with another file, that would be good.

We also hit one or two fiddly bits with opening a new notebook and switching between edit and command mode. @mpacer was writing a couple of utility functions to do those things.

@mpacer
Copy link
Member

mpacer commented Mar 19, 2018

In order to support multiple tests in the same notebook I’m running into some issues with the utility functions as they are currently. I’m going to try to make something a bit more comprehensive for manipulating the notebook on the page. If you want to do any tests on the tree page though (not on notebooks themselves) I won’t be touching that outside of #3412 until I get these utilities working.

@danagilliann
Copy link
Contributor

@takluyver Hi! Do you mind giving a brief overview as to what the example tests do? To me they look like they're doing something quite different, and I'm a bit confused

@anand0427
Copy link

anand0427 commented Mar 20, 2018

I would like to contribute as well. Need some help getting started.

@takluyver
Copy link
Member Author

You mean the test that's already converted to Selenium?

The changes in #3412 make it easier to follow, so I'll describe this version of it

It gets authenticated_browser from a py.test fixture defined in conftest.py. That deals with starting a notebook server in a separate process, starting a Selenium driver, and navigating it to the files list of the notebook server. The Selenium driver is authenticated_browser inside the test code.

The first loop clicks down into directories in the list, until it reaches one with no subdirectories. At each level, it waits for something with the HTML class item_link to be visible on the page. Then it makes a list of such elements and stores it in a dictionary by the URL. If there's only one item, we assume that's .. (go up a level); otherwise we get the second item, click it, and assert that the new URL is what the link pointed to.

The second loop goes back up the levels, using browser history. Again, at each step it waits for something with class item_link, then builds a list of those items, and checks it against the list of items it found on the page on the way in. Then it calls authenticated_browser.back() to go back a step in history.

This may not seem like much of a test, but we recently made it navigate directories without reloading the page, using the history API in Javascript, and there were a few bugs in that.

@danagilliann
Copy link
Contributor

danagilliann commented Mar 20, 2018

Ah thank you for explaining. Just for clarification though, this Selenium test
and this JS test tests the same thing, right? If so which JS file/function is it testing?

@takluyver
Copy link
Member Author

Nope, those are different. I removed the equivalent Javascript test when I added the Selenium, but you can still see it in history: this Javascript test was the equivalent of this Selenium test in master.

@anand0427
Copy link

So basically write python code that is equivalent to the JS file that you already have? Which one can i start with?

@takluyver
Copy link
Member Author

Yup, that's about right. However, it's somewhat blocked at the moment because @mpacer is working on some utilities for the test framework to interact with the notebook UI, and there was only one test for the file list, which was already converted.

The tests in the base and security directories mostly don't seem to use the DOM, so these could be converted to use a regular JS test framework like Mocha, and be run in Node without needing to control a browser. That should make them faster, and if there are failures they will probably be easier to debug.

Alternatively, you could write new Selenium tests for parts of the UI other than the notebook editor - e.g. the 'running' tab in the dashboard, or authentication (visit .../logout, check that we can't get back to .../tree, try an invalid token, then the correct one).

@takluyver
Copy link
Member Author

I had a go at creating a mocha test loading base/js/utils. Running this in node wasn't entirely successful. It looks like too much of the code assumes it's in a browser to easily test in node. Mocha can run in the browser, which might be an option.

@sheshtawy
Copy link
Contributor

@mpacer I'm trying to convert deletecell.js. I'll try to make it self-included until the utilities module is ready and I can use it to update the tests. I'm sorry if I seem to overcommunicate stuff, I'm just to trying to keep you guys updated and kind of get feedback if I'm working in the right direction and following the right process. Let me know if need to do something else or even completely wait until a further notice from you.

@vallme2003
Copy link

Hi, i am interested in contributing. What can i work on?

@AmanPal-Singh
Copy link

I am also interested in contributing, what tasks are recommended to work on?

@takluyver
Copy link
Member Author

Hi all - so long as there are JS test files left (there are - see the links in the initial issue description), there are things to do. Briefly:

  • Pick a JS test
  • Write an equivalent test in Selenium
  • Check that the Selenium test runs and passes
  • Remove the JS test
  • Make a pull request

There are several which have been done and merged already which can serve as examples - see the "this was referenced" links in this issue.

I marked this as 'good first issue' because it doesn't require deep understanding of Jupyter, and it's fairly unlikely to clash with other work people might be doing. But it's not easy: you'll need to set up Selenium, understand the intent behind some Javascript code, and figure out how to check the same thing from Selenium. Maybe 'good second issue' would be more accurate, but we don't have a tag like that.

@dSchurch
Copy link
Contributor

Hello. I'm interested in convert some JS to selenium. Can I start working on the move_multiselection.js test ?

@jimakr
Copy link

jimakr commented Feb 9, 2020

Hello. i would like to contribute by converting Javascript tests to Selenium. I am going to start with safe_append_output.js

nfelger added a commit to nfelger/notebook that referenced this issue Apr 21, 2020
nfelger added a commit to nfelger/notebook that referenced this issue Apr 21, 2020
nfelger added a commit to nfelger/notebook that referenced this issue May 17, 2020
@nfelger
Copy link
Contributor

nfelger commented May 17, 2020

Hi, I've converted display_id.js (#5394) and notifications.js (#5455).

blink1073 pushed a commit that referenced this issue May 21, 2020
* Add selenium test helper to wait for JS expressions

* Convert notifications JS test to selenium (#3335)
@06fahad
Copy link

06fahad commented Dec 2, 2020

Hi, is there a particular reason why none of the JS tests inside the tests/base folder have been converted? If not, I'd like to start working on converting security.js to selenium.

@ikyman
Copy link

ikyman commented Jan 27, 2021

I believe I could do convert a few tests, Let me know if that's good with Y'all

@brian-ketelboeter
Copy link

I would also be willing to help out. I will convert a few also. Is there a way to check which still need to be converted?

@maxhw98
Copy link

maxhw98 commented Jul 12, 2021

Hi, I'd love to help out, I have a lot of experience with Python and Selenium, and some experience in JS. Is this issue still open? What can I do?

@kevin-bates
Copy link
Member

Although we're not entertaining new features at this time, I think contained test improvements would qualify as the kinds of changes we can still consider.

@maxhw98, @brian-ketelboeter, @ikyman, and @06fahad - thank you for your interest and I really apologize for the delay in getting back to you. As there have been a number of you recently interested in helping (thank you!), I would suggest some coordination. Please ping back via a comment when you'd like to take things up. If there are multiple, perhaps some coordination can take place (dividing by subsystem/service perhaps?) as this may be a large task.

Since this issue is front-end-oriented, I won't be of much help, but I'm sure we can find folks to answer any questions you might have. Thank you!

@lucastosetto
Copy link

Hello! I'd like to contribute with some test cases. Are there any specific ones you guys think it will be better to start with? Plese let me know.

@maxhw98
Copy link

maxhw98 commented Sep 7, 2021 via email

@kevin-bates
Copy link
Member

Are there any specific ones you guys think it will be better to start with? Plese let me know.

@lucastosetto - Thank you again for your interest (@maxhw98 as well). Unfortunately, I cannot direct you as I'm not a front-end dev/maintainer and am not familiar with any of the tests or their structure. From a practical standpoint, I would say that kernel and content services are two primary areas of focus. Since they are relatively disjoint (from the backend service perspective), they might prove to be a good separation point.

@jtpio
Copy link
Member

jtpio commented Oct 4, 2022

Closing as the main branch of the repo is now using jest and Playwright for testing.

@jtpio jtpio closed this as completed Oct 4, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests