In core/modules/system/tests/modules/ajax_forms_test/ajax_forms_test.module there are two docblocks with an @return directive of unknown_type. To be consistent with the rest of core, it should be mixed. Patch to follow.

Files: 
CommentFileSizeAuthor
#10 system-return-values-1754712-9.patch685 bytesTravisCarden
PASSED: [[SimpleTest]]: [MySQL] 39,386 pass(es).
[ View ]
#5 system-return-values-1754712-5.patch785 bytespingers
PASSED: [[SimpleTest]]: [MySQL] 40,709 pass(es).
[ View ]
#2 system-return-values-1754712-2.patch796 bytesBußmeyer
PASSED: [[SimpleTest]]: [MySQL] 40,648 pass(es).
[ View ]
#1 system-return-mixed-1754712-1.patch814 bytesTravisCarden
PASSED: [[SimpleTest]]: [MySQL] 40,634 pass(es).
[ View ]

Comments

Status:Active» Needs review
Issue tags:+Novice
StatusFileSize
new814 bytes
PASSED: [[SimpleTest]]: [MySQL] 40,634 pass(es).
[ View ]

StatusFileSize
new796 bytes
PASSED: [[SimpleTest]]: [MySQL] 40,648 pass(es).
[ View ]

Hey TravisCarden,
Thank you for your patch. I have reviewed them.

function ajax_forms_test_menu() {
is an implemetation of a hook. The params and return values should not be included. See: http://drupal.org/node/1354#hookimpl

/**
* Form to display the Ajax Commands.
* @param $form
* @param $form_state
* @return undefined
*/
function ajax_forms_test_ajax_commands_form($form, &$form_state) {

This function defines a form, right? The return value is an array isn't it?

I created a new patch.

Regards,
Thomas

Status:Needs review» Needs work

Thanks both for your patches.
Bußmeyer is right, implementations of a hook don't include the params and return values.

Form generating functions don't, either (well, not for $form and $form_state or the return value), so we can have just:

/**
* Form to display the Ajax Commands.
*/
function ajax_forms_test_ajax_commands_form($form, &$form_state) {

According to http://drupal.org/node/1354#forms, you can also add @ingroup forms, which groups all forms into the Form Builder topic on api.drupal.org.

So… almost there :-)

Thank you both. @mcjim: This is just a test module. Do we @ingroup forms functions in tests?

StatusFileSize
new785 bytes
PASSED: [[SimpleTest]]: [MySQL] 40,709 pass(es).
[ View ]

From /core/modules

grep -r ingroup . | grep test
./simpletest/simpletest.pages.inc: * @ingroup themeable
./simpletest/simpletest.pages.inc: * @ingroup themeable
./simpletest/simpletest.pages.inc: * @ingroup forms
./system/tests/modules/form_test/form_test.module: * @ingroup forms
./system/tests/modules/form_test/form_test.module: * @ingroup forms
./system/tests/modules/form_test/form_test.module: * @ingroup forms
./system/tests/modules/form_test/form_test.module: * @ingroup forms
./system/tests/modules/form_test/form_test.module: * @ingroup forms
./file/tests/file_module_test.module: * @ingroup forms

Seems a bit inconsistent re: @ingroup. Re-rolled without the change.

Status:Needs work» Needs review

Doh. Test it!

Assigned:TravisCarden» Unassigned
Status:Needs review» Reviewed & tested by the community

Interesting observation about use of @ingroup, pingers.
Looking at core/modules/system/tests/form_test, the term "Form constructor" is used 10 times but "@ingroup forms" only 5. But that's an inconsistency to be discussed in another issue.

I think this is good to go.

Component:system.module» documentation
Status:Reviewed & tested by the community» Fixed

This looks right to me!

Committed and pushed to 8.x. Thanks!

Does this need backport to D7 as well?

Version:8.x-dev» 7.x-dev
Status:Fixed» Patch (to be ported)

Looks like it does need a backport. The file is
modules/simpletest/tests/ajax_forms_test.module

Status:Patch (to be ported)» Needs review
StatusFileSize
new685 bytes
PASSED: [[SimpleTest]]: [MySQL] 39,386 pass(es).
[ View ]

Status:Needs review» Reviewed & tested by the community

A straight backport of a trivial patch can be RTBC'ed once it passes automated testing, no?

Status:Reviewed & tested by the community» Fixed

Looks the same to me. :)

Committed and pushed to 7.x. Thanks!

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