Add automated tests

Closed #3022012: Add automated tests as a duplicate issue.

Tests are green now that #3022011: Implement config schema for block.settings.menu_block.*.{follow, follow_parent} is committed.

Should probably commit #3086171: Add @file doc block to menu_block.module first so that the testbot stops marking patches as failed...

Thanks,
-Derek

Nah, I guess not. ;) https://www.drupal.org/pift-ci-job/1427031 suggests that all patches uploaded to this queue will fail until this patch is committed, since the bot can't find any tests to run. So yeah, let's get this in ASAP. Bumping to major since the current project configuration is causing a bunch of issue churn in here, confusing contributors, etc.

However, looking closely at the patch, I have some concerns, and I think a re-roll is in order before this is truly RTBC...

1. +++ b/tests/src/Functional/MenuBlockTest.php
   @@ -0,0 +1,458 @@
   +    // Then create a simple link hierarchy:
   +    // - parent menu item
   +    //   - child-1 menu item
   +    //     - child-1-1 menu item
   +    //       - child-1-2 menu item
   +    //   - child-2 menu item.
   ...
   +    $child_1_2 = $base_options + [
   +      'title' => 'child-1-2 menu item',
   +      'link' => ['uri' => 'internal:/menu-test/hierarchy/parent/child2/child'],
   +      'parent' => $links['child-1'],
   +    ];
   

   A) Seems like the intention of the code comment is that child-1-2 has child-1-1 as the parent, but that doesn't match the code itself.

   B) The link itself seems like it should match the menu structure, so we don't confuse ourselves. E.g. child-1-2's link should be /menu-test/hierarchy/parent/child-1/child-1-2

2. +++ b/tests/src/Functional/MenuBlockTest.php
   @@ -0,0 +1,458 @@
   +      'link' => ['uri' => 'internal:/menu-test/hierarchy/parent/child'],
   

   Seems the link should be to .../parent/child-1 to be more self-documenting.

3. +++ b/tests/src/Functional/MenuBlockTest.php
   @@ -0,0 +1,458 @@
   +    $child_1_1 = $base_options + [
   +      'title' => 'child-1-1 menu item',
   +      'link' => ['uri' => 'internal:/menu-test/hierarchy/parent/child2/child'],
   +      'parent' => $links['child-1'],
   +    ];
   

   Maybe it doesn't matter, but it's not at all clear why the link for this is /hierarchy/parent/child2/child since it seems (based on the comments) that the link should be /hierarchy/parent/child-1/child-1-1

4. +++ b/tests/src/Functional/MenuBlockTest.php
   @@ -0,0 +1,458 @@
   +    $child_2 = $base_options + [
   +      'title' => 'child-2 menu item',
   +      'link' => ['uri' => 'internal:/menu-test/hierarchy/parent/child'],
   +      'parent' => $links['parent'],
   +    ];
   

   This link seems like it wants to be .../hierachy/parent/child2 (or .../child-2)

5. +++ b/tests/src/Functional/MenuBlockTest.php
   @@ -0,0 +1,458 @@
   +    self::assertSame(2, $block_settings['level']);
   +    self::assertSame(6, $block_settings['depth']);
   +    self::assertTrue($block_settings['expand']);
   +    self::assertSame('main:', $block_settings['parent']);
   +    self::assertTrue($block_settings['follow']);
   +    self::assertSame('active', $block_settings['follow_parent']);
   +    self::assertSame('main', $block_settings['suggestion']);
   

   core seems to use $this->assert* everywhere. Not clear why we're using self:: here.
   Yoda used is not by core much, either. ;)

6. +++ b/tests/src/Functional/MenuBlockTest.php
   @@ -0,0 +1,458 @@
   +    $this->drupalGet('menu-test/hierarchy/parent/child');
   +    $this->assertSession()->pageTextNotContains('parent menu item');
   +    $this->assertSession()->pageTextNotContains('child-1 menu item');
   +    $this->assertSession()->pageTextContains('child-1-1 menu item');
   +    $this->assertSession()->pageTextContains('child-1-2 menu item');
   +    $this->assertSession()->pageTextNotContains('child-2 menu item');
   

   This makes me slightly nervous, based on my concerns above. But since the tests are only ever asserting the text of the menu items, not where they point, it all seems to be working fine.

   But this is further evidence that child-1-1 and child-1-2 are both supposed to be immediate children of child-1, so the code comment should be properly indented, and all the paths should match that to make all this more robust...

Thanks! Having this in the code will definitely help further development.

Cheers,
-Derek

Fantastic, thanks! Almost there. :)

1. +++ b/tests/modules/menu_block_test/menu_block_test.info.yml
   @@ -0,0 +1,6 @@
   +version: VERSION
   

   This is only for core. Contrib modules should never do this.

2. +++ b/tests/modules/menu_block_test/menu_block_test.routing.yml
   @@ -0,0 +1,39 @@
   +    _title: 'Parent page'
   ...
   +    _title: 'Child-1 page'
   ...
   +    _title: 'child-1-1 page'
   ...
   +    _title: 'child-1-2 page'
   ...
   +    _title: 'child-2 page'
   

   For consistency, these should all be the same case. Probably lowercase for everything.

3. +++ b/tests/modules/menu_block_test/src/Controller/MenuBlockTestController.php
   @@ -0,0 +1,26 @@
   +use Drupal\Core\Routing\RouteMatchInterface;
   +use Drupal\Core\Theme\ThemeManagerInterface;
   +use Drupal\Core\Theme\ThemeNegotiatorInterface;
   +use Symfony\Component\DependencyInjection\ContainerInterface;
   

   None of these are needed.

   Oh look, the bot agrees:
   https://www.drupal.org/pift-ci-job/1427675
   ;)

4. +++ b/tests/modules/menu_block_test/src/Controller/MenuBlockTestController.php
   @@ -0,0 +1,26 @@
   +   * Some known placeholder content which can be used for testing.
   

   Core standards say comments like this should start with a verb. E.g. "Returns placeholder page content which can be used for testing." or something...

5. +++ b/tests/src/Functional/MenuBlockTest.php
   @@ -0,0 +1,458 @@
   +   * Create a simple hierarchy of links.
   

   ubernit:
   s/Create/Creates/

6. +++ b/tests/src/Functional/MenuBlockTest.php
   @@ -0,0 +1,458 @@
   +  protected function createLinkHierarchy() {
   

   Ahhh, so much better! Comments, paths, and link names all make sense now.
   Thanks!! :)

7. +++ b/tests/src/Functional/MenuBlockTest.php
   @@ -0,0 +1,458 @@
   +   * Check if all menu block configuration options are available.
   ...
   +   * Check if all menu block settings are saved correctly.
   ...
   +   * Test the menu_block level option.
   ...
   +   * Test the menu_block depth option.
   ...
   +   * Test the menu_block expand option.
   ...
   +   * Test the menu_block parent option.
   ...
   +   * Test the menu_block follow and follow_parent option.
   ...
   +   * Test the menu_block suggestion option.
   

   More ubernits:

   s/Check/Checks/
   s/Test/Tests/

Of these, only #1 and #3 need to be fixed. Everything else is just hyper nitpicking for consistency with core's (sometimes ridiculous) standards.

So as not to be too much of a pain in the ass, I'll just upload a fixed patch in a moment. ;) Stay tuned.

Thanks!
-Derek

This should be RTBC. Everything passes locally.

p.s. Note to committers: d.o now thinks I should be the author of this commit. :( Please no. @JeroenT should be the author. Thanks!

#17 looks good to me. +1 to RTBC

Okay, RTBC it is. :)

Thanks,
-Derek

Thanks @JeroenT and @dww, I've committed this to the dev branch

Yay, thanks!