Problem/Motivation

FieldApiDataTest is making no HTTP requests but is a functional test.

Proposed resolution

  1. Convert FieldApiDataTest into a Kernel test.
  2. Polish to make it more readable.
  3. Cleanup ::setUp() and move object creation only in tests that actually use them so that they are not unnecessary created every time.

Remaining tasks

None.

User interface changes

None.

API changes

None.

Data model changes

None.

Release notes snippet

N/A

CommentFileSizeAuthor
#4 3041696-4.interdiff.txt1001 bytesclaudiu.cristea
#4 3041696-4.patch26.31 KBclaudiu.cristea
#2 3041696-2.patch26.32 KBclaudiu.cristea
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

claudiu.cristea created an issue. See original summary.

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
CreditAttribution: claudiu.cristea as a volunteer and at Webikon commented
Status: Active » Needs review
Issue tags: +Testing system, +Test suite performance, +Performance, +tests convert, +functional2kernel
FileSize
3041696-2.patch26.32 KB

Patch.

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
CreditAttribution: claudiu.cristea as a volunteer and at Webikon commented
Parent issue: » #3041700: [META] Convert some tests into Kernel or Unit tests
claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
CreditAttribution: claudiu.cristea as a volunteer and at Webikon commented
FileSize
3041696-4.patch26.31 KB
3041696-4.interdiff.txt1001 bytes

Coding standards.

Lendude’s picture

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

Diff is pretty hard to read. Did a line by line comparison using FileMerge and it checks out.

The big change that throws of diff seems to be the move of the field and node creation out of setup and into testEntityFieldFilter.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented

Crediting lendude, line by line reviews are a lot of work. I applied the patch locally and rebuilt it with -M25% and it was much easier to read as it detected the move, confirming it looks good.

  • larowlan committed 3a200fd on 8.8.x 
    Issue #3041696 by claudiu.cristea, Lendude: Convert FieldApiDataTest...
larowlan’s picture

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

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

c/p to 8.7 as a28722f49b

  • larowlan committed a28722f on 8.7.x 
    Issue #3041696 by claudiu.cristea, Lendude: Convert FieldApiDataTest...

Status: Fixed » Closed (fixed)

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