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

Assigned: Unassigned » partyka

Reopening.

partyka’s picture

++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
The patch is provided as noted in the terms of the copyright notice.

The form_test_wrapper_callback is converted to use a controller instead of hook_menu

This patch is being submitted by Argonne National Laboratory, August 15, 2013

+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

partyka’s picture

Status: Active » Needs review

Setting patch to needs review.

Status: Needs review » Needs work

The last submitted patch, 1987720-form-test-wrapper-callback.patch, failed testing.

partyka’s picture

Status: Needs work » Needs review
FileSize
3.5 KB

Updated patch for line-endings.

dawehner’s picture

Status: Needs review » Needs work
  1. +++ drupal/core/modules/system/tests/modules/form_test/form_test.module	(revision )
    @@ -119,9 +119,7 @@
       $items['form_test/wrapper-callback'] = array(
         'title' => 'Form wrapper callback test',
    -    'page callback' => 'form_test_wrapper_callback',
    -    'page arguments' => array('form_test_wrapper_callback_form'),
    -    'access callback' => TRUE,
    +    'route_name' => 'form_test.route8',
         'type' => MENU_CALLBACK,
       );
    

    Let's drop this hook_menu() entry.

  2. +++ drupal/core/modules/system/tests/modules/form_test/lib/Drupal/form_test/Controller/FormTestController.php	(revision )
    @@ -0,0 +1,17 @@
    +<?php
    +
    ...
    +
    +class FormTestController {
    ...
    +  public function testWrapper($form_id) {
    

    Let's add some simple lines.

  3. +++ drupal/core/modules/system/tests/modules/form_test/lib/Drupal/form_test/Controller/FormTestController.php	(revision )
    Index: drupal/core/modules/system/tests/modules/form_test/lib/Drupal/form_test/ConfirmFormArrayPathTestForm.php
    ===================================================================
    
    ===================================================================
    --- drupal/core/modules/system/tests/modules/form_test/lib/Drupal/form_test/ConfirmFormArrayPathTestForm.php	(revision cd15b235db7353771053ad56018e4a65abffdd6f)
    
    --- drupal/core/modules/system/tests/modules/form_test/lib/Drupal/form_test/ConfirmFormArrayPathTestForm.php	(revision cd15b235db7353771053ad56018e4a65abffdd6f)
    +++ drupal/core/modules/system/tests/modules/form_test/lib/Drupal/form_test/ConfirmFormArrayPathTestForm.php	(revision )
    
    +++ drupal/core/modules/system/tests/modules/form_test/lib/Drupal/form_test/ConfirmFormArrayPathTestForm.php	(revision )
    +++ drupal/core/modules/system/tests/modules/form_test/lib/Drupal/form_test/ConfirmFormArrayPathTestForm.php	(revision )
    @@ -38,4 +38,6 @@
    
    @@ -38,4 +38,6 @@
         return t('ConfirmFormArrayPathTestForm::getCancelText().');
       }
     
    +
    +
     }
    

    No need for this change

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.

vijaycs85’s picture

Issue summary: View changes
Status: Needs work » Needs review
FileSize
3.67 KB

Re-roll that fixes all review comments in #8

Status: Needs review » Needs work

The last submitted patch, 10: 1987720-form_test-controller-10.patch, failed testing.

YesCT’s picture

Assigned: partyka » Unassigned
Issue tags: +Needs reroll

reroll instructions: https://drupal.org/contributor-tasks/reroll

@partyka, it has been a few months. unassigning. no worries. :)

Sam Hermans’s picture

Assigned: Unassigned » Sam Hermans

I'll check this out

Sam Hermans’s picture

Assigned: Sam Hermans » Unassigned
Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
3.67 KB

Re rolled patch agains latest HEAD.

Sam Hermans’s picture

FileSize
4.3 KB

Fixed failing tests

The last submitted patch, 14: 1987720-form_test-controller-14.patch, failed testing.

pfrenssen’s picture

Status: Needs review » Needs work
  1. +++ b/core/modules/system/tests/modules/form_test/lib/Drupal/form_test/Controller/FormTestController.php
    @@ -8,11 +8,40 @@
     use Drupal\Core\Controller\ControllerBase;
     use Drupal\Core\Language\Language;
    +use Drupal\Core\Form\FormBuilderInterface;
    +use Symfony\Component\DependencyInjection\ContainerInterface;
    +use Drupal\Core\DependencyInjection\ContainerInjectionInterface;
    

    These can be put in alphabetical order.

  2. +++ b/core/modules/system/tests/modules/form_test/lib/Drupal/form_test/Controller/FormTestController.php
    @@ -8,11 +8,40 @@
    +  /**
    +   * Constructs a new search controller.
    +   *
    

    This is not a search controller. This can be changed to:

    Constructs a new FormTestController.

Sam Hermans’s picture

Assigned: Unassigned » Sam Hermans
Sam Hermans’s picture

Assigned: Sam Hermans » Unassigned
Status: Needs work » Needs review
FileSize
1.21 KB
4.34 KB

Thank you for your reviewing this .. i adressed your remarks and added them in a seperate interdiff.txt

The last submitted patch, 15: 1987720-form_test-controller-15.patch, failed testing.

ianthomas_uk’s picture

This conflicts with #2110951: Remove hook_forms() and may even be a duplicate.

xjm’s picture

Status: Needs review » Needs work

The last submitted patch, 19: 1987720-form_test-controller-19.patch, failed testing.

xjm’s picture

Issue tags: +Needs reroll
amitgoyal’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
1.5 KB

Reroll of #19.

Mile23’s picture

Issue tags: +Needs reroll
amitgoyal’s picture

Re-roll of #25.

Mile23’s picture

Status: Needs review » Closed (fixed)

form_test_wrapper_callback() went away here: http://cgit.drupalcode.org/drupal/commit/?id=466ef999aeb2f7fde41e3fcd16d...

Issue: #2110951: Remove hook_forms()

Thanks, amitgoyal!

Mile23’s picture

Status: Closed (fixed) » Closed (duplicate)