Empty array causes entity_lookup to fail

I'm happy to write a few tests for this but I have to confess I've never written unit tests for Drupal and it's a bit of a learning curve. Is there documentation for how to get everything set up relating to this module specifically? There's no phpunit.xml file in this repo, so I assume testing has to be done in the context of a full drupal installation, but the way the current tests are organized just for migrate_plus is a bit confusing to a newcomer. A few questions off the top of my head:

* I assume this should be in the Unit\process namespace, not the Kernel\Plugin\migrate\process namespace? The Kernel stuff looks more end-to-end, while this test just needs to make sure the transform method works properly. (Though, after writing this and playing with it a bit more, I'm not sure I can get all the dependencies required by the entity_lookup plugin from within a simple unit test; maybe it has to go into the Kernel tests?)
* The only unit test is currently for skip_on_value, but I can't quite use that as a pattern because the constructor for entity_lookup uses some values that I'm not sure where they'd come from in a testing scenario (specifically the MigrationInterface, EntityManagerInterface, and SelectionPluginManagerInterface values if I'm using "new EntityLookup(...)" or the ContainerInterface if I'm using "EntityLookup::create()" in my setup).
* I ended up getting stuck while writing the Kernel test because I think if I'm going to write the kernel test I have to basically write an entire migration in the test?

Like I said, I'm new to Drupal unit testing and I'm not 100% sure how to proceed in this case. I'm attaching what I have so far as a test:

<?php

namespace Drupal\Tests\migrate_plus\Kernel\Plugin\migrate\process;

use Drupal\taxonomy\Entity\Term;
use Drupal\KernelTests\KernelTestBase;
use Drupal\taxonomy\Entity\Vocabulary;
use Drupal\migrate\Plugin\MigrationInterface;
use Drupal\migrate_plus\Plugin\migrate\process\EntityLookup;

/**
 * Tests the Entity Lookup process plugin.
 *
 * @group migrate_plus
 * @coversDefaultClass \Drupal\migrate_plus\Plugin\migrate\process\EntityLookup
 */
class EntityLookupTest extends KernelTestBase {

  /**
   * {@inheritdoc}
   */
  public static $modules = [
    'taxonomy',
    'text',
  ];

  /**
   * The vocabulary id.
   *
   * @var string
   */
  protected $vocabulary = 'test_vocab';

  /**
   * The vocabulary terms.
   *
   * @var array
   */
  protected $terms = [
    'test_term1',
    'test_term2',
  ];

  /**
   * {@inheritdoc}
   */
  protected function setUp() {
    parent::setUp();

    $this->installEntitySchema('taxonomy_term');
    $this->installEntitySchema('taxonomy_vocabulary');

    $vocab = Vocabulary::create([
      'name' => $this->vocabulary,
      'description' => $this->vocabulary,
      'vid' => $this->vocabulary,
      'langcode' => 'en',
    ]);
    $vocab->save();

    foreach ($this->terms as $term) {
      $t = Term::create([
        'name' => $term,
        'vid' => $this->vocabulary,
      ]);
      $t->save();
    }
  }

  /**
   * @covers ::transform
   */
  public function testTransformWithSingleValue() {
    $configuration['bundle'] = $this->vocabulary;
    $entityLookup = EntityLookup::create(
      $this->container,
      $configuration,
      'entity_lookup',
      [],
      $this->getMock(MigrationInterface::class)
    );
    $value = $entityLookup->transform('test_term1', $this->migrateExecutable, $this->row, 'destinationproperty');
    $this->assertNotNull($value);
  }

}

I've written a test that should cover the basic functionality of entity_lookup as well as the observed defect:

1. Test that looking up a single value that has a matching term saves a term reference.
2. Test that looking up a single value that does not have a matching term does not save a term reference.
3. Test that looking up multiple values that all have matching terms saves multiple term references.
4. Test that looking up multiple values, one of which has a matching term and the other which does not, saves only the matched term reference.
5. *DEFECT* Test that looking up an empty array does not save any term references and also does not throw an exception.

No need to add a whole new class, just add a new scenario.

I created a new class because we're testing two different plugins - unfortunately entity_generate doesn't share any code with entity_lookup, even when entity_generate looks up taxonomy terms. And the specific test case I'm covering - that passing an empty array to entity_lookup causes the lookup to fail - isn't something that would be testable in entity_generate. (Though now that I'm thinking about it, I wonder how entity_generate would handle an empty array.) So from a code organization standpoint and a test coverage standpoint, the specific test case and indeed any of the tests for entity_lookup I would think should live in a class called EntityGenerateTest.

Having said that, there is definitely overlap between the code for both and I'm sure some of that overlap could be broken out into a common class and some of the adjustments I had to make for entity_lookup could be ported over to entity_generate; but the core of the test - that looking up an empty array doesn't cause an error - I think can't/shouldn't be done in entity_generate because entity_generate wasn't the problem.

I'll take another crack at this now that I've written my own plugins and tests for those plugins.

The plugin itself didn't need any modifications but the test needed to be reworked since the original EntityGenerateTest changed since I played with this last. So here's a new patch incorporating both the plugin and the test.

As a lookup, though? That line is only seeing what kind of value should be returned in the case of an empty value; if the value passed in was an empty string, it returns NULL. I'm struggling to find a use case where an empty string would be intentionally used as a lookup value. I mean, if there is a use case, then sure, we can remove that logic; but part of me thinks if there's a legitimate need to look up something via an empty string, there's probably a better way to find what you're trying to look up.

And since the bug/error only occurs when $value is an array and not with any of the other "falsy" values, I would not change the behaviour for those.

That's fair. I'll look into restricting the logic to just arrays for the moment (rather than PHP's nebulous definition of what constitutes an empty() value). Tracking and stopping this in the query() method is a separate ball of wax for the moment.

Made my modification a bit less greedy. Still kept it in the transform method though; no use wasting extra cycles going to the query method if we can bail before that point.