minkphp / webdriver-classic-driver

Mink driver for the W3C WebDriver Classic protocol
MIT License
3 stars 5 forks source link

Fix and streamline capability handling #51

Open uuf6429 opened 3 days ago

uuf6429 commented 3 days ago

Fixes #46.

What Changed

  1. Fixes code preventing us from overwriting capbalities when they're already set by php-webdriver.
  2. Streamlines the flow so that it is clearer how it behaves:
    1. start with the browser-specific capabilities provided by php-webdriver
    2. add mink default capabilities
    3. add mink browser-specific default capabilities
    4. add capabilities defined by the end user
  3. 'add', in this case, always overwrites existing capabilities

Remaining Questions

codecov[bot] commented 3 days ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 87.37%. Comparing base (ca55cde) to head (25d1859). Report is 1 commits behind head on main.

:white_check_mark: All tests successful. No failed tests found.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #51 +/- ## ============================================ + Coverage 84.40% 87.37% +2.97% + Complexity 196 195 -1 ============================================ Files 1 1 Lines 500 499 -1 ============================================ + Hits 422 436 +14 + Misses 78 63 -15 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.


🚨 Try these New Features:

aik099 commented 1 day ago

Since we're not mocking the underlying WebDriver library to confirm, that proper desired capabilities were given at session creation time, then we should fall back to a functional test (like with timeouts) to indirectly confirm, that the correct desired capabilities were given.

@uuf6429 , any ideas?

stof commented 1 day ago

thus, this driver does not have a stable release yet. So there is no BC guarantee yet.

uuf6429 commented 1 day ago

Since we're not mocking the underlying WebDriver library to confirm, that proper desired capabilities were given at session creation time

@aik099 Why not? I can simply use an anonymous subclass to test that.

Anyway, you can look at my take on it when I push it soon.

uuf6429 commented 1 day ago

Side-note: Currently there's no way to unset top-level capabilities, except with potentially wrong values (e.g. to "unset" 'name' one would need to pass ['name' => null] - which is more like invalidation than unsetting, and might still affect selenium).

I don't know if this is significant. One possible way is to define a constant/special value as a amrker:

class WebDriver {
    public const CAPABILITY_UNSET = '{[%unset%]}';
...
    $capabilities = array_filter(
        array_merge(
            $capabilities1,
            $capabilities2,
            $capabilities3
        ),
        function ($value) {
            return $value !== self::CAPABILITY_UNSET;
        }
    );

Alternatively, a singleton (and later on, enums) could be used:

final class Capabilities
{
    private function __construct(){}

    public static function UNSET(): self
    {
        static $unset = null;
        return $unset ?? ($unset = new self());
    }
}

Come to think of it, we could also consider adding this functionality later if needed - it should be backword compatible.