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.

#9 drupal8.system-module.1987594-9.patch3.02 KBdisasm
PASSED: [[SimpleTest]]: [MySQL] 59,020 pass(es). View
#9 interdiff.txt1.41 KBdisasm
#4 system-theme_test-convert_to_controller-1987594-4.patch2.34 KBmparker17
PASSED: [[SimpleTest]]: [MySQL] 58,898 pass(es). View
Members fund testing for the Drupal project. Drupal Association Learn more


vijaycs85’s picture

Status: Active » Closed (won't fix)

Need to rewrite the whole module to make test sync with current test implementation. For more details, please refer: #1988802: [META] Rewrite test modules in system to provide better unit testing.

ayelet_Cr’s picture

Status: Closed (won't fix) » Active
mparker17’s picture

Assigned: Unassigned » mparker17

I will help!

mparker17’s picture

Assigned: mparker17 » Unassigned
Status: Active » Needs review
2.34 KB
PASSED: [[SimpleTest]]: [MySQL] 58,898 pass(es). View

Try this...

dawehner’s picture

Status: Needs review » Needs work

This is nearly perfect!

  1. +++ b/core/modules/system/tests/modules/theme_test/lib/Drupal/theme_test/ThemeTestController.php
    @@ -31,4 +31,10 @@ function functionTemplateOverridden() {
    +  function themeTestSuggestion() {
    +    return theme(array('theme_test__suggestion', 'theme_test'), array());

    Let's make the method public and return a render array.

  2. +++ b/core/modules/system/tests/modules/theme_test/theme_test.module
    @@ -47,8 +47,7 @@ function theme_test_system_theme_info() {
       $items['theme-test/suggestion'] = array(
         'title' => 'Suggestion',
    -    'page callback' => '_theme_test_suggestion',
    -    'access callback' => TRUE,
    +    'route_name' => 'theme_test_suggestion',
         'theme callback' => '_theme_custom_theme',
         'type' => MENU_CALLBACK,

    Let's just drop the full menu item, as MENU_CALLBACK is not needed.

mparker17’s picture

@dawehner, just two quick questions before I work on this...

1. I left the hook_menu item in because it had a 'theme callback'. Will returning a render array allow me to get around that?
2. I'm not entirely sure how to return a render array in this circumstance because of the unusual call to theme(). Could I trouble you to provide me with an example?


mparker17’s picture


disasm’s picture

Status: Needs work » Needs review
disasm’s picture

1.41 KB
3.02 KB
PASSED: [[SimpleTest]]: [MySQL] 59,020 pass(es). View

Doing some git searches, I can't find any previous conversions that have:
a) removed a theme_callback
b) converted a theme() function with an array() as first parameter to a render array.

As such, my suggestion is to let this slide and get it committed where we can deal with refactoring a and b in a follow-up. The tricky thing with a is no one has addressed what theme_callback because in new routes that I know of. The tricky thing in b is the theme() function when it's an array uses the first available hook, and ignores the other parameter, so in the case of the following:

theme(array('foo', 'bar'), array());

If the foo theme exists in the registry, it is used. If the foo theme does not exist, but the bar one does it is used. I am an un-aware of a #theme equivalent to this logic, and probably the whole thing should be refactored anyways.

Attached patch makes the methods public.

dawehner’s picture

+++ w/core/modules/system/tests/modules/theme_test/theme_test.module
@@ -47,8 +47,7 @@ function theme_test_system_theme_info() {
   $items['theme-test/suggestion'] = array(
     'title' => 'Suggestion',

+++ w/core/modules/system/tests/modules/theme_test/theme_test.routing.yml
@@ -4,3 +4,10 @@ function_template_override:
+  pattern: 'theme-test/suggestion'
+  defaults:
+    _content: '\Drupal\theme_test\ThemeTestController::themeTestSuggestion'

See title on the other review.

disasm’s picture

disasm’s picture

Status: Needs review » Closed (duplicate)