giorgiosironi / phpunit-selenium

Selenium RC integration for PHPUnit
http://www.phpunit.de/
Other
600 stars 271 forks source link

Browser resurrection in shared mode #218

Open zerkms opened 11 years ago

zerkms commented 11 years ago

Now if you closeWindow() in the end of the test method (sometimes it's a test scenario, like in my case) then you'll see the next test method is failed with the exception

Error communicating with the remote browser. It may have died.

or

Session not found ....

(it's driver dependent)

How about resurrecting the browser in PHPUnit_Extensions_Selenium2TestCase_SessionStrategy_Shared::session() in case if it was closed?

I see the implementation to look like

public function session(array $parameters, $internal = false)
{
    if ($this->lastTestWasNotSuccessful) {
        if ($this->session !== NULL) {
            $this->session->stop();
            $this->session = NULL;
        }
        $this->lastTestWasNotSuccessful = FALSE;
    }
    if ($this->session === NULL) {
        $this->session = $this->original->session($parameters);
        $this->mainWindow = $this->session->windowHandle();
    } else {
        try {
            $this->session->window($this->mainWindow);
        } catch (Exception $e) {
            if ($internal) {
                throw $e;
            }

            if (strpos($e->getMessage(), 'Session not found') === false && strpos($e->getMessage(), 'It may have died') === false) {
                throw $e;
            }

            $this->session->stop();
            $this->session = NULL;
            return $this->session($parameters, true);
        }
    }
    return $this->session;
}

What do you think?

If you think it makes sense - I'll cover this solution with tests and send PR.

giorgiosironi commented 11 years ago

Should be nice for performance, but have you located the reason why the current policy (opening a new session) fails?

zerkms commented 11 years ago

@giorgiosironi,

yep - the current implementation invokes

$this->session->window($this->mainWindow);

for every test run (except of first), assuming there is a mainWindow. But as long as we closed it - there is no one, thus we get an exception.

giorgiosironi commented 11 years ago

I will merge a smarter solution than the current one. Take a loot at the existing tests too (if there are any at the unit or end-to-end level) too to see if they need to be updated.

zerkms commented 11 years ago

@giorgiosironi,

"Take a loot" --- do like!

giorgiosironi commented 11 years ago

Oops :) "look"

emaks commented 11 years ago

@zerkms I have added new changes in #195 , is this variant ok?

zerkms commented 11 years ago

@emaks, I will try to look later, don't have much time now, sorry