Problem/Motivation

We need to test PropTypeInterface::normalize() function

We already have a test for LinksPropType : LinksPropTypeNormalizationTest

But this is not enough

Proposed resolution

At least, add similar tests for the 2 other prop types using the mehtod:

  • SlotPropType
  • AttributesPropType

If possible, fix an issue in LinksPropTypeNormalizationTest where 2 tests where disabled (commented) because they need the service container:

      "Standardized structure, flat, only primitives" => self::standardizedFlatPrimitives(),
      // "Standardized structure, flat, with objects" => self::standardizedFlatObjects(),
      "Breadcrumb structure, as generated by the core service" => self::breadcrumb(),
      "Mini pager, as generated by the Views module" => self::viewsMiniPager(),
      "Pager's pages, as generated by the Views module" => self::pagerPages(),
      "Pager's navigation links, as generated by the Views module" => self::pagesNavigationLinks(),
      // "Menu, as generated by the Menu module" => self::menu(),
      "Where link_attributes is already manually set" => self::linkAttributes(),

Do you know how to use teh service container in such tests?

Issue fork ui_patterns-3469665

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

pdureau created an issue. See original summary.

oldeb’s picture

oldeb
Primary language French
Location Namur 🇧🇪
commented
Assigned: Unassigned » oldeb
oldeb’s picture

oldeb
Primary language French
Location Namur 🇧🇪
commented

If possible, fix an issue in LinksPropTypeNormalizationTest where 2 tests where disabled (commented) because they need the service container:

If you need the service container then you need a KernelTest instead of a UnitTest.

Should I change the whole test class into a KernelTest or should I split the it into 2 tests (1 unit test and one kernel test) ?

pdureau’s picture

pdureau commented
Status: Active » Needs work

Should I change the whole test class into a KernelTest or should I split the it into 2 tests (1 unit test and one kernel test) ?

Single KernelTest. Maybe all 3 tests can become kernel tests (to keep them in the same folder)

oldeb’s picture

oldeb
Primary language French
Location Namur 🇧🇪
commented

The issue in fact is that the container is called from a static method which is not aware of the container.

What I think we should do :

  • move the fixutres data (current static functions) to yaml files
  • avoid calling the Url class in the fixtures data (if possible)
  • call the Url class dynamically in the test method (if required)
  • mock the pathValidator service
pdureau’s picture

pdureau commented

Careful, i did some changes in https://git.drupalcode.org/project/ui_patterns/-/blob/a1cba969c36e3891ab...

https://git.drupalcode.org/project/ui_patterns/-/merge_requests/188/diffs

oldeb’s picture

oldeb
Primary language French
Location Namur 🇧🇪
commented
Title: [2.0.0-beta2] Test PropTypeInterface::normalize() » [2.0.0-beta3] Test PropTypeInterface::normalize()
pdureau’s picture

pdureau commented
Title: [2.0.0-beta3] Test PropTypeInterface::normalize() » [2.0.0-beta4] Test PropTypeInterface::normalize()
just_like_good_vibes’s picture

just_like_good_vibes
Location PARIS
commented
Title: [2.0.0-beta4] Test PropTypeInterface::normalize() » [2.0.0-beta5] Test PropTypeInterface::normalize()
pdureau’s picture

pdureau commented
Status: Needs work » Postponed

Postponed because I am doing that first #3474822: [2.0.0-beta4] Normalize attributes values. and introduce the unit tests for URL, Attributes, and many other prop types.

So, this ticket will keep the following tasks

  • Slot tests
  • Move stuff to kernel tests and/or mock services

pdureau’s picture

pdureau commented
Status: Postponed » Needs work

There are 3 unit tests:

  • AttributesPropTypeNormalizationTest.php
  • LinksPropTypeNormalizationTest.php
  • UrlPropTypeNormalizationTest.php

However, they don't cover everything because of limitation of Unit tests. For example:

  • AttributesPropTypeNormalizationTest is avoiding testing the render service.
  • LinksPropTypeNormalizationTest is avoiding testing TranslatableMarkup & URL

Also, we need tests for SlotPropType::normalize()

pdureau’s picture

pdureau commented
Component: Code » Tests
pdureau’s picture

pdureau commented
Title: [2.0.0-beta5] Test PropTypeInterface::normalize() » [2.0.0-rc1] Test PropTypeInterface::normalize()
oldeb’s picture

oldeb
Primary language French
Location Namur 🇧🇪
commented
Assigned: oldeb » Unassigned
pdureau’s picture

pdureau commented
Assigned: Unassigned » just_like_good_vibes

Mikael, because you have added tests/src/Kernel/PropTypesNormalizationTest.php in beta6

Do we still need?

  • tests/src/Unit/PropTypeNormalization/AttributesPropTypeNormalizationTest.php
  • tests/src/Unit/PropTypeNormalization/LinksPropTypeNormalizationTest.php
  • tests/src/Unit/PropTypeNormalization/UrlPropTypeNormalizationTest.php

Can we consolidate those in a single collection of kernel tests?

Mikael's answer:

we can consolidate those tests yes and refactor. better to keep as kernel, and continue what i have introduced if possible. the file EnumNormalizationTest has been deleted, or should have been.

Can we have a tests/src/Kernel/PropTypeNormalization/ folder, with one file per prop type? a bit like what have been done in tests/src/Unit/PropTypeNormalization/ ?

just_like_good_vibes changed the visibility of the branch 3469665-test-proptypeinterfacenormalize to hidden.

just_like_good_vibes opened merge request !299

just_like_good_vibes’s picture

just_like_good_vibes
Location PARIS
commented
Assigned: just_like_good_vibes » pdureau
Status: Needs work » Needs review

did some massive work over there, please review :)

i think we should improve two prop types : ListPropType and NumberPropType.
A the moment, their normalization is not strong enough imho.

let's tackle this in separate tickets.

pdureau’s picture

pdureau commented
Assigned: pdureau » just_like_good_vibes
Status: Needs review » Reviewed & tested by the community

OK for me to be merged.

However, some tests are still commented:

     // "Standardized structure, flat, with objects" =>
      // self::standardizedFlatObjects(),
     ...
      // "Menu, as generated by the Menu module" => self::menu(),

https://git.drupalcode.org/project/ui_patterns/-/blob/cb853ea951afe89b24...

It would be great to uncomment before merging

just_like_good_vibes’s picture

just_like_good_vibes
Location PARIS
commented

ok i added them :)

pdureau’s picture

pdureau commented
Assigned: just_like_good_vibes » Unassigned
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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