Overview

This is blocked by #3501290: Introduce unit test coverage for both ComponentSource plugins (Block + SDC).
This blocks #3485878: Server-rendered component instances should NEVER result in a user-facing error, should fall back to a meaningful error instead (+ log).

Implement explicit kernel tests for 5 of the 6 acceptance criteria at #3517941: [META] Robust component instance error handling during hydration+rendering.

  1. A Drupal Block plugin (whose rendering logic throws a PHP exception) used as a component instance in an XB component tree does NOT make the rest of the component tree unusable.
  2. An SDC (whose Twig logic throws a PHP exception) used as a component instance in an XB component tree does NOT make the rest of the component tree unusable (except that if this SDC has slots, those slots MAY not appear in the preview , but they would appear in the “Layers” view 👈 out of scope to test here).
  3. A code component (whose JS logic throws a JS exception) used as a component instance whose explicit inputs are invalid does NOT make the rest of the component tree unusable (except that if this code component has slots, those slots MAY not appear in the preview, but they would appear in the “Layers” view). 👈 out of scope to test here
  4. A Drupal Block plugin used as a component instance whose explicit inputs are invalid does NOT make the rest of the component tree unusable.
  5. An SDC used as a component instance whose explicit inputs are invalid does NOT make the rest of the component tree unusable.
  6. A code component used as a component instance whose explicit inputs are invalid does NOT make the rest of the component tree unusable.

Proposed resolution

  1. Create a crash SDC in the xb_test_sdc module, which accepts a type: boolean "crash" prop which will cause the Twig to trigger a PHP exception. This is for criterium 2.
  2. Create a crash block plugin, which accepts a type: boolean "crash" block setting which will cause the Block plugin's ::build() method to trigger a PHP exception. This is for criterium 1.
  3. Add the following:
      /**
       * @dataProvider providerRenderComponent
       */
      public function testRenderComponent(array $inputs, string $componentUuid, bool $isPreview = FALSE, ?\Exception $expected_exception, string $expected_output): {
        
      }
    

    to \Drupal\Tests\experience_builder\Kernel\Plugin\ExperienceBuilder\ComponentSource\(Block|Js|SingleDirectory)ComponentTest.

    Make it test all 5 scenarios in scope for this issue, plus one successfully rendering one.

User interface changes

None.

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:

Comments

wim leers created an issue. See original summary.

wim leers’s picture

Issue summary: View changes
f.mazeikis’s picture

Assigned: Unassigned » f.mazeikis
nagwani’s picture

Issue tags: +sprint
wim leers’s picture

Title: [PP-1] Failing kernel tests for all ways a component source can fail to render on the server side » Failing kernel tests for all ways a component source can fail to render on the server side

#3501290: Introduce unit test coverage for both ComponentSource plugins (Block + SDC) is in!

That added ComponentSourceTestBase::testRenderComponentLive().

This issue's summary proposed

  /**
   * @dataProvider providerRenderComponent
   */
  public function testRenderComponent(array $inputs, string $componentUuid, bool $isPreview = FALSE, ?\Exception $expected_exception, string $expected_output): {
    
  }

That signature was not viable in #3501290, because that issue aimed to test all discovered components of the tested ComponentSource.

Do you have a proposal, @f.mazeikis? Perhaps what we want is a testRenderComponentFailure() instead with that signature? 🤔

f.mazeikis’s picture

Well, I've used the signature you've suggested as a suggestion and made some changes. I also renamed the method to testRenderComponentFailure().

I have added SDC and Block Plugin that fails by default by intentionally throwing exceptions when passed TRUE as value for "crash" prop/input. By default, the value is TRUE. These new component sources are currently part of xb_test_block and xb_test_sdc test modules.

At the moment this causes failures of Functional tests for a bunch of endpoints and the recently introduced testRenderComponentLive() in SDC and Block tests. By the way, what does "Live" part of testRenderComponentLive() refers to? Don't think I've seen this term used before in XB Components context.

I am not sure if that was what @wim-leers expected? I could move them into some sort of "xb_test_failures" submodules and somewhat "limit the damage". Or is the intent is to go the other way around and make even more breakage, so we can spot all the potential issues?

wim leers’s picture

Assigned: f.mazeikis » wim leers
wim leers’s picture

Assigned: wim leers » f.mazeikis
Status: Active » Needs work

By the way, what does "Live" part of testRenderComponentLive() refers to? Don't think I've seen this term used before in XB Components context.

It refers to it using isPreview: FALSE. I didn't like the name, but needed to unblock this 😅 Suggestions VERY welcome! 🙏


At the moment this causes failures of Functional tests for a bunch of endpoints a

That's because xb_test_sdc is installed by a bunch of tests; we shouldn't default to crashing — that immediately fixes those widespread failures!


I am not sure if that was what @wim-leers expected?

This is definitely looking like the direction I asked for!

But this was not yet proving used as a component instance in an XB component tree does NOT make the rest of the component tree unusable. See review feedback, and partial implementation.

wim leers’s picture

Assigned: f.mazeikis » Unassigned

@f.mazeikis is on PTO.

isholgueras’s picture

Assigned: Unassigned » isholgueras
larowlan’s picture

Assigned: isholgueras » larowlan

Wim asked me to pick this up today. @isholgueras is offline in slack so hoping that's OK.. will push and unassign at the end of my day

larowlan’s picture

Assigned: larowlan » Unassigned
Status: Needs work » Needs review
wim leers’s picture

Status: Needs review » Reviewed & tested by the community
wim leers’s picture

Hah, this conflicted with #3518838: ComponentSource robustness: add `ComponentSourceTestBase::testSettings()` having introduced more test expectations, which this MR didn't yet add for the new test block + SDC.

  • wim leers committed 440b726c on 0.x authored by f.mazeikis
    Issue #3517966 by f.mazeikis, wim leers, larowlan, isholgueras: Failing...
wim leers’s picture

Status: Reviewed & tested by the community » Fixed
nagwani’s picture

Issue tags: -sprint

Status: Fixed » Closed (fixed)

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