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
#22 drupal8.system-module.1987718-22.patch5.83 KBbrianV
PASSED: [[SimpleTest]]: [MySQL] 59,386 pass(es).
[ View ]
#20 drupal8.system-module.1987718-20.patch5.79 KBbrianV
FAILED: [[SimpleTest]]: [MySQL] 59,364 pass(es), 16 fail(s), and 8 exception(s).
[ View ]
#17 drupal8.system-module.1987718-17.patch5.1 KBbrianV
FAILED: [[SimpleTest]]: [MySQL] 59,270 pass(es), 16 fail(s), and 8 exception(s).
[ View ]
#15 drupal8.system-module.1987718-15.patch5.16 KBbrianV
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]
#13 drupal8.system-module.1987718-13.patch5.11 KBbrianV
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]
#7 drupal8.system-module.1987718-6.patch5.13 KBbrianV
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]
#5 drupal8.system-module.1987718-5.patch0 bytesbrianV
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]

Comments

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.

Status:Closed (won't fix)» Active

Assigned:Unassigned» kgoel

Going to work on this.

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.

Status:Active» Needs review
StatusFileSize
new0 bytes
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]

First attempt at a patch. Thanks @larowlan for walking me through it so far.

Assigned:kgoel» Unassigned

Assigned:Unassigned» kgoel
StatusFileSize
new5.13 KB
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]

And... this time without an empty patch file. Sorry all!

Assigned:kgoel» brianV

cross posted

Assigned:brianV» Unassigned

I think we made edit at the same time.

Assigned:Unassigned» brianV

Last cross post of the night ;)

Assigned:brianV» Unassigned

Great job, talk about jumping in at the deep end!

  1. +++ b/core/modules/system/tests/modules/form_test/lib/Drupal/form_test/Controller/FormTestController.php
    @@ -0,0 +1,42 @@
    +use Drupal\Core\Controller\ControllerBase;

    chuck a blank line between the namespace and use statements as per http://drupal.org/node/1354

  2. +++ b/core/modules/system/tests/modules/form_test/lib/Drupal/form_test/Controller/FormTestController.php
    @@ -0,0 +1,42 @@
    +  /**
    +   * Constructs a FormTestController object.
    +   */
    +  public function __construct() {
    +  }

    This empty method can go thanks to ControllerBase having all the stuff we need.

  3. +++ b/core/modules/system/tests/modules/form_test/lib/Drupal/form_test/Controller/FormTestController.php
    @@ -0,0 +1,42 @@
    +   *

    Extra blank line can go

  4. +++ b/core/modules/system/tests/modules/form_test/lib/Drupal/form_test/Controller/FormTestController.php
    @@ -0,0 +1,42 @@
    +  function twoFormInstances() {

    missing 'public'

  5. +++ b/core/modules/system/tests/modules/form_test/lib/Drupal/form_test/Controller/FormTestController.php
    @@ -0,0 +1,42 @@
    +}
    +++ b/core/modules/system/tests/modules/form_test/lib/Drupal/form_test/EventSubscriber/FormTestEventSubscriber.php
    @@ -33,8 +52,25 @@ public function onKernelRequest(GetResponseEvent $event) {
    }

    Needs a blank line before last }

StatusFileSize
new5.11 KB
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]

See attached, should resolve the above.

  1. +++ b/core/modules/system/tests/modules/form_test/lib/Drupal/form_test/Controller/FormTestController.php
    @@ -0,0 +1,38 @@
    +  ¶
    +++ b/core/modules/system/tests/modules/form_test/lib/Drupal/form_test/EventSubscriber/FormTestEventSubscriber.php
    @@ -33,8 +52,26 @@ public function onKernelRequest(GetResponseEvent $event) {
    +  ¶

    nitpick:trailing whitespace

  2. +++ b/core/modules/system/tests/modules/form_test/lib/Drupal/form_test/EventSubscriber/FormTestEventSubscriber.php
    @@ -17,6 +21,21 @@
    +
    +

    nitpick:can we loose one of these extra lines

Other than that RTBC in my opinion, assuming bot agrees

StatusFileSize
new5.16 KB
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]

What's a nitpick between friends?

Hopefully final revision attached.

+++ b/core/modules/system/tests/modules/form_test/lib/Drupal/form_test/Controller/FormTestController.php
@@ -0,0 +1,38 @@
\ No newline at end of file
+++ b/core/modules/system/tests/modules/form_test/lib/Drupal/form_test/EventSubscriber/FormTestEventSubscriber.php
@@ -33,8 +51,26 @@ public function onKernelRequest(GetResponseEvent $event) {
\ No newline at end of file

sorry I've confused you :( there were lines with whitespace, but the ones at the end of the file had to stay

StatusFileSize
new5.1 KB
FAILED: [[SimpleTest]]: [MySQL] 59,270 pass(es), 16 fail(s), and 8 exception(s).
[ View ]

Good. I thought you had suggested otherwise ;)

re-roll!

Status:Needs review» Reviewed & tested by the community

Unless bot disagrees.

Status:Reviewed & tested by the community» Needs work

The last submitted patch, drupal8.system-module.1987718-17.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new5.79 KB
FAILED: [[SimpleTest]]: [MySQL] 59,364 pass(es), 16 fail(s), and 8 exception(s).
[ View ]

New version with the two changes:

1. I never removed the original form_test_two_instances() callback. That's done now.

2. EntityManager::getStorageController('node')->create(); doesn't seem to create the node form with the form display settings set by entity_get_form_display()->setComponent()->save() into account. Therefore, the test failed in the above patch because the AJAX field created specifically for that test wasn't being added to the node form.

I asked about #2 in #drupal-contribute a few times, but got no responses, so I've reverted back to entity_create() in the controller to see if that fixes it.

Status:Needs review» Needs work

The last submitted patch, drupal8.system-module.1987718-20.patch, failed testing.

StatusFileSize
new5.83 KB
PASSED: [[SimpleTest]]: [MySQL] 59,386 pass(es).
[ View ]

This should work, passes all tests locally.

Turns out the issue was this the permissions for the router item that prevented the test user from seeing the page.

Reverted back to EntityManager::getStorageController('node')->create(); to create the node objects for the node forms.

Status:Needs work» Needs review

bumping back to NR

Status:Needs review» Reviewed & tested by the community

Legend, thanks for coming back and working through it

Status:Reviewed & tested by the community» Fixed

Great work! I found a couple of small indentation problems on the comments, but fixed those on commit.

Committed and pushed to 8.x. Thanks!

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