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.

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Task because it's updating code to the new API
Unfrozen changes Unfrozen because it's a change in automated tests
Files: 
CommentFileSizeAuthor
#27 1987612-26.patch4.18 KBaspilicious
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 85,832 pass(es).
[ View ]
#21 1987612-21.patch3.92 KBvalthebald
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 84,088 pass(es).
[ View ]
#20 1987612-20.patch3.92 KBvalthebald
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]
#18 1987612-18.patch3.91 KBvalthebald
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 84,070 pass(es).
[ View ]
#15 1987612_interdiff_11_44.txt1.51 KBvalthebald
#15 1987612_44.patch3.8 KBvalthebald
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 83,984 pass(es).
[ View ]
#14 1987612_43.patch3.69 KBvalthebald
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 84,084 pass(es), 7 fail(s), and 6 exception(s).
[ View ]
#11 1987612_11.patch2.97 KBMile23
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 83,551 pass(es), 10 fail(s), and 6 exception(s).
[ View ]
#8 system-ajax_test_dialog_contents-1987612-8.patch3.07 KBPinolo
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 70,382 pass(es), 10 fail(s), and 6 exception(s).
[ View ]
#6 system-ajax_test_dialog_contents-1987612-6.patch3.49 KBInternetDevels
PASSED: [[SimpleTest]]: [MySQL] 59,433 pass(es).
[ View ]

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

Assigned:Unassigned» mparker17

I'll help!

mparker17’s picture

Assigned:mparker17» Unassigned
Status:Active» Closed (fixed)

This appears to be already done in commit 22df596! :D

vijaycs85’s picture

Status:Closed (fixed)» Active

The commit mentioned in #4 isolated ajax_test_dialog_contents(), but it is not removing it. Still we have a controller method that calls ajax_test_dialog_contents().

InternetDevels’s picture

Issue summary:View changes
Status:Active» Needs review
StatusFileSize
new3.49 KB
PASSED: [[SimpleTest]]: [MySQL] 59,433 pass(es).
[ View ]

Let's see...

Pinolo’s picture

Assigned:Unassigned» Pinolo
Issue tags:+DrupalDays Milano 2014
Pinolo’s picture

StatusFileSize
new3.07 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 70,382 pass(es), 10 fail(s), and 6 exception(s).
[ View ]

Patch #6 doesn't apply any more. Attaching an equivalent patch that applies to latest HEAD.

Status:Needs review» Needs work

The last submitted patch, 8: system-ajax_test_dialog_contents-1987612-8.patch, failed testing.

Mile23’s picture

Issue tags:+Needs reroll
Mile23’s picture

Status:Needs work» Needs review
Issue tags:-Needs reroll
StatusFileSize
new2.97 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 83,551 pass(es), 10 fail(s), and 6 exception(s).
[ View ]

Reroll.

Status:Needs review» Needs work

The last submitted patch, 11: 1987612_11.patch, failed testing.

Pinolo’s picture

Assigned:Pinolo» Unassigned
valthebald’s picture

StatusFileSize
new3.69 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 84,084 pass(es), 7 fail(s), and 6 exception(s).
[ View ]
valthebald’s picture

Issue summary:View changes
Status:Needs work» Needs review
Issue tags:+#SprintWeekend2015
StatusFileSize
new3.8 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 83,984 pass(es).
[ View ]
new1.51 KB

There were 2 reasons for failing tests:

  1. \Drupal\ajax_test\Controller\AjaxTestController::dialogContents() must be declared as static
  2. Removed function ajax_test_dialog_contents() was called from AjaxTestDialogForm::dialog()

Both fixed, see attached patch

David Hernández’s picture

Issue tags:-#SprintWeekend2015+SprintWeekend2015

Hello!

Thank you for working on this issue!

We should all try and use the same sprint tag. According to https://groups.drupal.org/node/447258 it should be SprintWeekend2015 with no #.

Mile23’s picture

Status:Needs review» Needs work
+++ b/core/modules/system/tests/modules/ajax_test/src/Controller/AjaxTestController.php
@@ -17,9 +17,26 @@ class AjaxTestController {
   /**
    * Returns example content for dialog testing.
    */
-  public function dialogContents() {
-    // Re-use the utility method that returns the example content.
-    return ajax_test_dialog_contents();
+  public static function dialogContents() {

Add @return to the docblock, please. https://www.drupal.org/coding-standards/docs#return

valthebald’s picture

Status:Needs work» Needs review
StatusFileSize
new3.91 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 84,070 pass(es).
[ View ]

Added @return to docblock per #17

Mile23’s picture

Status:Needs review» Needs work
+++ b/core/modules/system/tests/modules/ajax_test/src/Controller/AjaxTestController.php
@@ -15,11 +15,30 @@
   /**
-   * Returns example content for dialog testing.
+   * Example content for dialog testing.
+   * @return array
+   *   Renderable array of AJAX dialog contents.
    */
-  public function dialogContents() {

Thanks... But now there has to be an empty line before the @return annotation. :-)

So like this:

  /**
   * Example content for dialog testing.
   *
   * @return array
   *   Renderable array of AJAX dialog contents.
   */
valthebald’s picture

Status:Needs work» Needs review
StatusFileSize
new3.92 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]

Here we go!

valthebald’s picture

StatusFileSize
new3.92 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 84,088 pass(es).
[ View ]

Removed extra space in separator comment line

The last submitted patch, 20: 1987612-20.patch, failed testing.

Mile23’s picture

Status:Needs review» Reviewed & tested by the community

Diggit. :-)

Refactors ajax_test_dialog_contents() into AjaxTestController::dialogContents().

The last submitted patch, 14: 1987612_43.patch, failed testing.

alexpott’s picture

Status:Reviewed & tested by the community» Needs work
Issue tags:+Needs reroll
git ac https://www.drupal.org/files/issues/1987612-21.patch
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  4012  100  4012    0     0  45828      0 --:--:-- --:--:-- --:--:-- 47200
error: patch failed: core/modules/system/tests/modules/ajax_test/src/Form/AjaxTestDialogForm.php:97
error: core/modules/system/tests/modules/ajax_test/src/Form/AjaxTestDialogForm.php: patch does not apply
aspilicious’s picture

Status:Needs work» Needs review
aspilicious’s picture

StatusFileSize
new4.18 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 85,832 pass(es).
[ View ]
aspilicious’s picture

Status:Needs review» Reviewed & tested by the community

I just rerolled this one. ANd triple checked my reroll was complete.

Wim Leers’s picture

Issue tags:-Needs reroll

Looks good!

RTBC++

alexpott’s picture

Status:Reviewed & tested by the community» Fixed

Committed fa9e185 and pushed to 8.0.x. Thanks!

Thanks for adding the beta evaluation to the issue summary.

  • alexpott committed fa9e185 on 8.0.x
    Issue #1987612 by valthebald, Mile23, aspilicious, Pinolo,...

Status:Fixed» Closed (fixed)

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