Problem/Motivation

AccessPermissionTest makes no HTTP requests but is a functional test

Proposed resolution

Convert AccessPermissionTest into a Kernel test

Remaining tasks

None.

User interface changes

None.

API changes

None.

Data model changes

None.

Release notes snippet

N/A

CommentFileSizeAuthor
#6 3042536-6.interdiff.txt977 bytesclaudiu.cristea
#6 3042536-6.patch5.61 KBclaudiu.cristea
#4 3042536-4.interdiff.txt962 bytesclaudiu.cristea
#4 3042536-4.patch5.61 KBclaudiu.cristea
#2 3042536-2.patch5.61 KBclaudiu.cristea

Comments

claudiu.cristea created an issue. See original summary.

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
Status: Active » Needs review
StatusFileSize
new 3042536-2.patch5.61 KB

This patch decreases the locally test run from 19.1 to 3.5 seconds.

lendude’s picture

lendude
Primary language Dutch
Location Amsterdam
commented
Status: Needs review » Needs work

Did a line by line comparison using FileMerge and no coverage is lost in the conversion.

One thing:

+++ b/core/modules/user/tests/src/Kernel/Views/AccessPermissionTest.php
@@ -0,0 +1,110 @@
+    // First access as user without access.
...
+    // Then with access.

These comments are the wrong way around.

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
Status: Needs work » Needs review
StatusFileSize
new 3042536-4.patch5.61 KB
new 3042536-4.interdiff.txt962 bytes

You're right.

Status: Needs review » Needs work

The last submitted patch, 4: 3042536-4.patch, failed testing. View results

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
Status: Needs work » Needs review
StatusFileSize
new 3042536-6.patch5.61 KB
new 3042536-6.interdiff.txt977 bytes

Oh, I forgot to switch the assertions in #4.

lendude’s picture

lendude
Primary language Dutch
Location Amsterdam
commented
Status: Needs review » Reviewed & tested by the community

Looks good to me now, thanks!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Version: 8.8.x-dev » 8.7.x-dev
Status: Reviewed & tested by the community » Fixed

Committed and pushed 41c242957c to 8.8.x and 4c8bdc11b5 to 8.7.x. Thanks!

As a test only change backported to 8.7.x - I discussed backporting test-only changes with @catch (as a release manager).

  • alexpott committed 41c2429 on 8.8.x 
    Issue #3042536 by claudiu.cristea, Lendude: Convert AccessPermissionTest...

  • alexpott committed 4c8bdc1 on 8.7.x 
    Issue #3042536 by claudiu.cristea, Lendude: Convert AccessPermissionTest...

Status: Fixed » Closed (fixed)

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