Allow custom processing of WebAssert::assert through callback about mink HOT 9 CLOSED

instead of forcing to customize WebAssert (which also means we will never be able to add new kinds of failures in the proposed API as that would break BC for your callback), a cleaner solution would be to use the dedicated PHPUnit feature to handle some exceptions as failures.

See https://github.com/minkphp/driver-testsuite/blob/af6107c1f16f4c43c19ea162e0da88d55c6cf21a/tests/TestCase.php#L74-L81 where our driver testsuite uses that hook to mark tests as skipped instead of errored when a Behat\Mink\Exception\UnsupportedDriverActionException exception is thrown. You could do the same to turn Behat\Mink\Exception\ExpectationException into a failure instead

from mink.

Yes but doing that means re-throwing the exception from the onNotSuccessful and therefore changing the exception type. We have tried to propose changes to PHPUnit to facilitate this see sebastianbergmann/phpunit#4983 (comment)

Atm if we want to mark Mink exceptions as PHPUnit failures we have do something like:

    if ($t instanceof MinkException) {
      $e = new AssertionFailedError($t->getMessage(), $t->getCode(), $t);
      $trace = $t->getTrace();
      foreach ($trace as $i => $call) {
        unset($trace[$i]['args']);
      }
      $reflection = new \ReflectionProperty($e, 'serializableTrace');
      $reflection->setValue($e, $trace);
      throw $e;
    }

in order to preserve the trace. Which in my mind not very sensible.

from mink.

AFAIU, the need for this complex code is caused by AssertionFailedError loosing the chained exception when serializing it for process isolation. I would rather work on fixing that in PHPUnit (as chaining exceptions was the suggested solution in sebastianbergmann/phpunit#4983 (comment))

from mink.

I have filed sebastianbergmann/phpunit#5068 which, I think, would solve the trace issue while using the solution proposed here in #835 (comment) which seems to me the cleanest one.

from mink.

PHPUnit rejected sebastianbergmann/phpunit#5069 and will not pursue sebastianbergmann/phpunit#5068.

from mink.

Well, if he decided that mapping the details is the way to go, he should probably bundle an utility in PHPUnit to make it easy to copy the trace, given the hacks it requires. but that should be discussed with PHPUnit, as that's clearly a PHPUnit limitation here.

The proposal done here adds lots of complexity in Mink and means that we cannot add new assertions outside of major versions as it would change the requirements for that callback.

from mink.

Alright, I understand the position here. I will try something else on Drupal side, before calling time of death. Thanks

from mink.

sebastianbergmann/phpunit#5201 might allow to solve the root cause without the need for this weird feature request. So you can subscribe to that issue.

from mink.

Thanks for the pointer, subscribed there.

I’d say what’s weird here could be the solution - the feature request per se is quite sensible.

from mink.