Exception when deleting a translation when there is no canonical link template

Back to work for the missing tests.

1. +++ b/core/modules/path/src/Tests/PathNoCanonicalLinkTest.php
   @@ -0,0 +1,74 @@
   +
   +namespace Drupal\path\Tests;
   +use Drupal\content_translation_test\Entity\EntityTestTranslatableUISkip;
   +use Drupal\language\Entity\ConfigurableLanguage;
   

   missing empty line between namespace and use.

2. +++ b/core/modules/path/src/Tests/PathNoCanonicalLinkTest.php
   @@ -0,0 +1,74 @@
   +    // Create test user and login.
   +    $admin_user = $this->drupalCreateUser(array(
   +      'translate any entity',
   +      'administer content translation',
   +      'administer languages',
   +      'administer content types',
   +    ));
   +    $this->drupalLogin($admin_user);
   ...
   +    // Visit the content translation.
   +    $this->drupalGet('admin/config/regional/content-language');
   +
   +    $this->assertText('Test entity - Translatable skip UI check');
   +    $this->assertText('Test entity - Translatable check UI (Translation is not supported)');
   +
   +    $edit = array(
   +      'entity_types[entity_test_translatable_UI_skip]' => 'entity_test_translatable_UI_skip',
   +      'settings[entity_test_translatable_UI_skip][entity_test_translatable_UI_skip][translatable]' => TRUE,
   +      'settings[entity_test_translatable_UI_skip][entity_test_translatable_UI_skip][settings][language][language_alterable]' => TRUE,
   +    );
   +    $this->drupalPostForm(NULL, $edit, t('Save configuration'));
   +    $this->drupalGet('admin/config/regional/content-language');
   

   I don't think we need any of this, this all doesn't matter for an API test.

   We should even be able to make this a fast kernel test.

Also, please provide a failing test-only patch.

Looks good! Tried switching to a Kernel test, but that doesn't seem so easy (added the language module, added the user module, now it complains about a missing table [entity_test_mul, I also tried added entity_test to the list of modules and installing the schema for it in the setUp, nothing worked]).

Looks good, but the test shouldn't be named KernelTest if it isn't one ;)

Also, I've recently found: #2539634: PathItem::delete() never runs because the path field type is a computed field in disguise

If I'm not mistaken then we can implement that issue in a way that will fix the problem here, you might want to consider working on that so we know if we can really replace that hook completely.

Looks good to go.

Mind the Berdir statement at #8 and check #2539634: PathItem::delete() never runs because the path field type is a computed field in disguise

I'm postponing this for the other... Please explain the resulting situation if you need to reopen it, or close it as a duplicate if it solves the problem.

Yes, but turns out that what I proposed in that other issue isn't actually how it works for field items at the moment, they are *not* called when a translation is removed. Which very likely is a bug but possibly not easy to fix.

So, let's get this in and then we have the test coverage to make sure this works, even if we end up removing that hook.

Looks like #2552687: Test failures in ConfigFormOverrideTest and ContainerRebuildWebTest on newly spun up testbot instances.

1. +++ b/core/modules/path/src/Tests/PathNoCanonicalLinkTest.php
   @@ -0,0 +1,52 @@
   +class PathNoCanonicalLinkTest extends PathTestBase {
   

   There is no need for this to be a WebTestBase test. It could be KernelTestBase afaics.

2. +++ b/core/modules/path/src/Tests/PathNoCanonicalLinkTest.php
   @@ -0,0 +1,52 @@
   +    $entity_type->addTranslation('de', ['name' => 'name italian']);
   

   name italian for a de language? A bit confusing.

Passing tests, tested manually and works. Code seems fine as well setting to RTBC.

+++ b/core/modules/path/src/Tests/PathNoCanonicalLinkTest.php
@@ -15,19 +16,37 @@
+    // In order to reflect the changes for a multilingual site in the container
+    // we have to rebuild it.
+    ConfigurableLanguage::create(['id' => 'de'])->save();

That comment doesn't make sense to me, what are we rebuilding? We just add a language.

Also, the negotation stuff below very likely isn't needed here.

+++ b/core/modules/path/src/Tests/PathNoCanonicalLinkTest.php
@@ -0,0 +1,64 @@
+ */
+class PathNoCanonicalLinkTest extends KernelTestBase {
+

Note: You could also write a phpunit based kerneltestbase, actually everything should work the same

Ok, looks good to me now.

+++ b/core/modules/path/tests/src/Kernel/PathNoCanonicalLinkTest.php
@@ -0,0 +1,64 @@
+ * Tests the Path when no canonical link template.

'when there is no'?

Apart from that nitpick looks good.

Also bumping to major.

Setting to RTBC since the fix cleanly implements what catch asked.

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