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.
CommentFileSizeAuthor
#22 test-comment-link-2555899-22.patch9.96 KBgeertvd
#22 interdiff-2555899-16-22.txt1.42 KBgeertvd
#17 test-comment-link-2555899-16.patch9.94 KBgeertvd
#17 interdiff-2555899-14-16.txt679 bytesgeertvd
#14 test-comment-link-2555899-14.patch9.92 KBgeertvd
#14 interdiff-2555899-12-14.txt1.39 KBgeertvd
#12 test-comment-link-2555899-12.patch9.94 KBgeertvd
#12 interdiff-2555899-11-12.txt942 bytesgeertvd
#11 test-comment-link-2555899-11.patch9.94 KBgeertvd
#11 interdiff-2555899-6-11.txt14.26 KBgeertvd
#6 test-linkapprove-2555899-6.patch4.31 KBgeertvd
#6 interdiff-2555899-2-6.txt877 bytesgeertvd
#2 test-linkapprove-2555899-2.patch4.46 KBgeertvd

Comments

geertvd created an issue. See original summary.

geertvd’s picture

geertvd commented
Status: Active » Needs review
StatusFileSize
new test-linkapprove-2555899-2.patch4.46 KB

Status: Needs review » Needs work

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

Status: Needs work » Needs review

geertvd queued 2: test-linkapprove-2555899-2.patch for re-testing.

Status: Needs review » Needs work

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

geertvd’s picture

geertvd commented
Status: Needs work » Needs review
StatusFileSize
new interdiff-2555899-2-6.txt877 bytes
new test-linkapprove-2555899-6.patch4.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

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

Now i feel stupid :)

geertvd queued 6: test-linkapprove-2555899-6.patch for re-testing.

geertvd’s picture

geertvd commented
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

geertvd commented
Status: Needs work » Needs review
StatusFileSize
new interdiff-2555899-6-11.txt14.26 KB
new test-comment-link-2555899-11.patch9.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 commented
StatusFileSize
new interdiff-2555899-11-12.txt942 bytes
new test-comment-link-2555899-12.patch9.94 KB

nitpicks

geertvd’s picture

geertvd commented
Assigned: geertvd » Unassigned
geertvd’s picture

geertvd commented
StatusFileSize
new interdiff-2555899-12-14.txt1.39 KB
new test-comment-link-2555899-14.patch9.92 KB

more nitpicks

dawehner’s picture

dawehner
Primary language German
commented
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

geertvd commented
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

geertvd commented
StatusFileSize
new interdiff-2555899-14-16.txt679 bytes
new test-comment-link-2555899-16.patch9.94 KB

And the patch :)

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 queued 17: test-comment-link-2555899-16.patch for re-testing.

geertvd’s picture

geertvd commented
Status: Needs review » Reviewed & tested by the community

Seems like a random fail, setting back to RTBC

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
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

geertvd commented
Status: Needs work » Needs review
StatusFileSize
new interdiff-2555899-16-22.txt1.42 KB
new test-comment-link-2555899-22.patch9.96 KB

Fair enough

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Reviewed & tested by the community

Good point alex

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
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.