Problem/Motivation

A bunch node access tests are functional tests but they don't make any HTTP requests.

Proposed resolution

Convert them to kernel tests using a new NodeAccessTestBase class that these tests can use.

Remaining tasks

None.

User interface changes

None.

API changes

None.

Data model changes

None.

Release notes snippet

N/A

CommentFileSizeAuthor
#2 3041863.patch14.39 KBamateescu
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

amateescu created an issue. See original summary.

amateescu’s picture

amateescu CreditAttribution: amateescu commented
Status: Active » Needs review
FileSize
3041863.patch14.39 KB

Here we go.

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
CreditAttribution: claudiu.cristea as a volunteer and at Webikon commented 
+++ b/core/modules/node/tests/src/Kernel/NodeAccessTestBase.php
@@ -62,8 +60,9 @@ protected function setUp() {
-    $this->accessHandler = $this->container->get('entity_type.manager')
-      ->getAccessControlHandler('node');
+
+    $this->accessHandler = \Drupal::entityTypeManager()->getAccessControlHandler('node');
+

It seems that, for Kernel tests, `$this->container` is safe. See @alexpott's comment #2066993-57: Use \Drupal consistently in tests ad this discussion https://drupal.slack.com/archives/C223PR743/p1552894588175000

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude at ezCompany commented
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

That diff is pretty hard to read, it's a lot of red and not that much green, at first glance it looks like there are all sorts of thing getting lost. I applied the patch and analysed it manually and I don't see anything getting lost, so that is great! Not sure why the diff looks like it does.

Updated the IS to inform readers about the new TestBase class.

Don't think #3 is blocking since we have both still in core?

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Fixed

Committed 084b1e4 and pushed to 8.8.x. Thanks!

  • catch committed 084b1e4 on 8.8.x 
    Issue #3041863 by amateescu, claudiu.cristea, Lendude: Convert a few...

Status: Fixed » Closed (fixed)

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