Part of #1971384: [META] Convert page callbacks to controllers

For instructions on how to convert a page callback into a controller, see the WSCCI Conversion Guide.

Files: 
CommentFileSizeAuthor
#8 drupal8.drupal_render_invalid_keys.2066523-8.patch3.05 KBdisasm
PASSED: [[SimpleTest]]: [MySQL] 58,533 pass(es). View
#8 interdiff.txt883 bytesdisasm
#3 drupal8.system-module.2066523-3.patch3.04 KBmparker17
FAILED: [[SimpleTest]]: [MySQL] 58,484 pass(es), 1 fail(s), and 0 exception(s). View
#3 interdiff.txt1.78 KBmparker17
#1 system-common_test-render_invalid_keys_controller-2066523-1.patch3.07 KBmparker17
PASSED: [[SimpleTest]]: [MySQL] 57,877 pass(es). View

Comments

mparker17’s picture

Assigned: mparker17 » Unassigned
Status: Active » Needs review
Issue tags: +WSCCI-conversion
FileSize
3.07 KB
PASSED: [[SimpleTest]]: [MySQL] 57,877 pass(es). View

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
FAILED: [[SimpleTest]]: [MySQL] 58,484 pass(es), 1 fail(s), and 0 exception(s). View

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
PASSED: [[SimpleTest]]: [MySQL] 58,533 pass(es). View

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.