Test failure: MysqliConnectionTest::testRestoresErrorHandlerOnException on appveyor

[photodude]

The following failure occurred on recent appveyor testing with
db=mariadb, driver=pdo_mysql
db=mssql, driver=sqlsrv, db_version=sql2008r2sp2
db=mssql, driver=pdo_sqlsrv, db_version=sql2008r2sp2

There was 1 failure:
1) Doctrine\Tests\DBAL\Driver\Mysqli\MysqliConnectionTest::testRestoresErrorHandlerOnException
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'Network is unreachable'
+'The requested address is not valid in its context.
+'

C:\projects\dbal\tests\Doctrine\Tests\DBAL\Driver\Mysqli\MysqliConnectionTest.php:45

https://ci.appveyor.com/project/photodude/dbal/build/1.0.256/job/b74yrv4i6h3i0b1y
https://ci.appveyor.com/project/photodude/dbal/build/1.0.256/job/iakaekt0evwh68lr
https://ci.appveyor.com/project/photodude/dbal/build/1.0.257/job/th66xvantxwway5t

[Majkl578]

Assertion was added in #2704.

[photodude]

@Majkl578 looking at #2704 it seems some platforms have different strings in this test.
Network is unreachable is a valid error in some platforms,
The requested address is not valid in its context is a valid error in others.

[Majkl578]

@photodude The simplest way seems to be to remove the assert for message and only assert that exception was thrown. Maybe it could be replaced by asserting exception code, if there is any.

[photodude]

@Ocramius Would @Majkl578's proposal^ to "only assert that an exception was thrown" be an acceptable solution?

[morozov]

FWIW, dropping the verification of the exception message is acceptable since it belongs to the current TCP/IP stack implementation, not to the DBAL or even the DB platform.

Unless there's something else which we can use to identify the nature of the error (like the error code mentioned above), we can just verify the fact of the exception thrown.

[photodude]

I did a quick test switching $e->getMessage() with $e->getCode() in the test. Unfortunetly I got 0 as the error code. 0 does not seem like a valid code to me.

https://travis-ci.org/photodude/dbal/jobs/322699909

seems like just checking that an exception was thrown might be the primary option.

[morozov]

@photodude #2704 seems to be about a generic error, not necessarily the "Network is unreachable" one. Looks like not the best way to trigger a generic error was chosen.

The test checks the error message to prove that this is the error triggered by mysqli::real_connect(), not another accidental one. We probably could find some other circumstances under which it will trigger a predictable error and use it instead. For instance, trying to connect with a knowingly wrong password. In this case, the error message will be defined by MySQL, not by the underlying network layer.

[photodude]

Also note this is an issue specifically with testing on windows as it's coming from the WIP appveyor CI PR

[morozov]

@photodude I understand. That's why I'm suggesting instead of triggering the error whose message depends on the OS to trigger another one which doesn't.

[photodude]

Closing as addressed with the completion of and scope changes in the initial AppVeyor project

[github-actions]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.