Closed (fixed)
Project:
Drupal core
Version:
10.3.x-dev
Component:
phpunit
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
11 Mar 2024 at 19:15 UTC
Updated:
1 May 2024 at 14:09 UTC
Jump to comment: Most recent
Comments
Comment #5
quietone commentedComment #6
smustgrave commentedChecked ConfigTest and this was the only instance of expectError().
Comment #7
mondrakeThis is not enough for PHPUnit 10, since PHPUnit's error handler is no longer converting the
trigger_errorinto an exception.Comment #9
longwaveNestedArray::setValue()already has some support for this but allowed the error to happen; we can catch it before it happens and throw an exception instead.Comment #10
mondrakeLooks good now, thanks
Comment #11
alexpottGiven this is a component I think we should update \Drupal\Tests\Component\Utility\NestedArrayTest to test for this too.
Comment #12
longwaveAdded test coverage to the component, also uncovered an existing bug where $force was not required if you were replacing a scalar directly with an array - this now triggers a LogicException without $force, which I think is the correct behaviour.
Comment #13
smustgrave commentedAppears to have caused large number of test failures.
Comment #14
quietone commentedThis is changing the behavior of NestedArray so I think another solution is needed. And it also prevents installing Drupal thus the many failed tests.
Comment #15
longwaveYeah maybe the behaviour is intended although it seemed wrong when I wrote the test, I will just write another test instead and not change the behaviour I think...
Comment #16
longwaveReverted behaviour change, fixed the test. I guess it's by design that you can overwrite a scalar with an array, just you can't try to make further children under the scalar that previously didn't exist.
Comment #17
smustgrave commentedFeedback and tests appear to be added.
Comment #18
alexpottComment #19
alexpottComment #20
alexpottCommitted 1c017c0 and pushed to 11.x. Thanks!
Committed e8ac11d and pushed to 10.3.x. Thanks!
Comment #22
longwave11.x commit didn't make it to the repo.
Comment #23
alexpott