MSSQL PDO cannot connect - Argument 1 passed ... must be an instance of PDO, array given

[TomHAnderson]

• Laravel Version: 8.18.1
• PHP Version: 7.4.13
• Database Driver & Version: MSSQL

Description:

Using a MSSQL database in a Docker container and trying to run migrations I get an exception. This is a real easy one.

https://github.com/laravel/framework/blob/8.x/src/Illuminate/Database/PDO/SqlServerDriver.php
types for an array which is then directly passed to
https://github.com/laravel/framework/blob/8.x/src/Illuminate/Database/PDO/Connection.php#L31
which types for a PDO object.

The correct fix is
https://github.com/laravel/framework/blob/8.x/src/Illuminate/Database/PDO/SqlServerDriver.php#L12
to

new Connection($params['pdo'])

Steps To Reproduce:

Try to run a migration on MSSQL.

[taylorotwell]

If you've already diagnosed it, why not send in a PR? 😄

[driesvints]

Thanks, I've sent in a PR here: #35564

[TomHAnderson]

I didn't send a PR because I'm not ready to dive into your framework for unit testing MSSQL.

[driesvints]

@TomHAnderson understandable. I've also just sent in the PR without tests since we don't have a CI setup for SqlServer atm.

[TomHAnderson]

You should use Docker for CI. No joke, that's going to be the best way to test all your supported databases. I'm taken aback that you're not testing your MSSQL support right now.

See https://github.com/doctrine/DoctrineMongoODMModule#development for an implementation I did for a Mongo module. See https://github.com/team-telnyx/telnyx-php#development for another Docker for unit testing I did.

I think Laravel is far behind since you're not already using this tech for your unit tests. This just doesn't cut the mustard: https://github.com/laravel/framework/blob/8.x/docker-compose.yml

If you agree LMK and I'll open a new PR issue for discussion.

[afraca]

I know this is closed already but I wholeheartedly agree with @TomHAnderson . If you provide drivers in the framework they should be tested. (Though perhaps part of it is already tested with extensive mocking?) This is a really embarrassing bug with something like Docker these days, although I will admit setting it up would be no small task!

[driesvints]

@TomHAnderson @afraca the entire MSSQL implementation has been largely a community contribution since none of us at Laravel use it anywhere in production or for our own products. We have no experience with it ourselves therefor we rely on the community to contribute here. This is still open source and we very much welcome any help in improving its implementation as well as additional tests.

This is a really embarrassing bug

The fact that no one stumbled on this before says a lot more about the adaptation of the driver than anything else really.

This just doesn't cut the mustard:

Btw Tom, that docker compose file is only meant for us to quickly run a part of the test suite locally and nothing more.