Problem/Motivation

FileManagedAccessTest makes no HTTP requests but is a functional test

Proposed resolution

Convert FileManagedAccessTest 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 3048434-6.patch4.93 KBclaudiu.cristea
#6 3048434-6.interdiff.txt703 bytesclaudiu.cristea
#3 3048434-3.patch5.01 KBlendude
#2 3048434-2.patch5.64 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 3048434-2.patch5.64 KB

This patch decreases the locally test run from 7.0 to 1.6 seconds.

lendude’s picture

lendude
Primary language Dutch
Location Amsterdam
commented
StatusFileSize
new 3048434-3.patch5.01 KB

Same patch rolled with -M10% to detect the move and make reviewing easier.

lendude’s picture

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

Conversion looks good, I don't see any loss of coverage and nice to see clearer naming of the created test files.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/modules/file/tests/src/Kernel/FileManagedAccessTest.php
@@ -1,83 +1,78 @@
+    // Create the anonymous user grant him with the 'access content' permission.

This should use gender neutral language (and will fail the coder rule just added), but also just doesn't need a comment at all since the code following is self-explanatory.

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
Status: Needs work » Needs review
StatusFileSize
new 3048434-6.interdiff.txt703 bytes
new 3048434-6.patch4.93 KB

Removed the line.

johnwebdev’s picture

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

Feedback has been adressed :)

  • larowlan committed 2adeb97 on 8.8.x 
    Issue #3048434 by claudiu.cristea, Lendude: Convert...
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Version: 8.8.x-dev » 8.7.x-dev
Status: Reviewed & tested by the community » Fixed

Committed 2adeb97 and pushed to 8.8.x. Thanks!

C/p as 13cc9de963 and pushed to 8.7.x

  • larowlan committed 13cc9de on 8.7.x 
    Issue #3048434 by claudiu.cristea, Lendude: Convert...

Status: Fixed » Closed (fixed)

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