Add tests for tracker module's Tools menu link

Just so others know, I'm currently working on a patch for this.

I've attached a patch for this. Question: should there be any tests created to confirm that the menu items are present? I don't think that other menu items in that menu have tests, but I could be mistaken.

Hey,

It does sound like your build is playing up. Works fine for me on the fresh install on which I created the patch. The Tools menu item is labelled "Recent content", in line with the description in the existing help text within the tracker module.

Re. the whitespace fix: as part of the tracker_cron() function, that's unrelated to this bug and the patch: the patch does not change that particular line within the module file. To be pedantic, my opinion would be that the whitespace there should be fixed in its own issue (I've not looked, but maybe there is already an issue for that?) :-)

Bryan

Updated issue summary.

makemineatriple, the whitespace reference by dags was wrong.
Your patch added a whitespace in hook_menu() though.

patch is outdated

Rerolled patch to remove whitespace.

This functionality was missing after port, because there's no test coverage.
A fix should add test coverage as well.

We also have to move to .menu_links.yml files.

I can see the Track link in user profile. So, Is this need to be done or just test case waiting ?

joshi.rohit100 you could check if there is a test by ... for instance file a patch that removes the Track link and see if it fails with the testbots on d.o.

Thank you for reporting this issue and posting a patch.

As part of the Bug Smash Initiative, we are triaging issues that are marked "Needs tests". This issue was marked "Needs tests" back in June 2014. The issue itself is fixed, joshi.rohit100 pointed that out in July 2014. Also in July 2014, miro_dietiker recommended adding tests because the "This functionality was missing after port".

If I get this, this needs a test to confirm that 'recent content' link exists in the tools menu and that the 'Activity' tab appears on the users profile page. Looking at the existing functional tests,TrackerTest and TrackerNodeAccessTest, there are usages of $this->drupalGet('activity'); and assertions on the text of the resulting page. Is that sufficient tests for the activity tab? I didn't see any tests using the 'recent content' link, so that will still need a test.

The bug seems to be fixed.
In core/modules/tracker/tracker.routing.yml

a tab on a user's profile page.

We now have

tracker.user_tab:
  path: '/user/{user}/activity'
  defaults:
    _controller: '\Drupal\tracker\Controller\TrackerController::buildContent'
    _title_callback: '\Drupal\tracker\Controller\TrackerController::getTitle'
  requirements:
    _permission: 'access content'
    _entity_access: 'user.view'
    user: \d+

And we do have tests for this in \Drupal\Tests\tracker\Functional\TrackerTest::testTrackerUser()

here should be a new link in the Tools menu

We have core/modules/tracker/tracker.links.menu.yml

tracker.page:
  title: 'Recent content'
  route_name: tracker.page

We don't have dedicated test for this.

I didn't see any tests using the 'recent content' link, so that will still need a test.

we have \Drupal\Tests\tracker\Functional\TrackerTest::testTrackerHistoryMetadata() for that.

@jibran, thanks for your help here and in #bugsmash. Without that his help this patch would not be here.

Yay! patch looks great. Here are some suggestions:

1. +++ b/core/modules/tracker/tests/src/Functional/TrackerRecentContentLinksTest.php
   @@ -0,0 +1,53 @@
   +  protected function setUp(): void {
   +    parent::setUp();
   +  }
   

   This can be removed.

2. +++ b/core/modules/tracker/tests/src/Functional/TrackerRecentContentLinksTest.php
   @@ -0,0 +1,53 @@
   +    $this->drupalPlaceBlock('system_menu_block:tools');
   

   Let's assert no link before adding this block.

3. +++ b/core/modules/tracker/tests/src/Functional/TrackerRecentContentLinksTest.php
   @@ -0,0 +1,53 @@
   +    $user = $this->drupalCreateUser([]);
   

   [] not required.

Yay, indeed!

1. Ah, silly me. Too focused on having a test outside of migrate actually work that I didn't notice this.
2. Good idea. Added.
3. Didn't know that. Fixed.

I renamde the test file which made an interdiff not helpful at all, so I made a diff instead.

This seems ready just a nit.

+++ b/core/modules/tracker/tests/src/Functional/TrackerRecentContentLinkTest.php
@@ -0,0 +1,47 @@
+    $this->drupalGet('<front>');

We can move this line two lines above because we are trying to assert that frontpage has no link before we place the block.

Oh, that makes sense.

Thanks, looks good now. Updated the title and IS.

Committed and pushed 1ea2763329 to 9.1.x and a832400398 to 9.0.x and d63ddf191e to 8.9.x. Thanks!

I ran the test on 9.1.x before committing - it was green.