D10: Re-vet Token (drupal/token) & Imagecache Token

I set up a new D7 instance and installed token & imagecache token. Here is what the /admin/help/token page looked like after just installing token:

Then installed imagecache token and the help page now has the additional tokens included:

To test that the token replacement is working (without needing to also pull in another module to vet such as token_filter) I created a new text field, added the added the following:

To test that this actually works I added a little hook:

function dan_token_test_preprocess_field(&$variables) {
  if($variables['element']['#bundle'] !== 'article') {
    return;
  }
  if($variables['element']['#field_name'] !== 'field_token_test') {
    return;
  }
  $variables['items']['0']['#markup'] = token_replace($variables['items']['0']['#markup']);
}

And here is the before/after:

I think now it would be best to run the test suite in the token module. Here are the results:

www-data@f17c72c38ff3:/app/docroot/core$ ../../vendor/bin/phpunit /app/docroot/modules/contrib/token
PHPUnit 9.6.16 by Sebastian Bergmann and contributors.

Testing /app/docroot/modules/contrib/token
.....E........E.................................................. 65 / 78 ( 83%)
..........F..                                                     78 / 78 (100%)

Time: 08:40.729, Memory: 16.00 MB

There were 2 errors:

1) Drupal\Tests\token\Functional\TokenMenuTest::testMenuTokens
Behat\Mink\Exception\ElementNotFoundException: Button with id|name|label|value "Save content type" not found.

/app/docroot/core/tests/Drupal/Tests/WebAssert.php:144
/app/docroot/core/tests/Drupal/Tests/UiHelperTrait.php:78
/app/docroot/modules/contrib/token/tests/src/Functional/TokenMenuTest.php:159
/app/vendor/phpunit/phpunit/src/Framework/TestResult.php:728

2) Drupal\Tests\token\Functional\Tree\HelpPageTest::testHelpPageTree
Behat\Mink\Exception\ResponseTextException: The text "The list of the currently available tokens on this site are shown below." was not found anywhere in the text of the current page.

/app/vendor/behat/mink/src/WebAssert.php:907
/app/vendor/behat/mink/src/WebAssert.php:293
/app/docroot/modules/contrib/token/tests/src/Functional/Tree/HelpPageTest.php:38
/app/vendor/phpunit/phpunit/src/Framework/TestResult.php:728

--

There was 1 failure:

1) Drupal\Tests\token\Kernel\ValidateD6MigrationStateTest::testMigrationState
assert(count($plugin_id_parts) >= 3) in /app/docroot/core/modules/menu_link_content/src/MenuLinkContentMigrationPluginsAlterer.php:78

/app/vendor/phpunit/phpunit/src/Framework/TestResult.php:756

ERRORS!
Tests: 78, Assertions: 1191, Errors: 2, Failures: 1.

🤔 will need to review these. I ran an one liner with drush and can confirm the token replace functionality is working:

Testing /app/docroot/modules/contrib/token
.....E........E.................................................. 65 / 78 ( 83%)
..........F..                                                     78 / 78 (100%)

Time: 08:40.729, Memory: 16.00 MB

There were 2 errors:

1) Drupal\Tests\token\Functional\TokenMenuTest::testMenuTokens
Behat\Mink\Exception\ElementNotFoundException: Button with id|name|label|value "Save content type" not found.
…
2) Drupal\Tests\token\Functional\Tree\HelpPageTest::testHelpPageTree
…

That's indeed odd, because it's passing here on 10.2.3: https://git.drupalcode.org/project/token/-/jobs/743268

… but you didn't apply any patches, so I don't see how that's possible. See https://git.drupalcode.org/project/token/-/commit/19bd13e587d0a4313b600c... (from 📌 Fix failled test on Drupal 10 Fixed ), which fixed tests on 10.2, and likely didn't make it into the actual release you're testing.

⚠️ More importantly:

There was 1 failure:

1) Drupal\Tests\token\Kernel\ValidateD6MigrationStateTest::testMigrationState
assert(count($plugin_id_parts) >= 3) in /app/docroot/core/modules/menu_link_content/src/MenuLinkContentMigrationPluginsAlterer.php:78

👆 Note that https://git.drupalcode.org/project/token/-/jobs/743268 is not running Kernel tests! 😱

So, it's very likely that that test is just plain broken. But it'd be good to understand for sure. Note that there's no mention about this at all in 📌 Use Gitlab CI Fixed .

That's indeed odd, because it's passing here on 10.2.3: https://git.drupalcode.org/project/token/-/jobs/743268

Regarding this issue:

2) Drupal\Tests\token\Functional\Tree\HelpPageTest::testHelpPageTree
Behat\Mink\Exception\ResponseTextException: The text "The list of the currently available tokens on this site are shown below." was not found anywhere in the text of the current page.

/app/vendor/behat/mink/src/WebAssert.php:907
/app/vendor/behat/mink/src/WebAssert.php:293
/app/docroot/modules/contrib/token/tests/src/Functional/Tree/HelpPageTest.php:38
/app/vendor/phpunit/phpunit/src/Framework/TestResult.php:728

I think the issue is with the permission being assigned to the user. "access administration pages" is not the a viable permission in and of itself. We need "access help pages" - at least that is what corrects the issue in my local setup.

Not sure why this seems to work on 10.2.3-dev. I think i found the changes needed - https://git.drupalcode.org/project/token/-/commit/19bd13e587d0a4313b600c... - but they are not landed in a release yet.

It looks like the failing Kernel test was introduced here - https://www.drupal.org/project/token/issues/3199214 → .

How/why is this being thrown from menu_link_content? How did we get there? ... the failing lines are in fixMultilingualNodeMenuLinkMigrations():

  /**
   * Refines multilingual node menu link migration.
   *
   * Removes "d7_menu_links" node derivatives if node menu link translation
   * migration is present.
   *
   * @param array[] $migrations
   *   An associative array of migration definitions.
   */
  public static function fixMultilingualNodeMenuLinkMigrations(array &$migrations) {
    $drupal_7_migrations = self::getMigrationsWithTag($migrations, 'Drupal 7');
    // If "content_translation" is enabled, we will migrate every node menu link
    // with "node_translation_menu_links" migration.
    $node_translation_menu_link_migrations = array_filter($drupal_7_migrations, function (array $definition) {
      return $definition['id'] === 'node_translation_menu_links';
    });
    if (empty($node_translation_menu_link_migrations)) {
      return;
    }

    foreach (array_keys($node_translation_menu_link_migrations) as $plugin_id) {
      $plugin_id_parts = explode(PluginBase::DERIVATIVE_SEPARATOR, $plugin_id);
      <strong>assert(count($plugin_id_parts) >= 3);</strong>
      [, $entity_type, $bundle] = $plugin_id_parts;
      assert($entity_type === 'node');

This gets triggered from a hook_migration_plugins_alter() in menu_link_content:

/**
 * Implements hook_migration_plugins_alter().
 */
function menu_link_content_migration_plugins_alter(array &$migrations) {
  MenuLinkContentMigrationPluginsAlterer::refineMenuLinkContentMigrationDependencies($migrations);
  MenuLinkContentMigrationPluginsAlterer::fixMultilingualNodeMenuLinkMigrations($migrations);
}

But im still not sure why this is being called to begin with. What triggers the migration_plugins_alter() hook, in the context of a ValidateD6MigrationStateTest?

I think it's this line causing issues:

foreach (array_keys($node_translation_menu_link_migrations) as $plugin_id) {

because the only array key in $node_translation_menu_link_migrations is node_translation_menu_links:

dump(array_keys($node_translation_menu_link_migrations));

...yields..

array:1 [
  0 => "node_translation_menu_links"
]

But this makes me wonder if there is some larger misconfiguration going on, as my array keys are way off - this code would never work if I am reading correctly. I'm still unsure why menu_link_content plugin alter hooks are being called from a token test.

I think the D7MenuLinkDeriver that was introduced here 🐛 Existing menu links show validation issues on migration (and ALL menu links pointing to node translations are invalid) Needs work is part of the issue. If it detects Drupal 6 as the source database, it will return without creating derivatives:

    // Do not create derivatives for Drupal 6 node translation menu link
    // migrations - return only one, with the ID which is equal to the base ID.
    // @see \Drupal\Component\Plugin\Discovery\DerivativeDiscoveryDecorator::getDerivatives()
    $legacy_drupal_version = static::getLegacyDrupalVersion($source->getDatabase());
    if ((int) $legacy_drupal_version !== 7) {
      return [0 => $base_plugin_definition];
    }

Without a check in fixMultilingualNodeMenuLinkMigrations() to ensure the plugin is a derivative, this fails. Adding a condition with a continue like this locally corrects the problem:

    foreach (array_keys($node_translation_menu_link_migrations) as $plugin_id) {
      $plugin_id_parts = explode(PluginBase::DERIVATIVE_SEPARATOR, $plugin_id);
      // Plugin is not a derivative.
      if (count($plugin_id_parts) < 3) {
        continue;
      }
      assert(count($plugin_id_parts) >= 3);

now running test:

www-data@2fdc9bf0644d:/app/docroot/core$ ../../vendor/bin/phpunit /app/docroot/modules/contrib/token/tests/src/Kernel/ValidateD6MigrationStateTest.php --debug
PHPUnit 9.6.16 by Sebastian Bergmann and contributors.

Testing Drupal\Tests\token\Kernel\ValidateD6MigrationStateTest
Test 'Drupal\Tests\token\Kernel\ValidateD6MigrationStateTest::testMigrationState' started
Test 'Drupal\Tests\token\Kernel\ValidateD6MigrationStateTest::testMigrationState' ended


Time: 00:08.926, Memory: 10.00 MB

OK (1 test, 52 assertions)

1. the Drupal\Tests\token\Kernel\ValidateD6MigrationStateTest::testMigrationState starts
2. this instantiates a new MigrationPluginManager
3. this triggers migration_plugins_alter() hooks
4. per https://www.drupal.org/project/drupal/issues/3051251 🐛 Existing menu links show validation issues on migration (and ALL menu links pointing to node translations are invalid) Needs work , there is a migration_plugins_alter() hook in menu_link_content which runs MenuLinkContentMigrationPluginsAlterer::refineMenuLinkContentMigrationDependencies() and MenuLinkContentMigrationPluginsAlterer::fixMultilingualNodeMenuLinkMigrations()
5. in fixMultilingualNodeMenuLinkMigrations() we attempt to first get all Migrations tagged as "Drupal 7", then we populate a variable ($node_translation_menu_link_migrations) with the plugin definition only if it has the id of node_translation_menu_links. At this point if the variable is empty we should return:

  public static function fixMultilingualNodeMenuLinkMigrations(array &$migrations) {
    $drupal_7_migrations = self::getMigrationsWithTag($migrations, 'Drupal 7');
    // If "content_translation" is enabled, we will migrate every node menu link
    // with "node_translation_menu_links" migration.
    $node_translation_menu_link_migrations = array_filter($drupal_7_migrations, function (array $definition) {
      return $definition['id'] === 'node_translation_menu_links';
    });
    if (empty($node_translation_menu_link_migrations)) {
      return;
    }

However, the variable is not empty due to the D7MenuLinkDeriver returning [0 => $base_plugin_definition];, which leads to the assertion failure.

Per #7:

1. ❌ ValidateD6MigrationStateTest fails when 🐛 Existing menu links show validation issues on migration (and ALL menu links pointing to node translations are invalid) Needs work is applied
2. ✅ ValidateD7MigrationStateTest passes when 🐛 Existing menu links show validation issues on migration (and ALL menu links pointing to node translations are invalid) Needs work is applied

It's really only the Drupal 7 one that matters as far as AM:A is concerned. Plus 🐛 Existing menu links show validation issues on migration (and ALL menu links pointing to node translations are invalid) Needs work is crucial; without it the UX of migrating menus falls apart.

So, I'm calling it: relevant tests are passing, so this is ready to go.

Automatically closed - issue fixed for 2 weeks with no activity.