add config.reopen_browser_on_fail

[yashaka]

(currently it's kind of "always true" with not ability to set it to false)

[SergeyPirogov]

What is the reason not to reopen browser on crash? Do we really need this feature?

[aleksandr-kotlyar]

@yashaka whats use case which you suggest for the option config.reopen_browser_on_fail: False? If browser will not reopen for next test after previous test fail - all tests will fail. In this context pytest already has a feature stop on fail-first or stop on N-fails: https://docs.pytest.org/en/stable/usage.html#stopping-after-the-first-or-n-failures

[yashaka]

probably this issue comes from the fact that Selenide has such feature: https://github.com/selenide/selenide/blob/7f0376d9a1616839429e4e1e3f045965014bc9cf/statics/src/main/java/com/codeborne/selenide/impl/StaticConfig.java#L40

It's a question why Selenide has it, and will selene users benefit from having it too...

for now I don't remember what I might think about it in the past:) We need to investigate:)

[aleksandr-kotlyar]

It's a question why Selenide has it, and will selene users benefit from having it too...

@yashaka I have collected information for you from Selenide. https://github.com/selenide/selenide/issues/244 https://github.com/selenide/selenide/issues/695

Seems that the idea was in not reopen shared browser inside one test if something goes wrong inside the test but open for the next tests. But i haven't found the full description of the idea about next tests in these issues. Just that "something" was implemented. We need to go deeper...

[yashaka]

mmmm... So the case might be like this:

from selene.support.shared import driver

def test_a():
    browser.open('...')  # open always inits driver if it's not alive and should be called before any other action
    
    browser.element('#one').click()  # GIVEN this click passed, but for some reason, after that the browser dies
    
    browser.element('#two').click()  # THEN here we should fail
    """
    i don't see any reason to spawn the browser again... 
    I guess that currently, selene has the behaviour described above... 
    selene will try to check that browser is alive before click, and if not - it should ask to call open again...
    and fail with exception
    """

def test_b()
    browser.open('...')  # this open yet should init driver as usual
    
    # ...

So, I'm not totally sure how now selene works, we have to check, provide the above kind of test, and ensure the correct implementation. Probably just this, I mean, we don't have to add one more option to config, just ensure the correct behavior from selene.

Yet, we can miss some cases where such config.reopen_browser_on_fail would make sense. Let's raise this topic in selenide's chat (like this one: https://t.me/selenide_ru) and ask about all reasons and cases where this config option might be relevant...

[aleksandr-kotlyar]

So, I'm not totally sure how now selene works, we have to check, provide the above kind of test, and ensure the correct implementation. Probably just this, I mean, we don't have to add one more option to config, just ensure the correct behavior from selene.

Ok, i will check it.

Yet, we can miss some cases where such config.reopen_browser_on_fail would make sense. Let's raise this topic in selenide's chat (like this one: https://t.me/selenide_ru) and ask about all reasons and cases where this config option might be relevant...

I think it would be better to ask in selenide's github for history and for community.

[aleksandr-kotlyar]

Tested on selene 2.0.0a35+ current master https://github.com/yashaka/selene/tree/0e7ed43957d08ff37f37c731e8fc9136500b85c4 Executed the whole module with two tests:

from selene import be, by
from selene.support.shared import browser


def test_shared_browser_not_reopen_on_find_actions():
    browser.open('http://google.ru')
    browser.element(by.name('q')).should(be.visible)

    browser.quit()

    browser.element(by.name('q')).should(be.visible)  # <--- fails here. See stacktrace below


def test_reopens_after_previous_failed_of_unexpectedly_closed_webdriver():  # <--- starts normally after shared_1 has been failed
    browser.open('http://google.ru')
    browser.element(by.name('q')).should(be.visible)

test_reopen_shared_1 fails with

    @property
    def instance(self) -> WebDriver:
        if self._closed:
            raise RuntimeError(
>               'Webdriver has been closed. '
                'You need to call open(url) '
                'to open a browser again.'
            )
E           RuntimeError: Webdriver has been closed. You need to call open(url) to open a browser again.

test_shared_browser_not_reopen_on_find_actions fails cause of "Webdriver has been closed" but test_reopens_after_previous_failed_of_unexpectedly_closed_webdriver starts new webdriver normally. Seems it has already work as expected and it doesn't need implementation as a separate feature.

[aleksandr-kotlyar]

Added more tests in PR #274