The classes for the forms system make use of the Form Builder.
In classes where a call to \Drupal::formBuilder() is done frequently, it is cleaner to make it into a local variable.
If possible, when it's used over multiple testX methods, it can even be extracted into a class variable.

Noticed while working on https://www.drupal.org/node/2335673

CommentFileSizeAuthor
#6 drupal8-replace-multiple-formbuilder-calls-2362699-6.patch10.22 KBoenie
#4 drupal8-replace-multiple-formbuilder-calls-2362699-4.patch10.22 KBoenie

Comments

oenie’s picture

oenie commented
Related issues: +#2335673: Remove usage of drupal_process_form().
oenie’s picture

oenie commented
Priority: Normal » Minor
Status: Active » Postponed

I will provide a patch when #2335673: Remove usage of drupal_process_form(). goes in.

rpayanm’s picture

rpayanm
he/him
Primary language Spanish
commented
Status: Postponed » Active

3...2...1 go!

oenie’s picture

oenie commented
StatusFileSize
new drupal8-replace-multiple-formbuilder-calls-2362699-4.patch10.22 KB

Stop the clock ! :)

oenie’s picture

oenie commented

And ... nevermind ... i'm going back to the drawingboard ... i was a wee bit too optimistic ... sorry 'bout that ...

oenie’s picture

oenie commented
Status: Active » Needs review
StatusFileSize
new drupal8-replace-multiple-formbuilder-calls-2362699-6.patch10.22 KB
wheatpenny’s picture

wheatpenny commented
Issue tags: -Novice

I am removing the Novice Tag from this issue because the next steps are not clear in the issue summary. (I am working with YesCT to triage the Novice list for the Friday sprint at DrupalConLA.)

Next Steps:

1. Write an issue summary based on the template defined at https://www.drupal.org/issue-summaries. If you have Novice tasks, be sure to clearly call them out.
2. Add a beta evaluation based on https://www.drupal.org/contributor-tasks/update-allowed-beta.
3. If appropriate, add back the Novice tag.

wheatpenny’s picture

wheatpenny commented
Issue tags: +Needs issue summary update
tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented

The form builder is a stateless service, so there's no additional benefit to doing this.
I think this could just be closed.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Status: Needs review » Closed (outdated)

Simpletest-based tests should be converted to KernelTestBase or BrowserTestBase tests. These tests have their own fixture container where you can get the form_builder service as needed, without \Drupal, and also add your own mocked services to the container.

Also, as a rule of thumb, test methods shouldn't have dependencies on class properties, because then you lose the benefit of test method isolation.

And +1 on #9. Closing this issue out since it's out of date.