Skip to content
Snippets Groups Projects

Resolve #3019582 "Data save col long"

Open Resolve #3019582 "Data save col long"
issue/drupal-3019582:3019582-data-save-col-long into 11.x
6 unresolved threads
Open Pooja Sharma requested to merge issue/drupal-3019582:3019582-data-save-col-long into 11.x
6 unresolved threads

Merge request reports

Members who can merge are allowed to add commits.

Merge request pipeline #251231 passed

Stage: 🪄 Lint
Stage: 🗜️ Test
Pipeline: drupal-3019582

#251232

    Merge request pipeline passed for 2690ba02

    Code Quality is loading
    Test summary results are being parsed
    Full report
    Ready to merge by members who can write to the target branch.
    Loading

    Activity

    Filter activity
    • Approvals
    • Assignees & reviewers
    • Comments (from bots)
    • Comments (from users)
    • Commits & branches
    • Edits
    • Labels
    • Lock status
    • Mentions
    • Merge request status
    • Tracking
    core/tests/Drupal/Tests/Core/Test/JUnitConverterTest.php
    108 <testcase name="testGetTestClasses" class="Drupal\Tests\simpletest\Unit\TestDiscoveryTest" classname="Drupal.Tests.simpletest.Unit.TestDiscoveryTest" file="/Users/paul/projects/drupal/core/modules/simpletest/tests/src/Unit/TestDiscoveryTest.php" line="108" assertions="2" time="0.100787"/>
    109 EOD,
    110 [
    111 'test_id' => 23,
    112 'test_class' => 'Drupal\Tests\simpletest\Unit\TestDiscoveryTest',
    113 'status' => 'pass',
    114 'message' => '',
    115 'message_group' => 'Other',
    116 'function' => 'Drupal\Tests\simpletest\Unit\TestDiscoveryTest->testGetTestClasses()',
    117 'line' => 108,
    118 'file' => '/Users/paul/projects/drupal/core/modules/simpletest/tests/src/Unit/TestDiscoveryTest.php',
    119 ],
    120 ],
    121 [
    122 <<<EOD
    123 <testcase name="{$long_function_name}" class="Drupal\Tests\big_pipe\Unit\Render\BigPipeResponseAttachmentsProcessorTest" classname="Drupal.Tests.big_pipe.Unit.Render.BigPipeResponseAttachmentsProcessorTest" file="/Users/paul/projects/drupal/core/modules/big_pipe/tests/src/Unit/Render/BigPipeResponseAttachmentsProcessorTest.php" line="83" assertions="4" time="0.100787"/>
    • C
      catch @catch
      Maintainer

      Does this have to use the big_pipe namespace or could it be something generic?

    • Pooja Sharma
      Pooja Sharma @pooja_sharma
      Author

      Issue is replicating for big_pipe as mentioned in IS , hence big_pipe namespace has been used here.

      Edited by Pooja Sharma
    • quietone
      quietone @quietone
      Maintainer

      I have been thinking about this and coming back now I do think this should be generic. One a first read someone may think this problem is unique to big_pipe. So let's avoid any possible confusion in the future. So, 'big_pipe' could be 'example_module' and the class could be something like "Drupal\Tests\example\FunctionalJavascript\Plugin\views\Handler\CustomFeatureTest.

    • Please register or sign in to reply
  • quietone
    quietone @quietone started a thread on an old version of the diff
    • core/tests/Drupal/Tests/Core/Test/JUnitConverterTest.php
    91 $simpletest = [
    92 'test_id' => 23,
    93 'test_class' => 'Drupal\Tests\simpletest\Unit\TestDiscoveryTest',
    94 'status' => 'pass',
    95 'message' => '',
    96 'message_group' => 'Other',
    97 'function' => 'Drupal\Tests\simpletest\Unit\TestDiscoveryTest->testGetTestClasses()',
    98 'line' => 108,
    99 'file' => '/Users/paul/projects/drupal/core/modules/simpletest/tests/src/Unit/TestDiscoveryTest.php',
    95 public function testConvertTestCaseToSimpletestRow(string $junitXmlString, array $expectedSimpletestRow): void {
    96 $this->assertEquals($expectedSimpletestRow, JUnitConverter::convertTestCaseToSimpletestRow($expectedSimpletestRow['test_id'], new \SimpleXMLElement($junitXmlString)));
    97 $this->assertLessThanOrEqual(255, strlen($expectedSimpletestRow['function']), 'Function value is less than or equal to 255');
    98 }
    99
    100 /**
    101 * See testConvertTestCaseToSimpletestRow method for test cases.
  • quietone
    quietone @quietone started a thread on an old version of the diff
    • core/tests/Drupal/Tests/Core/Test/JUnitConverterTest.php
    133 'file' => '/Users/paul/projects/drupal/core/modules/big_pipe/tests/src/Unit/Render/BigPipeResponseAttachmentsProcessorTest.php',
    134 ],
    135 ],
    100 136 ];
    101 $this->assertEquals($simpletest, JUnitConverter::convertTestCaseToSimpletestRow(23, new \SimpleXMLElement($junit)));
    137
    138 }
    139
    140 /**
    141 * Generates a string consisting of alphanumeric characters.
    142 */
    143 private static function generateAlphanumericStr(int $length = 220): string {
    144 $characters = '0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ';
    145 $charLength = strlen($characters);
    146 $randomString = '';
    147 for ($i = 0; $i < $length; $i++) {
  • Pooja Sharma added 1 commit

    added 1 commit

    • a06e90e5 - issue/3019582: Applied suggestion JUnitConverterTest

    Compare with previous version

  • Pooja Sharma added 1 commit

    added 1 commit

    • a96f4e5d - issue/3019582: Added core inbuilt random() JUnitConverterTest

    Compare with previous version

  • quietone
    quietone @quietone started a thread on an old version of the diff
    • core/tests/Drupal/Tests/Core/Test/JUnitConverterTest.php
    87 public function testConvertTestCaseToSimpletestRow(): void {
    88 $junit = <<<EOD
    89 <testcase name="testGetTestClasses" class="Drupal\Tests\simpletest\Unit\TestDiscoveryTest" classname="Drupal.Tests.simpletest.Unit.TestDiscoveryTest" file="/Users/paul/projects/drupal/core/modules/simpletest/tests/src/Unit/TestDiscoveryTest.php" line="108" assertions="2" time="0.100787"/>
    90 EOD;
    91 $simpletest = [
    92 'test_id' => 23,
    93 'test_class' => 'Drupal\Tests\simpletest\Unit\TestDiscoveryTest',
    94 'status' => 'pass',
    95 'message' => '',
    96 'message_group' => 'Other',
    97 'function' => 'Drupal\Tests\simpletest\Unit\TestDiscoveryTest->testGetTestClasses()',
    98 'line' => 108,
    99 'file' => '/Users/paul/projects/drupal/core/modules/simpletest/tests/src/Unit/TestDiscoveryTest.php',
    96 public function testConvertTestCaseToSimpletestRow(string $junitXmlString, array $expectedSimpletestRow): void {
    97 $this->assertEquals($expectedSimpletestRow, JUnitConverter::convertTestCaseToSimpletestRow($expectedSimpletestRow['test_id'], new \SimpleXMLElement($junitXmlString)));
    98 $this->assertLessThanOrEqual(255, strlen($expectedSimpletestRow['function']), 'Function value is less than or equal to 255');
    • quietone
      quietone @quietone
      Maintainer
      Suggested change
      98 $this->assertLessThanOrEqual(255, strlen($expectedSimpletestRow['function']), 'Function value is less than or equal to 255');
      98 $this->assertLessThanOrEqual(255, strlen($expectedSimpletestRow['function']));

      We rely on the PHPUnit error message instead of adding a custom one. The exception is usually if the assertion is a loop and details of which case failed is needed.

    • Pooja Sharma changed this line in version 7 of the diff

      changed this line in version 7 of the diff

    • Please register or sign in to reply
  • quietone
    quietone @quietone started a thread on the diff
    • core/tests/Drupal/Tests/Core/Test/JUnitConverterTest.php
    117 'function' => 'Drupal\Tests\simpletest\Unit\TestDiscoveryTest->testGetTestClasses()',
    118 'line' => 108,
    119 'file' => '/Users/paul/projects/drupal/core/modules/simpletest/tests/src/Unit/TestDiscoveryTest.php',
    120 ],
    121 ],
    122 [
    123 <<<EOD
    124 <testcase name="{$long_function_name}" class="Drupal\Tests\big_pipe\Unit\Render\BigPipeResponseAttachmentsProcessorTest" classname="Drupal.Tests.big_pipe.Unit.Render.BigPipeResponseAttachmentsProcessorTest" file="/Users/paul/projects/drupal/core/modules/big_pipe/tests/src/Unit/Render/BigPipeResponseAttachmentsProcessorTest.php" line="83" assertions="4" time="0.100787"/>
    125 EOD,
    126 [
    127 'test_id' => 24,
    128 'test_class' => 'Drupal\Tests\big_pipe\Unit\Render\BigPipeResponseAttachmentsProcessorTest',
    129 'status' => 'pass',
    130 'message' => '',
    131 'message_group' => 'Other',
    132 'function' => mb_substr("Drupal\Tests\big_pipe\Unit\Render\BigPipeResponseAttachmentsProcessorTest->{$long_function_name}()", 0, 255),
    • quietone
      quietone @quietone
      Maintainer

      Why is this trimming the test data to 255. Isn't the problem when strings are greater than 255?

    • Pooja Sharma
      Pooja Sharma @pooja_sharma
      Author

      if data is more than 255 then this can lead to MySQL errors (as the data provided can be too long for the 'function' column (varchar, 255) or it can have bad encoding) due to which truncate(stated in IS also)

    • Please register or sign in to reply
  • Pooja Sharma added 1 commit

    added 1 commit

    • c198f481 - issue/3019582: Applied suggestion JUnitConverterTest.

    Compare with previous version

  • Pooja Sharma added 200 commits

    added 200 commits

    • c198f481...ae308e40 - 195 commits from branch project:11.x
    • 18760ce8 - issue/3019582: Updated JUnitConverterTest.php
    • 1aed7cfe - issue/3019582: Updated JUnitConverter.php
    • 9542caad - issue/3019582: Applied suggestion JUnitConverterTest
    • 0d47da38 - issue/3019582: Added core inbuilt random() JUnitConverterTest
    • 2690ba02 - issue/3019582: Applied suggestion JUnitConverterTest.

    Compare with previous version

    Toggle commit list
  • catch @catch started a thread on the diff
    • core/lib/Drupal/Core/Test/JUnitConverter.php
    122 124 }
    123 125
    124 126 $attributes = $test_case->attributes();
    127 $function_name = Unicode::convertToUtf8($attributes->class . '->' . $attributes->name . '()', 'UTF-8');
    128 $function_name = mb_strlen($function_name) > 255 ? mb_substr($function_name, 0, 255) : $function_name;
    • C
      catch @catch
      Maintainer

      I think this should probably trim the string from the end rather than the beginning (probably need to calculate the offset based on the length) - all our tests start with \Drupal, it's the end that's unique and useful to know.

    • Please register or sign in to reply
    Please register or sign in to reply