Issue summary updated: #42
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.

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Task because it's updating code to the new API/code style
Issue priority Major because it's a refactoring task
Unfrozen changes Unfrozen because it is a change in automated tests
Prioritized changes The main goal of this issue is removing deprecated code style.
Disruption The change is not disruptive
Files: 
CommentFileSizeAuthor
#34 drupal8.test_page.1987868-34.patch12.18 KBdisasm
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch drupal8.test_page.1987868-34.patch. Unable to apply patch. See the log in the details link for more information. View
#32 drupal8.test_page.1987868-32.patch120.47 KBdisasm
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal8.test_page.1987868-32.patch. Unable to apply patch. See the log in the details link for more information. View
#32 interdiff.txt564 bytesdisasm
#28 drupal8.test_page.1987868-28.patch12.13 KBdisasm
FAILED: [[SimpleTest]]: [MySQL] 59,131 pass(es), 4 fail(s), and 0 exception(s). View
#28 interdiff.txt1.76 KBdisasm
#25 drupal8.test_page_test.1987868-25.patch12.13 KBdisasm
PASSED: [[SimpleTest]]: [MySQL] 58,144 pass(es). View
#25 interdiff.txt709 bytesdisasm
#23 drupal8.test_page_test.1987868-23.patch12.13 KBdisasm
FAILED: [[SimpleTest]]: [MySQL] 58,406 pass(es), 2 fail(s), and 1 exception(s). View
#23 interdiff.txt685 bytesdisasm
#21 drupal8.test_page_test.1987868-21.patch12.12 KBdisasm
FAILED: [[SimpleTest]]: [MySQL] 58,076 pass(es), 5 fail(s), and 3 exception(s). View
#21 interdiff.txt564 bytesdisasm
#21 Screen Shot 2013-08-31 at 1.34.26 AM.png22.8 KBdisasm
#19 drupal8.test_page_test.1987868-19.patch12.03 KBdisasm
FAILED: [[SimpleTest]]: [MySQL] 58,340 pass(es), 21 fail(s), and 83 exception(s). View
#19 interdiff.txt1004 bytesdisasm
#16 drupal8.test_page_test.1987868-16.patch46.93 KBdisasm
FAILED: [[SimpleTest]]: [MySQL] 58,174 pass(es), 17 fail(s), and 75 exception(s). View
#16 interdiff.txt527 bytesdisasm
#14 drupal8.test_page_test.1987868-14.patch46.93 KBdisasm
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/system/tests/modules/test_page_test/lib/Drupal/test_page_test/Controller/Test.php. View
#14 interdiff.txt1005 bytesdisasm
#12 drupal8.system-module.1987868-12.patch11.91 KBdisasm
FAILED: [[SimpleTest]]: [MySQL] 57,931 pass(es), 13 fail(s), and 3 exception(s). View
#12 interdiff.txt7.95 KBdisasm
#10 drupal8.system-module.1987868-10.patch12.16 KBdisasm
FAILED: [[SimpleTest]]: [MySQL] 57,890 pass(es), 13 fail(s), and 3 exception(s). View
#10 interdiff.txt5.09 KBdisasm
#9 drupal8.system-module.1987868-9.patch21.17 KBdisasm
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion. View
#9 interdiff.txt5.09 KBdisasm
#7 drupal8.system-module.1987868-7.patch12.16 KBdisasm
FAILED: [[SimpleTest]]: [MySQL] 57,669 pass(es), 46 fail(s), and 4 exception(s). View
#5 drupal8.system-module.1987868-5.patch12.15 KBdisasm
FAILED: [[SimpleTest]]: [MySQL] 54,855 pass(es), 17 fail(s), and 0 exception(s). View
#4 drupal8.system-module.1987868-4.patch1.77 MBdisasm
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion. View

Comments

tim.plunkett’s picture

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
disasm’s picture

Assigned: Unassigned » disasm
Status: Active » Needs review
FileSize
1.77 MB
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion. View

converting test_page_test_page to a new controller and updating all the menu hooks that call it.

disasm’s picture

FileSize
12.15 KB
FAILED: [[SimpleTest]]: [MySQL] 54,855 pass(es), 17 fail(s), and 0 exception(s). View

forgot to rebase before creating the diff.

Status: Needs review » Needs work

The last submitted patch, drupal8.system-module.1987868-5.patch, failed testing.

disasm’s picture

Status: Needs work » Needs review
FileSize
12.16 KB
FAILED: [[SimpleTest]]: [MySQL] 57,669 pass(es), 46 fail(s), and 4 exception(s). View

Try again with the correct namespace.

Status: Needs review » Needs work

The last submitted patch, drupal8.system-module.1987868-7.patch, failed testing.

disasm’s picture

Status: Needs work » Needs review
FileSize
5.09 KB
21.17 KB
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion. View

wrong key on hook_menu entries (route name -> route_name).

disasm’s picture

FileSize
5.09 KB
12.16 KB
FAILED: [[SimpleTest]]: [MySQL] 57,890 pass(es), 13 fail(s), and 3 exception(s). View

forgot to rebase

Status: Needs review » Needs work

The last submitted patch, drupal8.system-module.1987868-10.patch, failed testing.

disasm’s picture

Status: Needs work » Needs review
FileSize
7.95 KB
11.91 KB
FAILED: [[SimpleTest]]: [MySQL] 57,931 pass(es), 13 fail(s), and 3 exception(s). View

Moving testPage method to Test Controller added by #2032535: Resolve 'title' using the route and render array.

Status: Needs review » Needs work

The last submitted patch, drupal8.system-module.1987868-12.patch, failed testing.

disasm’s picture

Status: Needs work » Needs review
FileSize
1005 bytes
46.93 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/system/tests/modules/test_page_test/lib/Drupal/test_page_test/Controller/Test.php. View

attached is a patch that converts drupal_add_js and drupal_set_title to render array.

Status: Needs review » Needs work

The last submitted patch, drupal8.test_page_test.1987868-14.patch, failed testing.

disasm’s picture

Status: Needs work » Needs review
FileSize
527 bytes
46.93 KB
FAILED: [[SimpleTest]]: [MySQL] 58,174 pass(es), 17 fail(s), and 75 exception(s). View

I really need to run phpcs on patches when I work on them after midnight ;-) Fixing two syntax errors.

Status: Needs review » Needs work

The last submitted patch, drupal8.test_page_test.1987868-16.patch, failed testing.

jibran’s picture

Why we have a comment out code in the patch?

disasm’s picture

Status: Needs work » Needs review
FileSize
1004 bytes
12.03 KB
FAILED: [[SimpleTest]]: [MySQL] 58,340 pass(es), 21 fail(s), and 83 exception(s). View

I really messed up that branch last night. This interdiff goes back to #12.

Status: Needs review » Needs work

The last submitted patch, drupal8.test_page_test.1987868-19.patch, failed testing.

disasm’s picture

Status: Needs work » Needs review
FileSize
22.8 KB
564 bytes
12.12 KB
FAILED: [[SimpleTest]]: [MySQL] 58,076 pass(es), 5 fail(s), and 3 exception(s). View

Adding hook_menu back in. Some reason when the entry doesn't exist, no menus show up at all. This is a temporary fix until we figure out what the reason for it is.

Screen Shot 2013-08-31 at 1.34.26 AM.png

Status: Needs review » Needs work

The last submitted patch, drupal8.test_page_test.1987868-21.patch, failed testing.

disasm’s picture

Status: Needs work » Needs review
FileSize
685 bytes
12.13 KB
FAILED: [[SimpleTest]]: [MySQL] 58,406 pass(es), 2 fail(s), and 1 exception(s). View

fixing more tests. If this is green, will open up another issue to resolve the menu issue when no hook_menu exists.

Status: Needs review » Needs work

The last submitted patch, drupal8.test_page_test.1987868-23.patch, failed testing.

disasm’s picture

Status: Needs work » Needs review
FileSize
709 bytes
12.13 KB
PASSED: [[SimpleTest]]: [MySQL] 58,144 pass(es). View

Some more test fixes.

xjm’s picture

Thanks for your work on this issue! Please see #1971384-43: [META] Convert page callbacks to controllers for an update on the routing system conversion process.

dawehner’s picture

Status: Needs review » Needs work

Missing title on all of them.

  1. +++ b/core/modules/system/tests/modules/test_page_test/lib/Drupal/test_page_test/Controller/Test.php
    @@ -26,4 +26,23 @@ public function renderTitle() {
    +   *
    +   */
    +  public function testPage() {
    

    Missing @return.

  2. +++ b/core/modules/system/tests/modules/test_page_test/test_page_test.module
    @@ -1,26 +1,9 @@
    - */
     function test_page_test_menu() {
       $items['test-page'] = array(
         'title' => 'Test front page',
    -    'page callback' => 'test_page_test_page',
    -    'access callback' => TRUE,
    +    'route_name' => 'test_page_test_page',
         'type' => MENU_CALLBACK,
       );
    
    +++ b/core/modules/system/tests/modules/test_page_test/test_page_test.routing.yml
    @@ -4,3 +4,11 @@ test_page_render_title:
    +
    +test_page_test_page:
    +  pattern: '/test-page'
    +  defaults:
    +    _content: '\Drupal\test_page_test\Controller\Test::testPage'
    +  requirements:
    +    _access: 'TRUE'
    +
    

    This menu callback can be removed entirely. ... also missing title on the routing yml

disasm’s picture

Status: Needs work » Needs review
FileSize
1.76 KB
12.13 KB
FAILED: [[SimpleTest]]: [MySQL] 59,131 pass(es), 4 fail(s), and 0 exception(s). View

removing hook_menu. Adding @return, adding title.

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

The last submitted patch, drupal8.test_page.1987868-28.patch, failed testing.

disasm’s picture

Status: Needs work » Needs review

#28: drupal8.test_page.1987868-28.patch queued for re-testing.

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

The last submitted patch, drupal8.test_page.1987868-28.patch, failed testing.

disasm’s picture

Status: Needs work » Needs review
FileSize
564 bytes
120.47 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal8.test_page.1987868-32.patch. Unable to apply patch. See the log in the details link for more information. View

adding hook_menu back in.

Status: Needs review » Needs work

The last submitted patch, drupal8.test_page.1987868-32.patch, failed testing.

disasm’s picture

Status: Needs work » Needs review
FileSize
12.18 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch drupal8.test_page.1987868-34.patch. Unable to apply patch. See the log in the details link for more information. View

reroll!

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

The last submitted patch, drupal8.test_page.1987868-34.patch, failed testing.

disasm’s picture

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

#34: drupal8.test_page.1987868-34.patch queued for re-testing.

dawehner’s picture

+++ w/core/modules/system/tests/modules/test_page_test/test_page_test.module
@@ -1,26 +1,9 @@
   $items['test-page'] = array(
     'title' => 'Test front page',
-    'page callback' => 'test_page_test_page',
-    'access callback' => TRUE,
+    'route_name' => 'test_page_test_page',
     'type' => MENU_CALLBACK,
   );

We could just remove that.

disasm’s picture

sadly, that can't be removed without altering tests: Path from menu_get_item('test-page') is equal to 'test-page'. If you remove it, menu_get_item doesn't know
about test-page.

xjm’s picture

Status: Needs review » Needs work

The last submitted patch, 34: drupal8.test_page.1987868-34.patch, failed testing.

xjm’s picture

Issue summary: View changes
Issue tags: +Needs reroll
valthebald’s picture

Issue summary: View changes

Added beta stage evaluation

valthebald’s picture

Assigned: disasm » valthebald
Issue tags: +#SprintWeekend2015

Working on it as part of #SprintWeekend2015

valthebald’s picture

Status: Needs work » Closed (duplicate)

This issue is already solved by [#2301137]

David Hernández’s picture

Issue tags: -#SprintWeekend2015 +SprintWeekend2015

Hello!

Thank you for working on this issue!

We should all try and use the same sprint tag. According to https://groups.drupal.org/node/447258 it should be SprintWeekend2015 with no #.

penyaskito’s picture

@valthebald Wrong issue link?

DamienMcKenna’s picture

Issue tags: -Needs reroll

Removed the "Needs reroll" seeing as the issue was closed.