Connection::getDriverClass doesn't support non-core drivers

+++ b/core/lib/Drupal/Core/Database/Connection.php
@@ -659,13 +659,12 @@ protected function expandArguments(&$query, &$args) {
       else {
         // Fallback for Drupal 7 settings.php.
-        $driver_class = "Drupal\\Core\\Database\\Driver\\{$driver}\\{$class}";
+        $driver_class = 'Drupal\Core\Database\Driver\\' . $this->driver() . '\\' . $class;
       }

Why do we support the Drupal 7 way of doing it, in the first place? Can't we drop that now?

Alright, so we need it to run tests, maybe we could document it at some point

Looks great for me. Thank you for adding a bit more explanation.

I think we should get some feedback from crell.

I *think* this is what we're talking about; the new code for D8 should handle any autoloadable class. It's the D7 code path that doesn't work for non-core libs, which I think it only used for upgrades, no? I don't know how testbot gets into this...

So what is missing? Can this be RTBCed? It looks good to me ...

With that follow up filed, this looks good.

Any reason why we don't avoid reflection and just do substr(get_called_class(), 0, strrpos(get_called_class(), "\\"))

+++ b/core/lib/Drupal/Core/Database/Connection.php
@@ -659,14 +659,14 @@ protected function expandArguments(&$query, &$args) {
       if (!empty($this->connectionOptions['namespace'])) {
-        $driver_class  = $this->connectionOptions['namespace'] . '\\' . $class;
+        $namespace  = $this->connectionOptions['namespace'];
       }
       else {
-        // Fallback for Drupal 7 settings.php.
-        $driver_class = "Drupal\\Core\\Database\\Driver\\{$driver}\\{$class}";
+        // Fallback for Drupal 7 settings.php and the test runner script.
+        $namespace = (new \ReflectionObject($this))->getNamespaceName();

Can we set the $this->connectionOptions['namespace'] so we don't have to do the reflection multiple times for different classes?

Unassigning stale issue. Hopefully someone else will pursue this.

I have changed the patch so that the reflection is not run multiple times.

As for:

Any reason why we don't avoid reflection and just do substr(get_called_class(), 0, strrpos(get_called_class(), "\\"))

I have no idea.

For the testbot

With the patch from comment #22, I have tried to make the calling of the ReflextionObject as little as possible by saving the namespace to $this->connectionOptions['namespace']. The problem is that the test ConnectionTest::testConnectionOptions() then fails. In that test a second database connection is made. The connection info from both database connections are tested to see if they are identical. If I save the namespace value to $this->connectionOptions['namespace'] they are not identical. So the question of @alexpott:

Can we set the $this->connectionOptions['namespace'] so we don't have to do the reflection multiple times for different classes?

has to be answered with no.
The first question from @alexpott has been answered by @chx in the previous comment.

In comment #18 is this issue set to RTBC by @Crell. So back to RTBC for the patch from comment #14. That patch still applies.

testbot sadly disagrees with you :(

My interdiff tools says this is the interdiff between #14 and #22. Its possible and likely the missing file is causing your failures.

@neclimdul: The patch from comment #14 made by @chx is the patch that is RTBC for @Crell. Not my patch from comment #22. In that patch have I tried to fix the remarks made by @alexpott. Those remarks are not fixable so I have made the patch from comment #14 RTBC again. If the patch from comment #14 made by chx is not RTBC for you, then please change the status of this issue.

I made a mistake in my patch in comment #22 by not adding the the database stub. I have tried locally to pass the ConnectionTest with the database stub and I failed. See comment #27 for the explanation why.

I think we can do less reflection if we do something like the patch attached.

I am not exactly sure why it passes the ConnectionTest but it does.

It all looks good to me.
It fixed the problem from the issue summary.
The 2 problems from alexpott in comment #19 are addressed. With one being explained by chx in comment #26.

For me it is RTBC.

Neat! Thanks guys!

edit: PS: Sorry about my confusion. Thanks for addressing it and improving everything though!

Committed/pushed to 8.2.x and cherry-picked to 8.1.x. Thanks!