Support from Acquia helps fund testing for Drupal Acquia logo

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

Status: Active » Needs review
FileSize
2.14 KB

conversion patch.

dawehner’s picture

  1. +++ b/core/modules/system/tests/modules/url_alter_test/lib/Drupal/url_alter_test/Controller/URLAlterTestController.php
    @@ -0,0 +1,22 @@
    +class URLAlterTestController {
    +  /**
    

    This should have a newline in between.

  2. +++ b/core/modules/system/tests/modules/url_alter_test/lib/Drupal/url_alter_test/Controller/URLAlterTestController.php
    @@ -0,0 +1,22 @@
    +    print 'current_path=' . current_path() . ' request_path=' . request_path();
    +    exit;
    

    You should return just a response object.

  3. +++ b/core/modules/system/tests/modules/url_alter_test/url_alter_test.module
    @@ -11,17 +11,8 @@
       $items['url-alter-test/foo'] = array(
         'title' => 'Foo',
    -    'page callback' => 'url_alter_test_foo',
    -    'access arguments' => array('access content'),
    +    'route_name' => 'url_alter_test_foo',
         'type' => MENU_CALLBACK,
       );
    

    this hook can be removed as it is, because MENU_CALLBACKs are directly replaced with controllers.

dawehner’s picture

Status: Needs review » Needs work

.

disasm’s picture

Status: Needs work » Needs review
FileSize
1.65 KB
2.28 KB

Attached patch addresses comments in #5.

dawehner’s picture

+++ b/core/modules/system/tests/modules/url_alter_test/lib/Drupal/url_alter_test/Controller/URLAlterTestController.php
@@ -0,0 +1,24 @@
+   *
+   */

Let's also add a simple @return on there.

disasm’s picture

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Thank you.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 80d9f5c and pushed to 8.x. Thanks!

At some point we need to replace current_path and request_path with stuff based on Symfony's Request object.

+++ b/core/modules/system/tests/modules/url_alter_test/lib/Drupal/url_alter_test/Controller/URLAlterTestController.php
@@ -0,0 +1,26 @@
+    return new Response('current_path=' . current_path() . ' request_path=' . request_path());

I think we should use the Request to get the currecnt path. The function uses drupal_container()->get('request')->attributes->get('_system_path')

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