Task to expand test coverage for \Drupal\Component\Utility\Unicode.

See #1938068: [Meta] Convert UnitTestBase to PHPUnit.

Files: 
CommentFileSizeAuthor
#11 unicode-phpunit-2051717-11.interdiff.txt513 bytesMile23
#11 unicode-phpunit-2051717-11.patch5.1 KBMile23
PASSED: [[SimpleTest]]: [MySQL] 58,502 pass(es).
[ View ]
#8 unicode-phpunit-2051717-08.patch5.06 KBMile23
PASSED: [[SimpleTest]]: [MySQL] 58,124 pass(es).
[ View ]
#8 unicode-phpunit-2051717-08.interdiff.txt4.41 KBMile23
#4 unicode-phpunit-2051717-04.patch7.63 KBMile23
PASSED: [[SimpleTest]]: [MySQL] 58,209 pass(es).
[ View ]
#4 unicode-phpunit-2051717-04.interdiff.txt1.25 KBMile23
#1 unicode-phpunit-2051717-01.patch6.67 KBjhedstrom
PASSED: [[SimpleTest]]: [MySQL] 57,528 pass(es).
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new6.67 KB
PASSED: [[SimpleTest]]: [MySQL] 57,528 pass(es).
[ View ]

Here's a start that brings coverage from 48% to 68%. The remaining uncovered code seems to be largely dependent on specific environments from what I can tell.

As always, what a impressive amount of detail!

So which code is hard to test:

<?php
 
public static function check() {
   
// Check for mbstring extension.
   
if (!function_exists('mb_strlen')) {
      static::
$status = static::STATUS_SINGLEBYTE;
      return
'mb_strlen';
    }
   
// Check mbstring configuration.
   
if (ini_get('mbstring.func_overload') != 0) {
      static::
$status = static::STATUS_ERROR;
      return
'mbstring.func_overload';
    }
    if (
ini_get('mbstring.encoding_translation') != 0) {
      static::
$status = static::STATUS_ERROR;
      return
'mbstring.encoding_translation';
    }
    if (
ini_get('mbstring.http_input') != 'pass') {
      static::
$status = static::STATUS_ERROR;
      return
'mbstring.http_input';
    }
    if (
ini_get('mbstring.http_output') != 'pass') {
      static::
$status = static::STATUS_ERROR;
      return
'mbstring.http_output';
    }
   
// Set appropriate configuration.
   
mb_internal_encoding('utf-8');
   
mb_language('uni');
    static::
$status = static::STATUS_MULTIBYTE;
    return
'';
  }
 
/**
   * Converts data to UTF-8.
   *
   * Requires the iconv, GNU recode or mbstring PHP extension.
   *
   * @param string $data
   *   The data to be converted.
   * @param string $encoding
   *   The encoding that the data is in.
   *
   * @return string|bool
   *   Converted data or FALSE.
   */
 
public static function convertToUtf8($data, $encoding) {
    if (
function_exists('iconv')) {
      return @
iconv($encoding, 'utf-8', $data);
    }
    elseif (
function_exists('mb_convert_encoding')) {
      return @
mb_convert_encoding($data, 'utf-8', $encoding);
    }
    elseif (
function_exists('recode_string')) {
      return @
recode_string($encoding . '..utf-8', $data);
    }
   
// Cannot convert.
   
return FALSE;
  }
?>

as well as large amount of substr (which maybe uses also external code if possible)

At least for substr() we can call setStatus directly, what do you think?

Status:Needs review» Needs work

Status:Needs work» Needs review
StatusFileSize
new1.25 KB
new7.63 KB
PASSED: [[SimpleTest]]: [MySQL] 58,209 pass(es).
[ View ]

Added single- and multi-byte code paths to substr and strlen tests.

It'd require some refactoring to test check() and covertToUtf8() adequately, which might slow things down.

Still though... C.R.A.P. score went from 788 to 80. :-)

  1. +++ b/core/tests/Drupal/Tests/Component/Utility/UnicodeTest.php
    @@ -12,6 +12,8 @@
    + *
    + * @see \Drupal\Component\Utility\Unicode
      */
    class UnicodeTest extends UnitTestCase {

    Lets also add @group Drupal too? and maybe unicode?

  2. +++ b/core/tests/Drupal/Tests/Component/Utility/UnicodeTest.php
    @@ -82,9 +84,11 @@ public function testMimeHeader($value, $encoded) {
    -  public function providerTestMimeHeader() {
    +  public static function providerTestMimeHeader() {

    i can see why we static here, but maybe we should add it on #2057905: [policy, no patch] Discuss the standards for phpunit based tests so other people are aware? Right now only one provider is static in core

I hadn't noticed that the data providers were set static. I'm curious why that would be preferable (or not).

Let's not make them static, as there is no point in doing it.

StatusFileSize
new4.41 KB
new5.06 KB
PASSED: [[SimpleTest]]: [MySQL] 58,124 pass(es).
[ View ]

Took away the static.

Status:Needs review» Reviewed & tested by the community

Really impressive additional test coverage!

+++ b/core/tests/Drupal/Tests/Component/Utility/UnicodeTest.php
@@ -12,6 +12,8 @@
/**
  * Test unicode handling features implemented in Unicode component.
+ *
+ * @see \Drupal\Component\Utility\Unicode
  */
class UnicodeTest extends UnitTestCase {

Just in case you have to do another rerole you could add @group Drupal and @group Unicode

Doh!

Will add when the test comes back.

StatusFileSize
new5.1 KB
PASSED: [[SimpleTest]]: [MySQL] 58,502 pass(es).
[ View ]
new513 bytes

Added @group Drupal and Unicode.

Status:Reviewed & tested by the community» Fixed

Wow, great stuff, thanks!

Committed and pushed to 8.x.

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