Fix up doc in SqlBase mapJoinable() and add test coverage

Can we add some tests for the issue addressed in #2181775: SQL idmap broke when using separate mysql users for source and destination

Everything looks good - just a minor comment and a question.

Thanks,
-mike

1. +++ b/core/modules/migrate/tests/src/Unit/SqlBaseTest.php
   @@ -0,0 +1,153 @@
   +
   

   Minor - extra linefeed.

2. +++ b/core/modules/migrate/tests/src/Unit/SqlBaseTest.php
   @@ -0,0 +1,153 @@
   +    $this->assertEquals($sql_base->mapJoinable(), TRUE);
   

   I'm not sure I understand why this assertion fails the first time and works the second time...

Let's do this.

1. +++ b/core/modules/migrate/tests/src/Unit/SqlBaseTest.php
   @@ -0,0 +1,170 @@
   +class SqlBaseTest extends MigrateTestCase {
   

   Why is this not just extending UnitTestCase - I can't see why it needs MigrateTestCase

2. +++ b/core/modules/migrate/tests/src/Unit/SqlBaseTest.php
   @@ -0,0 +1,170 @@
   +  /**
   ...
   +  public function testMapJoinable() {
   +
   +    // Source ids are empty so mapJoinable() is false.
   +    $sql_base = $this->getSqlBase(FALSE, FALSE);
   +    $this->assertEquals($sql_base->mapJoinable(), FALSE);
   +
   +    // Still false because getIdMap() is not a subclass of Sql.
   +    $sql_base = $this->getSqlBase(FALSE, TRUE);
   +    $this->assertEquals($sql_base->mapJoinable(), FALSE);
   +
   +    // Test mapJoinable() returns false when source and id connection options
   +    // differ.
   +    $sql_base = $this->getSqlBase(TRUE, TRUE, ['username' => 'different_from_map', 'password' => 'different_from_map'], ['username' => 'different_from_source', 'password' => 'different_from_source']);
   +    $this->assertEquals($sql_base->mapJoinable(), FALSE);
   +
   +    // Returns true because source and id map connection options are the same.
   +    $options = ['username' => 'same_value', 'password' => 'same_value'];
   +    $sql_base = $this->getSqlBase(TRUE, TRUE, $options, $options);
   +    $this->assertEquals($sql_base->mapJoinable(), TRUE);
   +  }
   

   This could be a dataprovider to getSqlBase - which would need to be changed to a test and have an assertion.

That's really great, let me resurrect the comments however, those were useful. Did very minor cleanup as well.

Back to where it belongs...

Looks like Alex was looking into this one.

And again back to RTBC.

Migrate changes are unfrozen as part of beta evaluation. Committed ca51249 and pushed to 8.0.x. Thanks!