- πΊπ¦Ukraine abramm Lutsk
The patch in #34 doesn't apply to 10.3, so re-rolled it against both 10.3.x (for those of us who need this patch to be applied to their sites) and 11.x branches.
Also, removed the unrelated change and split patches to test and test+fix. - πΊπ¦Ukraine abramm Lutsk
abramm β changed the visibility of the branch 11.x to hidden.
- πΊπ¦Ukraine abramm Lutsk
abramm β changed the visibility of the branch 9.2.x to hidden.
- Merge request !8582Issue #3202631: Add test for Textarea option to normalize newlines to \n β (Open) created by abramm
- Merge request !8583Issue #3202631: Add Textarea option to normalize newlines to \n β (Open) created by abramm
- πΊπ¦Ukraine abramm Lutsk
Added two MRs (test only and test + code change), let's see if tests still passes.
- πΊπ¦Ukraine abramm Lutsk
abramm β changed the visibility of the branch 3202631-add-textarea-option to hidden.
- Status changed to RTBC
7 months ago 12:11pm 29 June 2024 - Status changed to Needs work
7 months ago 12:48pm 19 July 2024 - π¬π§United Kingdom catch
This seems good but still needs a change record.
- First commit to issue fork.
- πΊπ¦Ukraine Taran2L Lviv
taran2l β changed the visibility of the branch 3202631-11.x-test-only to hidden.
- Status changed to Needs review
5 months ago 4:53pm 4 September 2024 - πΊπ¦Ukraine Taran2L Lviv
Small code update + added a change record. Please review
- Status changed to Needs work
5 months ago 6:25pm 4 September 2024 - π«π·France andypost
One test still fails
---- Drupal\Tests\Core\Render\Element\TextareaTest ---- Status Group Filename Line Function -------------------------------------------------------------------------------- [31mFail Other phpunit-367.xml 0 Drupal\Tests\Core\Render\Element\Te [0m PHPUnit Test failed to complete; Error: PHPUnit 10.5.30 by Sebastian Bergmann and contributors. Runtime: PHP 8.3.11 Configuration: /builds/issue/drupal-3202631/core/phpunit.xml.dist ..WWW.. 7 / 7 (100%) Time: 00:00.038, Memory: 8.00 MB 3 tests triggered 1 PHP warning: 1) /builds/issue/drupal-3202631/core/lib/Drupal/Core/Render/Element/Textarea.php:66 Undefined array key "#normalize_newlines" Triggered by: * Drupal\Tests\Core\Render\Element\TextareaTest::testValueCallback#2 /builds/issue/drupal-3202631/core/tests/Drupal/Tests/Core/Render/Element/TextareaTest.php:22 * Drupal\Tests\Core\Render\Element\TextareaTest::testValueCallback#3 /builds/issue/drupal-3202631/core/tests/Drupal/Tests/Core/Render/Element/TextareaTest.php:22 * Drupal\Tests\Core\Render\Element\TextareaTest::testValueCallback#4 /builds/issue/drupal-3202631/core/tests/Drupal/Tests/Core/Render/Element/TextareaTest.php:22 OK, but there were issues! Tests: 7, Assertions: 7, Warnings: 1.
The test was working earlier. It should be
public function testNormalizeNewlines() { $form_state = $this->prophesize(FormStateInterface::class)->reveal(); $element = ['#normalize_newlines' => TRUE]; $input = "some\r\ndifferent\rline\nendings"; $expected = "some\ndifferent\nline\nendings"; $this->assertSame($expected, Textarea::valueCallback($element, $input, $form_state)); }
- Status changed to Needs review
5 months ago 1:01pm 5 September 2024 - πΊπ¦Ukraine Taran2L Lviv
It's green now.
I've refactored test a bit to be more robust and support more cases.
- Status changed to RTBC
5 months ago 1:37pm 10 September 2024 - πΊπΈUnited States smustgrave
1) Drupal\Tests\Core\Render\Element\TextareaTest::testValueCallback with data set #5 ('some\ndifferent\nline\nendings', 'some\r\ndifferent\rline\nendings') Failed asserting that two strings are identical. --- Expected +++ Actual @@ @@ #Warning: Strings contain different line endings! -'some -different -line +'some line endings'
Ran test-only feature to see coverage
Summary appears complete
Code change itself seems pretty straight forwardLGTM!
- First commit to issue fork.
- π³πΏNew Zealand quietone
There are no unanswered questions here and is does look straight forward as said in the previous comment.
Unfortunately, one of the comments was not wrapped correctly. I updated that one and made a change to another.
I then reviewed the test and found that most of the defaults for the element did not need to be set. That lead me to rework the test and the data provider. The data provider now sets the element configuration as needed and the structure is changed as well.
This now needs another review.
- π³πΏNew Zealand quietone
Coming back to check the change record. I see no evidence here that anyone has reviewed the change record. I took a brief look and was overwhelmed by the second paragraph with a long sentence that is hard to parse. Remember, the readers of change records will have varying levels of skills with the English language.
Setting to needs work for a review of the change record.