Problem/Motivation

\Drupal\system\Tests\System\AdminTest doesn't document $admin_user or $web_user and has a non-standard @file

Proposed resolution

add documentation

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

pwolanin’s picture

Status: Active » Needs review
FileSize
947 bytes

Simple doc fixes.

tim.plunkett’s picture

That's true of dozens of other tests true...

$ grep -lr "this->web_user " *
core/modules/book/lib/Drupal/book/Tests/BookTest.php
core/modules/comment/lib/Drupal/comment/Tests/CommentFieldsTest.php
core/modules/comment/lib/Drupal/comment/Tests/CommentLinksTest.php
core/modules/comment/lib/Drupal/comment/Tests/CommentNodeAccessTest.php
core/modules/comment/lib/Drupal/comment/Tests/CommentTestBase.php
core/modules/config/lib/Drupal/config/Tests/ConfigImportUITest.php
core/modules/config/lib/Drupal/config/Tests/ConfigImportUploadTest.php
core/modules/contact/lib/Drupal/contact/Tests/ContactPersonalTest.php
core/modules/field/lib/Drupal/field/Tests/Email/EmailFieldTest.php
core/modules/field/lib/Drupal/field/Tests/FieldImportDeleteUninstallUiTest.php
core/modules/field/lib/Drupal/field/Tests/Number/NumberFieldTest.php
core/modules/filter/lib/Drupal/filter/Tests/FilterAdminTest.php
core/modules/filter/lib/Drupal/filter/Tests/FilterFormatAccessTest.php
core/modules/filter/lib/Drupal/filter/Tests/FilterHtmlImageSecureTest.php
core/modules/forum/lib/Drupal/forum/Tests/ForumTest.php
core/modules/language/lib/Drupal/language/Tests/LanguageUrlRewritingTest.php
core/modules/link/lib/Drupal/link/Tests/LinkFieldTest.php
core/modules/link/lib/Drupal/link/Tests/LinkFieldUITest.php
core/modules/node/lib/Drupal/node/Tests/NodeAccessLanguageAwareCombinationTest.php
core/modules/node/lib/Drupal/node/Tests/NodeAccessLanguageAwareTest.php
core/modules/node/lib/Drupal/node/Tests/NodeAccessPagerTest.php
core/modules/node/lib/Drupal/node/Tests/NodeAccessRebuildTest.php
core/modules/node/lib/Drupal/node/Tests/NodeFormButtonsTest.php
core/modules/node/lib/Drupal/node/Tests/NodeSaveTest.php
core/modules/node/lib/Drupal/node/Tests/NodeTypeRenameConfigImportTest.php
core/modules/node/lib/Drupal/node/Tests/PageEditTest.php
core/modules/options/lib/Drupal/options/Tests/OptionsWidgetsTest.php
core/modules/path/lib/Drupal/path/Tests/PathLanguageTest.php
core/modules/system/lib/Drupal/system/Tests/Ajax/FormValuesTest.php
core/modules/system/lib/Drupal/system/Tests/Ajax/MultiFormTest.php
core/modules/system/lib/Drupal/system/Tests/Entity/EntityOperationsTest.php
core/modules/system/lib/Drupal/system/Tests/Entity/EntityRevisionsTest.php
core/modules/system/lib/Drupal/system/Tests/File/ConfigTest.php
core/modules/system/lib/Drupal/system/Tests/Form/ElementsVerticalTabsTest.php
core/modules/system/lib/Drupal/system/Tests/Form/RebuildTest.php
core/modules/system/lib/Drupal/system/Tests/Form/StorageTest.php
core/modules/system/lib/Drupal/system/Tests/Menu/BreadcrumbTest.php
core/modules/system/lib/Drupal/system/Tests/System/AdminTest.php
core/modules/system/lib/Drupal/system/Tests/System/MainContentFallbackTest.php
core/modules/text/lib/Drupal/text/Tests/TextFieldTest.php
core/modules/user/lib/Drupal/user/Tests/UserPictureTest.php
core/modules/user/lib/Drupal/user/Tests/UserSignatureTest.php
core/modules/views/lib/Drupal/views/Tests/Plugin/AccessTest.php
$ grep -lr "this->admin_user " *
core/modules/block/lib/Drupal/block/Tests/BlockCacheTest.php
core/modules/book/lib/Drupal/book/Tests/BookTest.php
core/modules/ckeditor/lib/Drupal/ckeditor/Tests/CKEditorAdminTest.php
core/modules/comment/lib/Drupal/comment/Tests/CommentBlockTest.php
core/modules/comment/lib/Drupal/comment/Tests/CommentFieldsTest.php
core/modules/comment/lib/Drupal/comment/Tests/CommentNonNodeTest.php
core/modules/comment/lib/Drupal/comment/Tests/CommentTestBase.php
core/modules/comment/lib/Drupal/comment/Tests/CommentTranslationUITest.php
core/modules/config_translation/lib/Drupal/config_translation/Tests/ConfigTranslationUiTest.php
core/modules/config_translation/lib/Drupal/config_translation/Tests/ConfigTranslationUiThemeTest.php
core/modules/contact/lib/Drupal/contact/Tests/ContactPersonalTest.php
core/modules/editor/lib/Drupal/editor/Tests/EditorAdminTest.php
core/modules/entity_reference/lib/Drupal/entity_reference/Tests/EntityReferenceFieldDefaultValueTest.php
core/modules/file/lib/Drupal/file/Tests/FileFieldTestBase.php
core/modules/file/lib/Drupal/file/Tests/FileListingTest.php
core/modules/filter/lib/Drupal/filter/Tests/FilterAdminTest.php
core/modules/filter/lib/Drupal/filter/Tests/FilterFormatAccessTest.php
core/modules/filter/lib/Drupal/filter/Tests/FilterSecurityTest.php
core/modules/forum/lib/Drupal/forum/Tests/ForumTest.php
core/modules/image/lib/Drupal/image/Tests/ImageFieldTestBase.php
core/modules/locale/lib/Drupal/locale/Tests/LocaleExportTest.php
core/modules/locale/lib/Drupal/locale/Tests/LocaleImportFunctionalTest.php
core/modules/menu_ui/lib/Drupal/menu_ui/Tests/MenuLanguageTest.php
core/modules/menu_ui/lib/Drupal/menu_ui/Tests/MenuNodeTest.php
core/modules/menu_ui/lib/Drupal/menu_ui/Tests/MenuTest.php
core/modules/node/lib/Drupal/node/Tests/NodeAccessFieldTest.php
core/modules/node/lib/Drupal/node/Tests/NodeAccessLanguageAwareCombinationTest.php
core/modules/node/lib/Drupal/node/Tests/NodeAccessLanguageAwareTest.php
core/modules/node/lib/Drupal/node/Tests/NodeAdminTest.php
core/modules/node/lib/Drupal/node/Tests/NodeFormButtonsTest.php
core/modules/node/lib/Drupal/node/Tests/NodeTitleTest.php
core/modules/node/lib/Drupal/node/Tests/PageEditTest.php
core/modules/responsive_image/lib/Drupal/responsive_image/Tests/ResponsiveImageAdminUITest.php
core/modules/responsive_image/lib/Drupal/responsive_image/Tests/ResponsiveImageFieldDisplayTest.php
core/modules/search/lib/Drupal/search/Tests/SearchCommentTest.php
core/modules/shortcut/src/Tests/ShortcutTestBase.php
core/modules/simpletest/lib/Drupal/simpletest/Tests/InstallationProfileModuleTestsTest.php
core/modules/simpletest/lib/Drupal/simpletest/Tests/OtherInstallationProfileTestsTest.php
core/modules/system/lib/Drupal/system/Tests/Common/FormatDateTest.php
core/modules/system/lib/Drupal/system/Tests/Form/ElementsVerticalTabsTest.php
core/modules/system/lib/Drupal/system/Tests/Image/ToolkitSetupFormTest.php
core/modules/system/lib/Drupal/system/Tests/Menu/BreadcrumbTest.php
core/modules/system/lib/Drupal/system/Tests/Module/ModuleTestBase.php
core/modules/system/lib/Drupal/system/Tests/Pager/PagerTest.php
core/modules/system/lib/Drupal/system/Tests/System/AccessDeniedTest.php
core/modules/system/lib/Drupal/system/Tests/System/AdminTest.php
core/modules/system/lib/Drupal/system/Tests/System/DateTimeTest.php
core/modules/system/lib/Drupal/system/Tests/System/FrontPageTest.php
core/modules/system/lib/Drupal/system/Tests/System/MainContentFallbackTest.php
core/modules/system/lib/Drupal/system/Tests/System/PageNotFoundTest.php
core/modules/system/lib/Drupal/system/Tests/System/SiteMaintenanceTest.php
core/modules/system/lib/Drupal/system/Tests/System/SystemAuthorizeTest.php
core/modules/system/lib/Drupal/system/Tests/System/ThemeTest.php
core/modules/system/lib/Drupal/system/Tests/Theme/TwigTransTest.php
core/modules/taxonomy/lib/Drupal/taxonomy/Tests/EfqTest.php
core/modules/taxonomy/lib/Drupal/taxonomy/Tests/LegacyTest.php
core/modules/taxonomy/lib/Drupal/taxonomy/Tests/RssTest.php
core/modules/taxonomy/lib/Drupal/taxonomy/Tests/TaxonomyTermIndentationTest.php
core/modules/taxonomy/lib/Drupal/taxonomy/Tests/TermIndexTest.php
core/modules/taxonomy/lib/Drupal/taxonomy/Tests/TermLanguageTest.php
core/modules/taxonomy/lib/Drupal/taxonomy/Tests/TermTest.php
core/modules/taxonomy/lib/Drupal/taxonomy/Tests/TermTranslationUITest.php
core/modules/taxonomy/lib/Drupal/taxonomy/Tests/TokenReplaceTest.php
core/modules/taxonomy/lib/Drupal/taxonomy/Tests/VocabularyLanguageTest.php
core/modules/taxonomy/lib/Drupal/taxonomy/Tests/VocabularyTest.php
core/modules/text/lib/Drupal/text/Tests/TextFieldTest.php
core/modules/toolbar/lib/Drupal/toolbar/Tests/ToolbarAdminMenuTest.php
core/modules/toolbar/lib/Drupal/toolbar/Tests/ToolbarHookToolbarTest.php
core/modules/toolbar/lib/Drupal/toolbar/Tests/ToolbarMenuTranslationTest.php
core/modules/user/lib/Drupal/user/Tests/UserCancelTest.php
core/modules/user/lib/Drupal/user/Tests/UserPermissionsTest.php
core/modules/user/lib/Drupal/user/Tests/UserRoleAdminTest.php
core/modules/user/lib/Drupal/user/Tests/UserRolesAssignmentTest.php
core/modules/user/lib/Drupal/user/Tests/UserSignatureTest.php
core/modules/user/lib/Drupal/user/Tests/UserTranslationUITest.php
core/modules/views/lib/Drupal/views/Tests/Plugin/AccessTest.php
pwolanin’s picture

@tim.plunkett - is it better to fix them in small patches when working on related code or to try to make one monster patch?

I was working on this test file for the menu links patch - putting this patch up so as to avoid including unrelated cleanup in that patch seems like the better pattern.

jhodgdon’s picture

Status: Needs review » Needs work

This patch isn't quite right if you want to fix the standards:

+++ b/core/modules/system/lib/Drupal/system/Tests/System/AdminTest.php
@@ -2,7 +2,7 @@
 
 /**
  * @file
- * Definition of Drupal\system\Tests\System\AdminTest.
+ * contains \Drupal\system\Tests\System\AdminTest.
  */

===> Contains should be capitalized.

amitgoyal’s picture

Status: Needs work » Needs review
FileSize
890 bytes

The patch in #1 was no longer getting applied on current code. So fixes for that and changes as per #4.

jhodgdon’s picture

Status: Needs review » Needs work
+ * Contains Drupal\system\Tests\System\AdminTest.

Sorry, missed this in my last review. The class name here should start with \ at the beginning of the namespace.

amitgoyal’s picture

Status: Needs work » Needs review
FileSize
891 bytes
410 bytes

Please review updated patch as per fix in #6.

jhodgdon’s picture

Status: Needs review » Needs work

Sorry again... The @var lines should not end in $variable_name. See
https://drupal.org/node/1354#var

amitgoyal’s picture

Status: Needs work » Needs review
FileSize
869 bytes
674 bytes

Sure and thanks for the feedback in #8. Please review updated patch.

jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

OK, now we're OK. Thanks!

jhodgdon’s picture

Status: Reviewed & tested by the community » Fixed

Thanks again! Committed to 8.x.

  • Commit 7ceb2d2 on 8.x by jhodgdon:
    Issue #2262247 by amitgoyal, pwolanin: Document member variables in...

Status: Fixed » Closed (fixed)

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