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

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
StatusFileSize
new1.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

StatusFileSize
new12.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
StatusFileSize
new12.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
StatusFileSize
new5.09 KB
new21.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

StatusFileSize
new5.09 KB
new12.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
StatusFileSize
new7.95 KB
new11.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
StatusFileSize
new1005 bytes
new46.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
StatusFileSize
new527 bytes
new46.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
StatusFileSize
new1004 bytes
new12.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
StatusFileSize
new22.8 KB
new564 bytes
new12.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
StatusFileSize
new685 bytes
new12.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
StatusFileSize
new709 bytes
new12.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
StatusFileSize
new1.76 KB
new12.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
StatusFileSize
new564 bytes
new120.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
StatusFileSize
new12.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.