Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mparker17’s picture

Assigned: mparker17 » Unassigned
Status: Active » Needs review
Issue tags: +WSCCI-conversion
FileSize
3.07 KB

Try this...

dawehner’s picture

Status: Needs review » Needs work
  1. +++ b/core/modules/system/tests/modules/common_test/lib/Drupal/common_test/Controller/CommonTestController.php
    @@ -59,4 +59,23 @@ public function typeLinkActiveClass() {
    +   * Renders an element with an invalid render array key.
    +   */
    ...
    +    return drupal_render($element);
    

    There is no need to already render the result, so let's also document the return statement.

  2. +++ b/core/modules/system/tests/modules/common_test/lib/Drupal/common_test/Controller/CommonTestController.php
    @@ -59,4 +59,23 @@ public function typeLinkActiveClass() {
    +    drupal_set_title('Drupal Render');
    

    This bit can be replaced by setting '#title' on the render array.

  3. +++ b/core/modules/system/tests/modules/common_test/lib/Drupal/common_test/Controller/CommonTestController.php
    @@ -59,4 +59,23 @@ public function typeLinkActiveClass() {
    +    // Keys that begin with # may contain a value of any type, otherwise they must
    +    // contain arrays.
    +    $key = 'child';
    +    $value = 'This should be an array.';
    +    $element = array(
    +      $key => $value,
    +    );
    

    I am confused by that bit, why do we not just create array('child' => 'This should be an array')

mparker17’s picture

Status: Needs work » Needs review
FileSize
1.78 KB
3.04 KB

Try this...

Status: Needs review » Needs work
Issue tags: -WSCCI-conversion

The last submitted patch, drupal8.system-module.2066523-3.patch, failed testing.

mparker17’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work
Issue tags: +WSCCI-conversion

The last submitted patch, drupal8.system-module.2066523-3.patch, failed testing.

disasm’s picture

In comment #2 1 is incorrect. This needs to return drupal_render, because that forces the error to happen inside the method that's defining SIMPLETEST_COLLECT_ERRORS to FALSE. Yes, for any normal use case, it doesn't make sense to render the array in a controller, but in this case, being a specific test where we're overriding error handling by simpletest, it's needed.

disasm’s picture

Status: Needs work » Needs review
FileSize
883 bytes
3.05 KB

attached patch should get this green again.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Perfect!

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 7470a18 and pushed to 8.x. Thanks!

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