search

minkphp / driver-testsuite

Functional testsuite for Mink drivers
MIT License
8 stars 29 forks source link

Test was not remapping remotely-used fixture file #67

Closed uuf6429 closed 1 year ago

uuf6429 commented 1 year ago

Title says it all. :)

aik099 commented 1 year ago

Good catch, but I have no idea how to test it considering we're using Selenium 2 for testing and planned (but not implemented) to create a separate driver and Selenium 3.

uuf6429 commented 1 year ago

Good catch, but I have no idea how to test it considering we're using Selenium 2 for testing and planned (but not implemented) to create a separate driver and Selenium 3.

I'm not sure what you mean - this is used within tests; if they keep on passing all is well. What is a bit confusing, actually, is how MinkSelenium2Driver tests were not failing.

aik099 commented 1 year ago

Good catch, but I have no idea how to test it considering we're using Selenium 2 for testing and planned (but not implemented) to create a separate driver and Selenium 3.

I'm not sure what you mean - this is used within tests; if they keep on passing all is well. What is a bit confusing, actually, is how MinkSelenium2Driver tests were not failing.

GitHub Tests use Selenium 2. Maybe it's working there.

We don't use Selenium 3 in GitHub tests. Maybe it's failing for Selenium 3 only.

uuf6429 commented 1 year ago

@aik099 the reason it wasn't failing before is because the MinkSelenium2Driver test shares the fixture path in a way that it doesn't need a remapping, which is also why the variables are not set by default: https://github.com/minkphp/MinkSelenium2Driver/blob/master/phpunit.xml.dist#L24 Although I believe this may be a case of completely skipping failures in that test class: https://github.com/minkphp/MinkSelenium2Driver/blob/master/tests/Selenium2Config.php#L32

I could change my current tests in github to also make it work like so, but failing to remap the paths properly (as is the case now) will break the test on Windows (something I already noticed and fixed).

So quite frankly, if the tests are currently passing for everyone and with my changes they'll still be passing, then don't worry about it and just merge. ;)

Edit: if you'd like to test MinkSelenium2Driver against this, you could always open a temporary branch in that project and point it to my branch through composer, and push the changes to trigger github actions.

aik099 commented 1 year ago

@uuf6429 , I like the current PR code.