JavaScriptComponentValidationTest::testInvalidSlotIdentifiedByConfigSchema fails

I can't reproduce. It runs well for me in 0.x:

╭─    ~/apps/contrib/drupal11/web/modules/contrib/experience_builder    0.x ·························· ✔  17:23:44  ─╮
╰─ g pull --prune --rebase                                                                                                  ─╯
Already up to date.

╭─    ~/apps/contrib/drupal11/web/modules/contrib/experience_builder    0.x ·························· ✔  17:23:49  ─╮
╰─ ddev phpunit tests/src/Kernel/Config/JavaScriptComponentValidationTest.php                                               ─╯
PHPUnit 10.5.46 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.3.19
Configuration: /var/www/html/web/core/phpunit.xml.dist

................................D.D.....................          56 / 56 (100%)

Time: 01:35.943, Memory: 6.00 MB

2 tests triggered 1 PHP deprecation:

1) /var/www/html/web/core/modules/options/src/Plugin/Field/FieldType/ListItemBase.php:514
Implicit conversion from float 3.14 to int loses precision

Triggered by:

* Drupal\Tests\experience_builder\Kernel\Config\JavaScriptComponentValidationTest::testInvalidEnumsAndExamples#Invalid string
  /var/www/html/web/modules/contrib/experience_builder/tests/src/Kernel/Config/JavaScriptComponentValidationTest.php:145

* Drupal\Tests\experience_builder\Kernel\Config\JavaScriptComponentValidationTest::testValidEnumsAndExamples#3
  /var/www/html/web/modules/contrib/experience_builder/tests/src/Kernel/Config/JavaScriptComponentValidationTest.php:121

OK, but there were issues!
Tests: 56, Assertions: 164, Deprecations: 1.

What I see is that I have this specific issue reproducible in 2 different unrelated builds that are not updated with 0.x at this time:

• https://git.drupalcode.org/project/experience_builder/-/jobs/5229666#L885
• https://git.drupalcode.org/project/experience_builder/-/jobs/5229606#L703

99% likely due to https://www.drupal.org/project/drupal/releases/11.1.7 → having been released, and CI just having started using it by default.

Reverting 🐛 SDC slots not being validated against json config schema Active fixes the test.

To me though the strange thing is:

    $this->entity->set('slots', ['test-slot' => []]);

causes the error [slots.test-slot] Array value found, but an object is required but

    $this->entity->set('slots', ['test-slot' => ['title' => 'test']]);

does not - @isholgueras suggests this may be due to json_encode() where an empty array is not converted to an object, but an associative array is.

If that is the case then just expecting the error on >= 11.1.7 seems wrong - instead we should fix this at the point of encoding if we can?

instead we should fix this at the point of encoding if we can?

+1

Did some debugging but we aren't really in control of that, and it's going to be hard to change.

Core's ComponentValidator does this:

    $definition_object = Validator::arrayToObjectRecursive($definition);
    $this->validator->reset();
    $this->validator->validate(
      $definition_object,
      (object) ['$ref' => 'file://' . dirname(__DIR__, 5) . '/assets/schemas/v1/metadata-full.schema.json']
    );

Validator::arrayToObjectRecursive() is from justinrainbow/json-schema. I think we can probably argue that it is doing the right thing:

> \JsonSchema\Validator::arrayToObjectRecursive(['slots' => []]);
= {#8010
    +"slots": [],
  }

> \JsonSchema\Validator::arrayToObjectRecursive(['slots' => ['test' => 'slot']]);
= {#7977
    +"slots": {#8011
      +"test": "slot",
    },
  }

It can't know at this point whether an empty array should stay as an empty array or be cast to an empty object - there are likely just as many cases where an empty array is correct here.

Also, because ::arrayToObjectRecursive() works by encoding and decoding JSON, even if we force an empty slots to an object first, that is lost and it is cast back to an empty array.

So, perhaps the pragmatic thing to do here is just treat this as an edge case in the test with a comment, and move on for now.

We might want to raise this upstream and solve it in core in ComponentValidator, but we can fix this for us in ::toSdcDefinition():

    if ($this->slots) {
      foreach ($this->slots as $slot_name => $slot) {
        // Force empty slots to be an object; ComponentValidator casts non-
        // empty arrays to objects, but empty arrays trigger a false positive
        // validation error: "Array value found, but an object is required".
        if ($slot === []) {
          $slot = new \stdClass();
        }
        $definition['slots'][$slot_name] = $slot;
      }
    }

As soon as there's a core follow-up + our code pointing to it with a // @todo Remove in <core issue URL>, this is IMHO ready to go 👍

Epic digging, @longwave! 👏

I merged this as is because there some issues I think I would like to merge and I don't feel qualified to make the core follow-up. I would rather not merged the other issue with failing phpunit tests.

so setting back to Needs Work and assigning to @longwave to create another MR for the core follow-up link

Good call, @tedbow :) Thanks! 🙏

Opened 🐛 [regression] Empty slots give incorrect validation message Active and linked it from a comment in MR!1026.

Looks good