Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 16 additions & 2 deletions src/Element/ElementFinder.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
namespace Behat\Mink\Element;

use Behat\Mink\Driver\DriverInterface;
use Behat\Mink\Selector\NamedSelectorMode;
use Behat\Mink\Selector\SelectorsHandler;
use Behat\Mink\Selector\Xpath\Manipulator;

Expand All @@ -32,12 +33,25 @@ class ElementFinder
* @var Manipulator
*/
private $xpathManipulator;
/**
* @var string one of the NamedSelectorMode constants

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @var string one of the NamedSelectorMode constants
* @var NamedSelectorMode::*

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied. Session::__construct() was already documented as NamedSelectorMode::*, so ElementFinder was simply the odd one out. I also dropped the "one of the NamedSelectorMode constants" prose from both, since the type now says it better than the sentence did.

*/
private $namedMode;

public function __construct(DriverInterface $driver, SelectorsHandler $selectorsHandler, ?Manipulator $xpathManipulator = null)
/**
* @param string $namedMode How the "named" selector is resolved: one of the

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @param string $namedMode How the "named" selector is resolved: one of the
* @param NamedSelectorMode::* $namedSelectorMode

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied, with one small change: the parameter is called $namedMode, not $namedSelectorMode, so I kept the existing name to avoid documenting a parameter that does not exist.

Worth knowing that the narrowed type made PHPStan fail on ElementFinderTest, where testUnknownNamedModeIsRejected deliberately passes invalid values to exercise the runtime guard. I put a targeted @phpstan-ignore argument.type on that single line with a reason. This project usually keeps its exclusions in phpstan.dist.neon, so tell me if you would rather have it there and I will move it.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would actually rename the parameter. What is named is the selector, not the mode.

I put a targeted @phpstan-ignore argument.type on that single line with a reason.

A targeted comment is fine for this case, as this is a case where we intentionally pass invalid data.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed, you're right — the mode is a property of the selector, not something named. $namedSelectorMode now, for the constructor parameter and the property on both ElementFinder and Session, with the test names following suit (testUnknownNamedSelectorModeIsRejected, testNamedSelectorModeIsForwardedToTheElementFinder, testTheDefaultNamedSelectorModeStillFallsBackToPartial).

And thanks for settling the ignore, I'll leave the targeted comment where it is.

* NamedSelectorMode constants.
*/
public function __construct(DriverInterface $driver, SelectorsHandler $selectorsHandler, ?Manipulator $xpathManipulator = null, string $namedMode = NamedSelectorMode::PARTIAL_FALLBACK)
{
if (NamedSelectorMode::PARTIAL_FALLBACK !== $namedMode && NamedSelectorMode::EXACT !== $namedMode) {
throw new \InvalidArgumentException(sprintf('Unknown named selector mode "%s".', $namedMode));
}

$this->driver = $driver;
$this->selectorsHandler = $selectorsHandler;
$this->xpathManipulator = $xpathManipulator ?? new Manipulator();
$this->namedMode = $namedMode;
}

/**
Expand All @@ -49,7 +63,7 @@ public function findAll(string $selector, $locator, string $parentXpath)
{
if ('named' === $selector) {
$items = $this->findAll('named_exact', $locator, $parentXpath);
if (empty($items)) {
if (empty($items) && NamedSelectorMode::EXACT !== $this->namedMode) {
$items = $this->findAll('named_partial', $locator, $parentXpath);
}

Expand Down
36 changes: 36 additions & 0 deletions src/Selector/NamedSelectorMode.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
<?php

/*
* This file is part of the Mink package.
* (c) Konstantin Kudryashov <ever.zet@gmail.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Behat\Mink\Selector;

/**
* How the "named" selector resolves a locator.
*
* This is a placeholder for an enum, which the minimum PHP version does not allow yet.
*/
final class NamedSelectorMode
{
/**
* Try an exact match first, and fall back to a partial one when it finds nothing.
*/
public const PARTIAL_FALLBACK = 'partial_fallback';

/**
* Only match exactly, the way the "named_exact" selector does.
*/
public const EXACT = 'exact';

/**
* @codeCoverageIgnore
*/
private function __construct()
{
}
}
9 changes: 7 additions & 2 deletions src/Session.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

use Behat\Mink\Driver\DriverInterface;
use Behat\Mink\Element\ElementFinder;
use Behat\Mink\Selector\NamedSelectorMode;
use Behat\Mink\Selector\SelectorsHandler;
use Behat\Mink\Element\DocumentElement;

Expand Down Expand Up @@ -39,11 +40,15 @@ class Session
*/
private $selectorsHandler;

public function __construct(DriverInterface $driver, ?SelectorsHandler $selectorsHandler = null)
/**
* @param NamedSelectorMode::* $namedMode How the "named" selector is resolved: one of the
* NamedSelectorMode constants.
*/
public function __construct(DriverInterface $driver, ?SelectorsHandler $selectorsHandler = null, string $namedMode = NamedSelectorMode::PARTIAL_FALLBACK)
{
$this->driver = $driver;
$this->selectorsHandler = $selectorsHandler ?? new SelectorsHandler();
$this->elementFinder = new ElementFinder($driver, $this->selectorsHandler);
$this->elementFinder = new ElementFinder($driver, $this->selectorsHandler, null, $namedMode);
$this->page = new DocumentElement($this);

$driver->setSession($this);
Expand Down
46 changes: 46 additions & 0 deletions tests/Element/ElementFinderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use Behat\Mink\Driver\DriverInterface;
use Behat\Mink\Element\ElementFinder;
use Behat\Mink\Element\NodeElement;
use Behat\Mink\Selector\NamedSelectorMode;
use Behat\Mink\Selector\SelectorsHandler;
use Behat\Mink\Selector\Xpath\Manipulator;
use PHPUnit\Framework\MockObject\MockObject;
Expand Down Expand Up @@ -159,4 +160,49 @@ public function testNamedPartialFallback()
$this->assertEquals('element1', $results[0]->getXpath());
$this->assertEquals('element2', $results[1]->getXpath());
}

public function testNamedExactModeDoesNotFallBackToPartial()
{
$finder = new ElementFinder($this->driver, $this->selectorsHandler, $this->manipulator, NamedSelectorMode::EXACT);

$this->selectorsHandler->expects($this->once())
->method('selectorToXpath')
->with('named_exact', 'test')
->will($this->returnValue('named_xpath'));

$this->manipulator->expects($this->once())
->method('prepend')
->with('named_xpath', 'parent_xpath')
->will($this->returnValue('full_xpath'));

$this->driver->expects($this->once())
->method('find')
->with('full_xpath')
->will($this->returnValue(array()));

$this->assertEquals(array(), $finder->findAll('named', 'test', 'parent_xpath'));
}

/**
* @dataProvider provideInvalidNamedModes
*/
public function testUnknownNamedModeIsRejected(string $namedMode)
{
$this->expectException(\InvalidArgumentException::class);
$this->expectExceptionMessage(sprintf('Unknown named selector mode "%s".', $namedMode));

new ElementFinder($this->driver, $this->selectorsHandler, $this->manipulator, $namedMode);
}

/**
* @return array<string, array{string}>
*/
public static function provideInvalidNamedModes()
{
return array(
'unknown value' => array('nope'),
'empty string' => array(''),
'the selector name itself' => array('named_exact'),
);
}
}
40 changes: 40 additions & 0 deletions tests/SessionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
namespace Behat\Mink\Tests;

use Behat\Mink\Driver\DriverInterface;
use Behat\Mink\Element\ElementFinder;
use Behat\Mink\Selector\NamedSelectorMode;
use Behat\Mink\Selector\SelectorsHandler;
use Behat\Mink\Session;
use PHPUnit\Framework\MockObject\MockObject;
Expand Down Expand Up @@ -355,4 +357,42 @@ public function testMaximizeWindow()

$this->session->maximizeWindow('test');
}

public function testNamedModeIsForwardedToTheElementFinder()
{
$selectorsHandler = $this->createMock(SelectorsHandler::class);
$selectorsHandler->expects($this->once())
->method('selectorToXpath')
->with('named_exact', 'test')
->willReturn('named_xpath');

// a partial fallback would query the driver a second time
$this->driver->expects($this->once())
->method('find')
->willReturn(array());

$session = new Session($this->driver, $selectorsHandler, NamedSelectorMode::EXACT);

$this->assertSame(array(), $session->getPage()->findAll('named', 'test'));
}

public function testTheDefaultNamedModeStillFallsBackToPartial()
{
$selectorsHandler = $this->createMock(SelectorsHandler::class);
$selectorsHandler->expects($this->exactly(2))
->method('selectorToXpath')
->willReturnMap(array(
array('named_exact', 'test', 'named_xpath'),
array('named_partial', 'test', 'partial_xpath'),
));

// the exact lookup finds nothing, so the partial one is queried as well
$this->driver->expects($this->exactly(2))
->method('find')
->willReturn(array());

$session = new Session($this->driver, $selectorsHandler);

$this->assertSame(array(), $session->getPage()->findAll('named', 'test'));
}
}
Loading