Part of #1971384: [META] Convert page callbacks to controllers and sub-set of #2078867: Covert all form menu items to new form builder in form_test module

Convert below form callback to new form builder.

form_test_alter_form
form_test_button_class
form_test_checkboxes_radios
form_test_checkboxes_zero
form_test_clicked_button
form_test_color
form_test_email
form_test_empty_select
form_test_form_rebuild_preserve_values_form
form_test_form_state_values_clean_advanced_form
form_test_form_state_values_clean_form
form_test_group_container
form_test_group_details
form_test_group_fieldset
form_test_group_vertical_tabs
form_test_language_select
form_test_limit_validation_errors_form
form_test_load_include_custom
form_test_load_include_menu
form_test_number
form_test_pattern_form
form_test_placeholder_test
form_test_range
form_test_range_invalid
form_test_redirect
form_test_required_attribute
form_test_select
form_test_state_persist
form_test_storage_form
form_test_url #2030165: Convert form_test_url() to a Controller Assigned to: rteijeiro
form_test_validate_form
form_test_validate_required_form
form_test_validate_required_form_no_title

For instructions on how to convert a page callback into a controller, see the WSCCI Conversion Guide.

Files: 
CommentFileSizeAuthor
#23 convert_menuitems_to_form_object-2030165-23.patch145.22 KBtkuldeep17
PASSED: [[SimpleTest]]: [MySQL] 59,210 pass(es).
[ View ]
#23 2030165-diff-23-21.txt724 bytestkuldeep17
#21 convert_menuitems_to_form_object-2030165-21.patch144.81 KBtkuldeep17
FAILED: [[SimpleTest]]: [MySQL] 58,698 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#17 2030165-diff-15-17.txt451 bytesvijaycs85
#17 2030165-form_test_controller-17.patch14.85 KBvijaycs85
FAILED: [[SimpleTest]]: [MySQL] 59,164 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#15 2030165-diff-13-15.txt4.91 KBvijaycs85
#15 2030165-form_test_controller-15.patch14.85 KBvijaycs85
FAILED: [[SimpleTest]]: [MySQL] 59,145 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#13 convert_menuitems_to_form_object-2030165-13.patch15.21 KBtkuldeep17
FAILED: [[SimpleTest]]: [MySQL] 58,931 pass(es), 112 fail(s), and 11 exception(s).
[ View ]

Comments

Title:Convert form_test_url() to a ControllerCovert all form menu items to new form builder in form_test module part 2

Assigned:rteijeiro» Unassigned
Status:Closed (won't fix)» Active

Assigned:Unassigned» mrded

@mrded, are you still working on this?

@vijaycs85 yes, it's quite a lot to do

Mrded, quite a lot of these are assigned to you. I would suggest just keeping one and post the work you have so far. It does not have to work or be perfect. Lets get it out where it can be seen and get some feedback on the approach and the details.

Assigned:mrded» Unassigned

Issue summary:View changes

updating issue with new list of functions

There is a very good chance we don't want to convert most of these, as they should become phpunit tests.
I have an example somewhere, I'll post it when I get back to my laptop.

Um, I thought I posted back here again.

Anyway, see #1987716: Provide a FormTestBase class to allow PHPUnit form tests for an example.

@tim.plunkett

How does one recognise which ones should be phpunit tests? I was hoping to pick one of these, but now confused. Happy to work on either phpunit conversion or controller conversion, but limited knowledge on deciding which one should be which? Any pointers?

For converting all these menu item into Form object, I am thinking to follow this approach

  • Choose any Testcase in form_test module from Drupal\system\Tests\Form\
  • Convert all the menu items to Form object, then test locally and submit patch.
  • By this way, Testing of Form API will become easy. we have to not run all test cases within Form API.

So I am submitting patch for Drupal\system\Tests\Form\ValidationTest.php test case. Following menu items have been converted into Form object.

form_test_limit_validation_errors_form
form_test_validate_form
form_test_validate_required_form
form_test_pattern_form

Issue summary:View changes
Status:Active» Needs review
StatusFileSize
new15.21 KB
FAILED: [[SimpleTest]]: [MySQL] 58,931 pass(es), 112 fail(s), and 11 exception(s).
[ View ]

so I am submitting patch..

Status:Needs review» Needs work

The last submitted patch, 13: convert_menuitems_to_form_object-2030165-13.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new14.85 KB
FAILED: [[SimpleTest]]: [MySQL] 59,145 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
new4.91 KB

Thanks for working on this @tkuldeep17. seems routing.yml got more update than what we have converted. Reverting them back.

The last submitted patch, 15: 2030165-form_test_controller-15.patch, failed testing.

StatusFileSize
new14.85 KB
FAILED: [[SimpleTest]]: [MySQL] 59,164 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
new451 bytes

Fixing a minor issue made on #15

The last submitted patch, 17: 2030165-form_test_controller-17.patch, failed testing.

The last submitted patch, 17: 2030165-form_test_controller-17.patch, failed testing.

Almost I have all converted menu items into Form object. Tested on my local, these are two test cases which are failing from Drupal\system\Tests\Form\ElementsTableSelectTest.. and not able to find out cause..
So please help me here..

StatusFileSize
new144.81 KB
FAILED: [[SimpleTest]]: [MySQL] 58,698 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Submitting updated patch..

Status:Needs review» Needs work

The last submitted patch, 21: convert_menuitems_to_form_object-2030165-21.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new724 bytes
new145.22 KB
PASSED: [[SimpleTest]]: [MySQL] 59,210 pass(es).
[ View ]

Fixing #21.

@vijaycs85

Regarding Name Convention: Currently if form_id was form_test_clicked_button it changed to Form object\Drupal\form_test\Form\FormTestClickedButton
but in some cases form_id was too long like form_test_form_rebuild_preserve_values_form changed to \Drupal\form_test\Form\TestRebuildPreservation.

So at somewhere Name of class generating Form is following form_id, somewhere not..
I think there should be consistency in Form naming convention.

Should I change name of class or not.?

Even after converting these menu Items into Form Object, still there are lot of menus, which has to be converted.

And also Test case classes is using Simpletest not Unittest. I am getting confusion, it will remain as Simpletest or it will be changed to Unittest.

In #9/#10, @tim.plunkett suggested that we should be replacing these tests with PHPUnit tests, rather than just shuffling round the existing tests, which is what #12-25 have been doing.

But I've seen concern from @sun on IRC that the PHPUnit tests mock too much, so are not really testing what they claim to be. I can certainly understand this view point, e.g. looking at #1987716-40: Provide a FormTestBase class to allow PHPUnit form tests there is a test testUniqueHtmlId(), but the desired behaviour is being mocked elsewhere in the patch (TestFormBuilder::drupalHtmlId()) so isn't that really just testing the mocking code.

I think we need further thought from someone familiar with PHPUnit before we continue with this work.

It depends on what is being tested. We have no full replacement for hook_element_info in PHPUnit, so anything testing the form types themselves would definitely need to be DUTB.

But in the interest of progress, porting these directly would be fine.