Problem/Motivation

Drupal\comment\Plugin\views\field\LinkApprove has no test coverage at the moment

Proposed resolution

Add test coverage

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Task because we are just adding tests
Issue priority Normal because we are adding tests for something small that works but just didn't have test coverage.
Unfrozen changes Unfrozen because it only adds tests.
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

geertvd created an issue. See original summary.

geertvd’s picture

Status: Active » Needs review
FileSize
4.46 KB

Status: Needs review » Needs work

The last submitted patch, 2: test-linkapprove-2555899-2.patch, failed testing.

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 2: test-linkapprove-2555899-2.patch, failed testing.

geertvd’s picture

Status: Needs work » Needs review
FileSize
877 bytes
4.31 KB

Not really sure why this is not working, no issues locally.

Status: Needs review » Needs work

The last submitted patch, 6: test-linkapprove-2555899-6.patch, failed testing.

geertvd’s picture

Version: 8.1.x-dev » 8.0.x-dev
Status: Needs work » Needs review

Now i feel stupid :)

geertvd’s picture

Title: Add test coverage for Drupal\comment\Plugin\views\field\LinkApprove handler » Add test coverage for Comment link handlers handler
Assigned: Unassigned » geertvd
Status: Needs review » Needs work

I'm going to include the linkReply handler in this test also.

geertvd’s picture

Status: Needs work » Needs review
FileSize
14.26 KB
9.94 KB

Ok, I think this is better.

I added test coverage for Drupal\comment\Plugin\views\field\LinkReply.
I also added a CommentKernelTestBase class since there are some other comment handlers that are still untested, those could extend from CommentKernelTestBase.
Ideally CommentUserNameTest should also extend CommentKernelTestBase, this can be picked up in a follow-up.

geertvd’s picture

geertvd’s picture

Assigned: geertvd » Unassigned
geertvd’s picture

dawehner’s picture

Status: Needs review » Reviewed & tested by the community
  1. +++ b/core/modules/comment/src/Tests/Views/CommentKernelTestBase.php
    @@ -0,0 +1,83 @@
    + */
    +abstract class CommentKernelTestBase extends ViewKernelTestBase {
    +
    

    I'm curious whether you want to try to port over the ViewKernelTestBase to KTBTNG, so for new tests we could use it. This does not belong into this issue, this is for sure.

  2. +++ b/core/modules/comment/src/Tests/Views/CommentKernelTestBase.php
    @@ -0,0 +1,83 @@
    +   *
    +   * @var \Drupal\user\UserStorage
    +   */
    

    Nitpick: Let's use \Drupal\user\UserStorageInterface

geertvd’s picture

Issue summary: View changes

Fixed those last nitpicks in #15, keeping this RTBC since I'm just changing some comments.

15.1: I could look into that, I think we should wait for #2553533: KernelTestBaseTNG™ is not cleaning up after itself to get in though, created a follow-up for that: #2556855: Port ViewKernelTestBase to extend from KernelTestBaseTNG™

Also created a follow-up for #11: #2556863: Make CommentUserNameTest extend from CommentViewKernelTestBase

Added beta evaluation.

geertvd’s picture

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 17: test-comment-link-2555899-16.patch, failed testing.

Status: Needs work » Needs review
geertvd’s picture

Status: Needs review » Reviewed & tested by the community

Seems like a random fail, setting back to RTBC

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/modules/comment/src/Tests/Views/CommentKernelTestBase.php
@@ -0,0 +1,83 @@
+abstract class CommentKernelTestBase extends ViewKernelTestBase {

ConmmentKernelTestBase feels like the wrong name since this about testing comment and views functionality - perhaps CommentViewKernelTestBase?

geertvd’s picture

Status: Needs work » Needs review
FileSize
1.42 KB
9.96 KB

Fair enough

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Good point alex

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Thank you. Nice - more test coverage of views. Committed d551f8c and pushed to 8.0.x. Thanks!

  • alexpott committed d551f8c on 8.0.x
    Issue #2555899 by geertvd: Add test coverage for Comment link handlers...

Status: Fixed » Closed (fixed)

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