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

Add new SelenideDriver(new FirefoxDriver()).find("#element") syntax in addition to $("#element") #354

Closed
yashaka opened this Issue Jun 28, 2016 · 15 comments

Comments

Projects
None yet
6 participants
@yashaka
Copy link
Contributor

yashaka commented Jun 28, 2016

Usually the "automatic driver management" is great.

But sometimes, when you need to operate with two opened browsers in one test - there is just no easy option in Selenide to do this.

Usecase:
Social network. Unfortunately there is no access to backend so you have to automate it as "full black box". You have to test "Joe can see a post created by his friend Bob".
In order to "save time" for "login/logout" the developer decides to open two browsers and login in each with different user - Joe and Bob correspondingly - so then once Bob created a post Joe can "immediately" check the result without "logout/login".

In Selenide there is the only option to implement this:

  • remember two drivers in test itself and switch drivers via WebDriverRunner#setDriver correspondingly. Even if we have pageobjects, we still have no way to say to pageobject to which driver should it belong to. This makes a code for such test overcomplicated and hardly supportable.

In one project we had to create our own "spikes" over Selenide WebDriverRunner implementation using reflection to get access to private fields in order to hide "driver substitution" and reduce boilerplate in tests...

It's sad:)

I recommend to refactor implementation of Selenide's driver management to be of object oriented style, where we have an object of Selenide wrapper over Selenium driver,
so we can create SelenideElement via:

SelenideDriver driver = new SelenideDriver(new FirefoxDriver());
driver.element("#element").click();

or some other "naming" style can be chosen:

SelenideDriver I = new SelenideDriver(new FirefoxDriver());
I.find("#element").click();

But the first style seems to me more "object oriented" and of "less magic" (find does not find, it creates a a lazy proxy element object which can find itself by request). We still can keep both aliases find and element, and even $.

Then we can build "procedural/static" helpers like $, $$ on the base of the latter object-oriented implementation.

This will add a big plus to popularity of Selenide. Because many engineers are afraid of using Selenide because of its "the only procedural" nature (in context of "managing driver and creating elements").

@asolntsev

This comment has been minimized.

Copy link
Contributor

asolntsev commented Jun 28, 2016

I still believe that using 2 browser windows in the same test is bad idea.
Never do it!

But probably we will implement this feature request. Just because many people ask for this.

@jtolotta

This comment has been minimized.

Copy link

jtolotta commented Jun 29, 2016

@yashaka

Because many engineers are afraid of using Selenide because of its "the only procedural" nature (in context of "managing driver and creating elements").

I don't understand what you trying to say about Selenide.
Selenide is just a wrapper around Selenium and it gives you access to all of Selenium's core components.

Any limitation Selenide has Selenium has as well.

If you need to use two browser windows, then just like Selenium, you will need to create two WebDriver instances.

If you need to use Selenide, you will need to assign each WebDriver instance using the setWebDriver() static method in the WebDriverRunner class when you need to use that browser to perform the test.

If you need to use the same Page Objects with multiple browsers, then the context must be set for each before instantiating it.

Below is a JUnit example using a single Page Object between two different WebDriver instances.
It also opens a Google page to prove the context switch.

NOTE 1: Reference the setUp(), set each WebDriver instance in the WebDriverRunner before creating the page objects.

NOTE 2: By doing this you will need to close the WebDriver instances manually just like Selenium. This also assumes Firefox browser is used and no proxy settings.

import static com.codeborne.selenide.Selenide.page;

import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
import org.openqa.selenium.WebDriver;
import org.openqa.selenium.firefox.FirefoxDriver;
import org.openqa.selenium.support.FindBy;

import com.codeborne.selenide.Selenide;
import com.codeborne.selenide.SelenideElement;
import com.codeborne.selenide.WebDriverRunner;

public class AppTest {
    private WebDriver w1;
    private WebDriver w2;
    private WebDriver w3;
    private ExamplePage ex1;
    private ExamplePage ex2;

    @Before
    public void setUp() {
        w1 = new FirefoxDriver();
        w2 = new FirefoxDriver();
        w3 = new FirefoxDriver();

        WebDriverRunner.setWebDriver(w1);
        ex1 = page(ExamplePage.class);

        WebDriverRunner.setWebDriver(w2);
        ex2 = page(ExamplePage.class);
    }

    @Test
    public void test() {
        WebDriverRunner.setWebDriver(w1);
        Selenide.open("http://www.example.com");
        Assert.assertTrue(Selenide.title().equals("Example Domain"));

        WebDriverRunner.setWebDriver(w3);
        Selenide.open("http://www.google.com");
        Assert.assertTrue(Selenide.title().equals("Google"));

        WebDriverRunner.setWebDriver(w2);
        Selenide.open("http://www.example.com");
        Assert.assertTrue(Selenide.title().equals("Example Domain"));
        ex2.moreInfoLink.click();
        Assert.assertTrue(Selenide.title().equals("IANA — IANA-managed Reserved Domains"));

        WebDriverRunner.setWebDriver(w1);
        ex1.moreInfoLink.click();
        Assert.assertTrue(Selenide.title().equals("IANA — IANA-managed Reserved Domains"));
    }

    @After
    public void tearDown() {
        w1.close();
        w2.close();
        w3.close();
    }

    static class ExamplePage {
        @FindBy(tagName = "a")
        public SelenideElement moreInfoLink;
    }
}

It also seems the use case you desire wants an almost undetermined number of clients to be generated.

Using reflection is probably the best way to spawn WebDriver instances and Page Objects.

I agree with @asolntsev with the whole messiness of dealing with multiple browsers.

You may need to look to performance testing software like Gatling
http://gatling.io/

@yashaka

This comment has been minimized.

Copy link
Contributor

yashaka commented Jun 29, 2016

@JasonTolotta
Yeah, you really did not understand my post. Seems like either because did not read it with enough attention or I explained it "too complicated".

Everything you wrote, I also mentioned in my post.

And your example just reflects my note on the subject:

remember two drivers in test itself and switch drivers via WebDriverRunner#setDriver correspondingly. Even if we have pageobjects, we still have no way to say to pageobject to which driver should it belong to. This makes a code for such test overcomplicated and hardly supportable.

Pay attention to this part:

This makes a code for such test overcomplicated and hardly supportable.

As I explained, your example is a BADly implemented test. Good test implementation should contain test logic, not TECHNICAL DETAILS like "switching drivers". Such test is too complicated and needs more time for support. It's also prone to errors when something changes in test logic. There are also a plenty of arguments in context of "good design", but this is just a test, not a "complicated software system" so I will leave them out of the board:)

Taking this into account, I can't agree with:

Any limitation Selenide has Selenium has as well.

Because Selenium is free of limitation - "overload your test with bulky technical details in context of driver management".

Saying all this, I still completely agree with @asolntsev that

I still believe that using 2 browser windows in the same test is bad idea.
Never do it!

But sometimes, on some projects, you just have no other choice (except of "retire from work"), because of many things.
Sometimes you have no access to backend.
Sometimes management is foolish and you are too young to convince management in what you believe.

And in such cases, you have to use selenium (some own selenium based framework) over selenide, if you still want to write "concise and easy to read and support" tests ... And it's a sad...

I believe that if we make Selenide "more friendly" in this context, we can make it more popular, and so have more power to convince people that yeah,

I still believe that using 2 browser windows in the same test is bad idea.
Never do it!

@yashaka

This comment has been minimized.

Copy link
Contributor

yashaka commented Jun 29, 2016

And, by the way, there is one more use case for this feature.

It is described here with examples in C#: https://github.com/yashaka/NSelene/tree/master/NSeleneExamples/TodoMVC/IntegratedToSeleniumBasedFramework

The idea is the following.
We have a project with many automated tests, which are built using "own selenium based framework with pageobjects + pagefactory style".

And let's assume that this selenium based framework is not good enough, some steps are not stable (because of e.g. Ajax problems), and engineers would like to switch to Selenide. But they have no time to "rewrite all old tests and pageobjects".
They just want to
1 write "everything new using Selenide"
2 fix unstable parts by using Selenide over own framework.

And now, with current Selenide implementation the only option they have is:

  • to write tests for new web pages using Selenide
    to achieve goal 1

But to achieve the goal 2 (fix unstable parts by using Selenide over own framework) they have to rewrite pageobject fully what is not a good choice taking into account our precondition.

But if we add to Selenide a correspondent webdriver decorator, everything will become much smoother.

Want to cover new webpage? - no problem, just add a pageobject in a normal "selenide way" with automatic driver management

Want to fix some unstable step which uses driver.findElement(locator)? - no problem, just change it to new SelenideDriver(driver).find(locator)

Have problems with some @FindBy(css="#its-locator") webelement? - no problem, just decorate it with new SelenideDriver(driver).element(webelement)
or, if webelement is used in more than one step, for DRY - just delete annotation, and add webelement = browser.element("#its-locator") to the page constructor, or implement it not as a field, but as a method with the return driver.find("#its-locator") code
(here browser = new SeleneDriver(driver);

Easy;)

In this way, we will not only give Selenide as a "better tool for your next automation project", we also will sell it as a "better tool for your current automation project".

I have trained a lot of my students to work with selenide (in addition to selenium). And many of them find new projects and face the "our own selenium based frameworks" and have to live in "hell" after come back from "paradise"... They have to support "legacy bulky not stable code" in the same "bulky" way. I believe that the proposed feature can really make their life much easier...

@yashaka

This comment has been minimized.

Copy link
Contributor

yashaka commented Jun 29, 2016

One more use case for "bad test with two drivers":)

A backend of some project will be completely refactored, completely:)

So we may need some "acceptance tests" that should be fully of "black box" style, with no any dependence to backend. Such tests will be slow... And (recall example with social network from my first post) it will really save some execution time to optimise them via dealing with several opened browsers...

@yashaka

This comment has been minimized.

Copy link
Contributor

yashaka commented Jun 30, 2016

Code examples for original post:

Social network. Unfortunately there is no access to backend so you have to automate it as "full black box". You have to test "Joe can see a post created by his friend Bob".
In order to "save time" for "login/logout" the developer decides to open two browsers and login in each with different user - Joe and Bob correspondingly - so then once Bob created a post Joe can "immediately" check the result without "logout/login".

// somewhere in init
joeDriver = new FirefoxDriver();
bobDriver = new FirefoxDriver();
joePage = new Page(joeDriver);
bobPage = new Page(bobDriver);

joePage.open()
joePage.login("joe", "qwerty");

bobPage.open()
joePage.login("bob", "123456");

...

//in test "create post"
bobPage.createPost("red fox jumps over lazy dog");
joePage.stream.posts.first().shouldHave(text("red fox jumps over lazy dog"));

vs

// somewhere in init
joeDriver = new FirefoxDriver();
bobDriver = new FirefoxDriver();
joePage = new Page(joeDriver);
bobPage = new Page(bobDriver);

setDriver(joeDriver);
joePage.open();
joePage.login("joe", "qwerty");

setDriver(bobDriver);
bobPage.open();
joePage.login("bob", "123456");

...

//in test "create post"
setDriver(bobDriver);
bobPage.createPost("red fox jumps over lazy dog");

setDriver(joeDriver);
joePage.stream.posts.first().shouldHave(text("red fox jumps over lazy dog"));
@yashaka

This comment has been minimized.

Copy link
Contributor

yashaka commented Jun 30, 2016

@JasonTolotta
First of all, this

ex1 = new MyPage<ExamplePage>(w1, ExamplePage.class);

seems to me as "boilerplate" code. Solution is far away from being "elegant" and "concise".

Second, your wrapper can "implicitly" switch drivers only during navigation.

This means that the test will still contain the same "technical details" - this time - navigate() calls instead of setDriver(...) - for each "page context" switch.

The one idea is to provide a solution that can remove "technical details" from "test logic" for tests working with "Several browsers" and built with "pageobject steps".

The second idea is to provide solution that can integrate selenide into existing selenium based framework taking into account that the majority of such frameworks uses "driver as a state of PageObject".

@jtolotta

This comment has been minimized.

Copy link

jtolotta commented Jun 30, 2016

@yashaka

seems to me as "boilerplate" code. Solution is far away from being "elegant" and "concise".

Yeah, stupid me. I should have seen that. LoL.

@yashaka yashaka changed the title Add new Browser(new FirefoxDriver()).element("#element") syntax in addition to $("#element") Add new SelenideDriver(new FirefoxDriver()).find("#element") syntax in addition to $("#element") Aug 5, 2016

@asolntsev

This comment has been minimized.

Copy link
Contributor

asolntsev commented Aug 5, 2016

For me, definitely one practical usage for this feature is possibility to use 2 browsers simultaneously. (It was not possible in Selenide before). I think that it's totally bad idea to use 2 browsers in test, but people ask for it too often. So, let's do it.

@yashaka

This comment has been minimized.

Copy link
Contributor

yashaka commented Aug 5, 2016

Along with this feature, we have to also make an "object-oriented" alternative to Configuration class.

There is one more thing to do about Configuration - make Configuration properties - static ones - like Configuration.timeout - ThreadLocal - the same as internal "automatic" driver in Selenide. Because currently you can have "driver per thread" in Selenide, but can't have "configuration per thread". But this seems to be filed as separate issue, like "make Configuration properties - ThreadLocal to allow custom configuration per thread in parallel testing"

@yashaka

This comment has been minimized.

Copy link
Contributor

yashaka commented Sep 13, 2016

Unless we have proper implementation in Selenide, here is the simplest implementation which anybody can already use if needed: https://gist.github.com/yashaka/dc7607239518bd37298ef5eb5b08da9b

@Kiranannam

This comment has been minimized.

Copy link

Kiranannam commented Aug 15, 2017

Hi,
Any update on this implementation?

@asolntsev

This comment has been minimized.

Copy link
Contributor

asolntsev commented Aug 15, 2017

@Kiranannam

This comment has been minimized.

Copy link

Kiranannam commented Aug 16, 2017

@asolntsev, Thanks for your reply. Will wait for your update.

@asolntsev

This comment has been minimized.

Copy link
Contributor

asolntsev commented Oct 26, 2017

@yashaka @jtolotta @Kiranannam FYI
I started working on this issue in branch use-dependency-injection. It's not a final solution, I am just experimenting.

@BorisOsipov BorisOsipov added feature and removed help wanted labels May 15, 2018

@asolntsev asolntsev self-assigned this Aug 15, 2018

asolntsev added a commit that referenced this issue Aug 29, 2018

asolntsev added a commit that referenced this issue Aug 31, 2018

asolntsev added a commit that referenced this issue Aug 31, 2018

asolntsev added a commit that referenced this issue Sep 4, 2018

#354 move part of code from WebDriverContainer to SelenideDriver
* SelenideDriver doesn't use Threads. It only contains WebDriver and SelenideProxy.
* WebDriverContainer holds instances of SelenideDriver in ThreadLocal.

asolntsev added a commit that referenced this issue Sep 22, 2018

#354 move part of code from WebDriverContainer to SelenideDriver
* SelenideDriver doesn't use Threads. It only contains WebDriver and SelenideProxy.
* WebDriverContainer holds instances of SelenideDriver in ThreadLocal.

asolntsev added a commit that referenced this issue Sep 22, 2018

#354 remove dependency WebDriverBinaryManager -> WebDriverRunner
* also move part of code from WebDriverFactory to a smaller class BrowserResizer (for lesser Fan-Out complexity)
* also inline method Describe.describe(WebDriver) (for lesser Fan-Out complexity)

asolntsev added a commit that referenced this issue Sep 22, 2018

#354 move browser-related constants from WebDriverRunner to Browsers
to allow removing all dependencies on WebDriverRunner static methods from Selenide code

asolntsev added a commit that referenced this issue Sep 22, 2018

#354 avoid using static methods WebDriverRunner.* and Selenide.* insi…
…de Selenide code

* pass Context(WebDriver, SelenideProxyServer) as a parameter everywhere
* move "static" classes like WebDriverRunner and Selenide to a separate module to guarantee that they are not used inside Selenide code

asolntsev added a commit that referenced this issue Sep 22, 2018

#354 make SelenideDriver implement Context
...to make opening a browser a "lazy" operation (see LazyEvaluationTest)

asolntsev added a commit that referenced this issue Sep 22, 2018

asolntsev added a commit that referenced this issue Sep 22, 2018

#354 move driver+proxy logic from SelenideDriver to a smaller class D…
…river

also use Driver class instead of Context (this is essentially the same)

asolntsev added a commit that referenced this issue Sep 22, 2018

asolntsev added a commit that referenced this issue Sep 22, 2018

asolntsev added a commit that referenced this issue Sep 22, 2018

#354 simplify code: merge BrowserConfig and Config
it seems that having 2 separate classes only makes things harder

asolntsev added a commit that referenced this issue Sep 22, 2018

#354 move Configuration class too `statics` folder
... to guarantee that it's not used inside Selenide code

asolntsev added a commit that referenced this issue Sep 22, 2018

asolntsev added a commit that referenced this issue Sep 25, 2018

#354 convert (part of) integration tests to non-static form
... and move others to `statics` module

asolntsev added a commit that referenced this issue Sep 25, 2018

#354 convert (part of) integration tests to non-static form
... and move others to `statics` module

@rosolko rosolko closed this Sep 30, 2018

@asolntsev asolntsev added this to the 5.0.0 milestone Oct 1, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment