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.
- 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.
- 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).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- A Drupal Block plugin used as a component instance whose explicit inputs are invalid does NOT make the rest of the component tree unusable.
- An SDC used as a component instance whose explicit inputs are invalid does NOT make the rest of the component tree unusable.
- 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
- Create a
crashSDC in thexb_test_sdcmodule, which accepts atype: boolean"crash" prop which will cause the Twig to trigger a PHP exception. This is for criterium 2. - Create a
crashblock plugin, which accepts atype: boolean"crash" block setting which will cause the Block plugin's::build()method to trigger a PHP exception. This is for criterium 1. - 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.
Issue fork experience_builder-3517966
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
Comment #2
wim leersComment #3
f.mazeikis commentedComment #4
nagwani commentedComment #5
wim leers#3501290: Introduce unit test coverage for both ComponentSource plugins (Block + SDC) is in!
That added
ComponentSourceTestBase::testRenderComponentLive().This issue's summary proposed
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? 🤔Comment #7
f.mazeikis commentedWell, 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
TRUEas value for "crash" prop/input. By default, the value isTRUE. These new component sources are currently part ofxb_test_blockandxb_test_sdctest 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 oftestRenderComponentLive()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?
Comment #8
wim leersComment #9
wim leersIt refers to it using
isPreview: FALSE. I didn't like the name, but needed to unblock this 😅 Suggestions VERY welcome! 🙏That's because
xb_test_sdcis installed by a bunch of tests; we shouldn't default to crashing — that immediately fixes those widespread failures!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.Comment #10
wim leers@f.mazeikis is on PTO.
Comment #11
isholgueras commentedComment #12
larowlanWim 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
Comment #13
larowlanComment #14
wim leersComment #15
wim leersHah, 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.
Comment #17
wim leersComment #18
nagwani commented