Skip to content

Commit

Permalink
Fix page navigation that does not support loader to be replaced
Browse files Browse the repository at this point in the history
  • Loading branch information
gsouf committed Oct 13, 2018
1 parent 7ce409b commit d19ec0c
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 13 deletions.
4 changes: 2 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
* Bug fixes:
* none

## 0.2.5
## 0.3.0

> *20xx-xx-xx* (not released)
Expand All @@ -23,7 +23,7 @@
* Removed unused option ``debug``
* Added ``BrowserFactory::connectToBrowser``
* Bug fixes:
* none
* (BC Break) Page navigation now allows by default that the initial loader is replaced with a new one #40

## 0.2.4

Expand Down
10 changes: 7 additions & 3 deletions src/Page.php
Original file line number Diff line number Diff line change
Expand Up @@ -89,14 +89,18 @@ public function getSession(): Session
}

/**
* @param $url
* @param string $url
* @param array $options
* - strict: make waitForNAvigation to fail if a new navigation is initiated. Default: false
*
* @return PageNavigation
* @throws Exception\CommunicationException
*/
public function navigate($url)
public function navigate(string $url, array $options = [])
{
$this->assertNotClosed();

return new PageNavigation($this, $url);
return new PageNavigation($this, $url, $options['strict'] ?? false);
}

/**
Expand Down
35 changes: 27 additions & 8 deletions src/PageUtils/PageNavigation.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,16 +50,24 @@ class PageNavigation
*/
protected $page;

/**
* @var bool
*/
protected $strict;

/**
* PageNavigation constructor.
* @param Page $page
* @param string $url
* @param bool $strict by default this method will wait for the page to load even if a new navigation occurs
* (ie: a new loader replaced the initial navigation). Passing $string to true will make the navigation to fail
* if a new loader is generated
*
* @throws Exception\CommunicationException
* @throws Exception\CommunicationException\CannotReadResponse
* @throws Exception\CommunicationException\InvalidResponse
* @throws Exception\NoResponseAvailable
*/
public function __construct(Page $page, string $url)
public function __construct(Page $page, string $url, bool $strict = false)
{

// make sure latest loaderId was pulled
Expand All @@ -76,6 +84,7 @@ public function __construct(Page $page, string $url)
$this->page = $page;
$this->frame = $page->getFrameManager()->getMainFrame();
$this->url = $url;
$this->strict = $strict;
}

/**
Expand All @@ -96,7 +105,7 @@ public function __construct(Page $page, string $url)
* ```
*
* @param string $eventName
* @param int $timeout
* @param int $timeout time in ms to wait for the navigation to complete. Default 30000 (30 seconds)
* @return mixed|null
* @throws Exception\CommunicationException\CannotReadResponse
* @throws Exception\CommunicationException\InvalidResponse
Expand All @@ -105,15 +114,19 @@ public function __construct(Page $page, string $url)
* @throws NavigationExpired
* @throws ResponseHasError
*/
public function waitForNavigation($eventName = Page::LOAD, $timeout = 30000)
public function waitForNavigation($eventName = Page::LOAD, int $timeout = null)
{
if (null === $timeout) {
$timeout = 30000;
}
return Utils::tryWithTimeout($timeout * 1000, $this->navigationComplete($eventName));
}

/**
* To be used with @see Utils::tryWithTimeout
*
* @param $eventName
* @param string $eventName
*
* @return bool|\Generator
* @throws Exception\CommunicationException\CannotReadResponse
* @throws Exception\CommunicationException\InvalidResponse
Expand All @@ -126,6 +139,7 @@ private function navigationComplete($eventName)
$delay = 500;

while (true) {
// read the response only if it was not read already
if (!$this->navigateResponseReader->hasResponse()) {
$this->navigateResponseReader->checkForResponse();
if ($this->navigateResponseReader->hasResponse()) {
Expand Down Expand Up @@ -163,9 +177,14 @@ private function navigationComplete($eventName)

// else if a new loader is present that means that a new navigation started
} else {
throw new NavigationExpired(
'The page has navigated to an other page and this navigation expired'
);
// if strict then throw or else replace the old navigation with the new one
if ($this->strict) {
throw new NavigationExpired(
'The page has navigated to an other page and this navigation expired'
);
} else {
$this->currentLoaderId = $this->frame->getLatestLoaderId();
}
}

$this->page->getSession()->getConnection()->readData();
Expand Down
14 changes: 14 additions & 0 deletions test/suites/BrowsingTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -85,4 +85,18 @@ public function testGetCurrentUrl()

$this->assertEquals($this->sitePath('a.html'), $page->getCurrentUrl());
}

public function testPageNavigationLocalNotFoundUrl()
{
$factory = new BrowserFactory();
$browser = $factory->createBrowser();

$page = $browser->createPage();

// for some reasons chrome creates a new loader when we navigate to a local non-existent file
// here we are testing that feature with strict and non strict modes
$page->navigate('file:///does-not-exist')->waitForNavigation();

$this->assertTrue(true);
}
}

0 comments on commit d19ec0c

Please sign in to comment.