Problem/Motivation

CommentFieldNameTest makes no HTTP requests but is a functional test

Proposed resolution

Convert CommentFieldNameTest into a Kernel test

Remaining tasks

None.

User interface changes

None.

API changes

None.

Data model changes

None.

Release notes snippet

N/A

CommentFileSizeAuthor
#2 3049604-2.patch5.69 KBclaudiu.cristea

Comments

claudiu.cristea created an issue. See original summary.

claudiu.cristea’s picture

Status: Active » Needs review
StatusFileSize
new5.69 KB

This patch decreases the locally test run from 14.5 to 2.4 seconds.

manuel garcia’s picture

This patch decreases the locally test run from 14.5 to 2.4 seconds.

Awesome!

+++ b/core/modules/comment/tests/src/Kernel/Views/CommentFieldNameTest.php
@@ -23,53 +49,54 @@ class CommentFieldNameTest extends CommentTestBase {
-    // Grant permission to properly check view access on render.
-    user_role_grant_permissions(RoleInterface::ANONYMOUS_ID, ['access comments']);
-    $this->container->get('account_switcher')->switchTo(new AnonymousUserSession());
     $view = Views::getView('test_comment_field_name');
-    $this->executeView($view);
+    $view->preview();

Are we loosing coverage here for view access on render?

claudiu.cristea’s picture

That view has nothing to do with the user context. Neither has contextual filters depending on the logged in user

manuel garcia’s picture

Status: Needs review » Reviewed & tested by the community

Ah ok then, thats the only thing I could think of, and the rest looks great- so RTBCing :)

alexpott’s picture

Version: 8.8.x-dev » 8.7.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

@claudiu.cristea the access thing here feels tricky. If you comment out user_role_grant_permissions(RoleInterface::ANONYMOUS_ID, ['access comments']); in the original test it still passes so you're right that access is not being tested but should it be being tested? The only thing in CommentViewsData I can find is $data['comment_field_data']['table']['base']['access query tag'] = 'comment_access'; and that's not being tested here (or anywhere else for that matter).

Committed 3a1d204 and pushed to 8.8.x. Thanks!

Will backport to 8.7.x later.

  • alexpott committed 3a1d204 on 8.8.x
    Issue #3049604 by claudiu.cristea, Manuel Garcia: Convert...
alexpott’s picture

Status: Patch (to be ported) » Fixed

  • alexpott committed 92ea100 on 8.7.x
    Issue #3049604 by claudiu.cristea, Manuel Garcia: Convert...

Status: Fixed » Closed (fixed)

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