There are a number of tests that use Test class members with underscored names. Some examples are big_user, web_user and admin_user, but there are others. According to our coding conventions, these should be renamed to bigUser, webUser and adminUser. In addition, some properties are undefined but should be (see #2380023-4: Clean-up Comment module Test members - ensure property definition and use of camelCase naming convention).

You can use this phpcs one-liner to aid in reviewing these issues: https://gist.github.com/paul-m/bdb4c44ecd1de8f934cd

Remaining Tasks:

Open a sub-issue for each affected module. Please review or work on some of the existing issues to get them to RTBC before opening new ones.

Base classes:

Modules:

Missed or new items:

Contributor Tasks Needed (on sub-issues)

Task Novice task? Contributor instructions
Create a patch Novice Instructions
Reroll the patch if it no longer applies. Novice Instructions
Improve patch documentation or standards (for just lines changed by the patch) Novice Instructions
Manually test the patch Novice Instructions
Review patch to ensure that it fixes the issue, stays within scope, is properly documented, and follows coding standards Instructions

Hints for Contributors/Reviewers

We are attempting to fix three distinct problems with each of these subissues.

  • Problem #1 is that all class properties should follow the camelCase naming convention ($adminUser, $webUser, etc.), and many of the 'test' classes do not follow this convention (with properties such as $admin_user, $web_user, $field_name, etc. being used). The first task is therefore to rename these properties where needed.

    Finding test files with underscored properties:

    $ egrep -rl '\$this\->[a-z]+_[a-z_]+' core/modules/*/src/Tests/
  • Problem #2 is that some of these properties are not declared at all in the class declaration, and are simply referred to / assigned in the code itself. All class properties should be declared and documented with a docblock before use.

    A patch has been proposed (#2383287: Add warning about use of undeclared class properties in test classes) that adds a warning to the simpletest system about the sudden appearance of undeclared class properties in test classes, which may be helpful if you are running tests locally.

    Without that patch, you simply have to look at the code and read it carefully, bearing in mind that some properties may be declared in parent classes. Some IDEs can inspect code to find undefined properties.

  • Problem #3 is that many of these properties, if they are declared at all, do not have appropriate, any, or correct documentation/docblocks before them. See API documentation and comment standards for more information.

Do not, as part of this work, add documentation, variables, or otherwise refactor any code that you are otherwise not changing at all to accomplish the above three tasks. If you spot other problems, it is better to create a new issue for these.

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Task, because this is a coding standards change.
Issue priority Not critical because coding standard changes are not critical.
Unfrozen changes Unfrozen because it only changes automated tests.
Disruption There is no disruption expected from this sort of change.
CommentFileSizeAuthor
#9 change-big-user-name-pattern.patch1.37 KBzijing07
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Title: Clean-up Test members to use camelCase naming convention » [meta] Clean-up Test members to use camelCase naming convention

Thanks @Lars Toomre! Scoping as a meta since this would be a lot of test files. I might recommend postponing this until later in the release cycle.

Lars Toomre’s picture

Lars Toomre CreditAttribution: Lars Toomre commented

There are more admin_user and web_user members in the Test classes for the Comment module. A patch for those only should be created only after #1812034: Only use standard profile where necessary in comment.module tests (10% bot speed-up) has been committed.

kbasarab’s picture

kbasarab CreditAttribution: kbasarab commented

There are lots of files that will change by this. How should we break these up into individual tasks? I'm thinking possibly take each module and files for each and change in each module as an individual task and list those issue #'s here in the issue summary?

Noting file locations for current variable names:

big_user

/core/modules/color/lib/Drupal/color/Tests/ColorTest.php
/core/modules/dblog/lib/Drupal/dblog/Tests/DBLogTest.php
/core/modules/menu/lib/Drupal/menu/Tests/MenuTest.php

web_user:

./core/modules/aggregator/lib/Drupal/aggregator/Tests/AggregatorTestBase.php
./core/modules/book/lib/Drupal/book/Tests/BookTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentActionsTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentBlockTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentContentRebuildTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentCSSTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentFieldsTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentInterfaceTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentLinksTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentNewIndicatorTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentNodeAccessTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentNodeChangesTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentPreviewTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentRssTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentStatisticsTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentTestBase.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentThreadingTest.php
./core/modules/config/lib/Drupal/config/Tests/ConfigImportUITest.php
./core/modules/contact/lib/Drupal/contact/Tests/ContactPersonalTest.php
./core/modules/contextual/lib/Drupal/contextual/Tests/ContextualDynamicContextTest.php
./core/modules/field/lib/Drupal/field/Tests/FieldAccessTest.php
./core/modules/field/lib/Drupal/field/Tests/FormTest.php
./core/modules/field/lib/Drupal/field/Tests/TranslationTest.php
./core/modules/field/modules/email/lib/Drupal/email/Tests/EmailFieldTest.php
./core/modules/field/modules/link/lib/Drupal/link/Tests/LinkFieldTest.php
./core/modules/field/modules/link/lib/Drupal/link/Tests/LinkFieldUITest.php
./core/modules/field/modules/number/lib/Drupal/number/Tests/NumberFieldTest.php
./core/modules/field/modules/options/lib/Drupal/options/Tests/OptionsSelectDynamicValuesTest.php
./core/modules/field/modules/options/lib/Drupal/options/Tests/OptionsWidgetsTest.php
./core/modules/field/modules/text/lib/Drupal/text/Tests/TextFieldTest.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/ForumIndexTest.php
./core/modules/forum/lib/Drupal/forum/Tests/ForumTest.php
./core/modules/language/lib/Drupal/language/Tests/LanguagePathMonolingualTest.php
./core/modules/language/lib/Drupal/language/Tests/LanguageUrlRewritingTest.php
./core/modules/locale/lib/Drupal/locale/Tests/LocaleContentTest.php
./core/modules/node/lib/Drupal/node/Tests/MultiStepNodeFormBasicOptionsTest.php
./core/modules/node/lib/Drupal/node/Tests/NodeAccessLanguageTest.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/NodeAccessRecordsTest.php
./core/modules/node/lib/Drupal/node/Tests/NodeAccessTest.php
./core/modules/node/lib/Drupal/node/Tests/NodeBlockFunctionalTest.php
./core/modules/node/lib/Drupal/node/Tests/NodeCreationTest.php
./core/modules/node/lib/Drupal/node/Tests/NodeEntityViewModeAlterTest.php
./core/modules/node/lib/Drupal/node/Tests/NodeLoadMultipleTest.php
./core/modules/node/lib/Drupal/node/Tests/NodePostSettingsTest.php
./core/modules/node/lib/Drupal/node/Tests/NodeRevisionsTest.php
./core/modules/node/lib/Drupal/node/Tests/NodeSaveTest.php
./core/modules/node/lib/Drupal/node/Tests/NodeTitleXSSTest.php
./core/modules/node/lib/Drupal/node/Tests/NodeTypeInitialLanguageTest.php
./core/modules/node/lib/Drupal/node/Tests/NodeTypePersistenceTest.php
./core/modules/node/lib/Drupal/node/Tests/NodeTypeTest.php
./core/modules/node/lib/Drupal/node/Tests/PageEditTest.php
./core/modules/node/lib/Drupal/node/Tests/PagePreviewTest.php
./core/modules/node/lib/Drupal/node/Tests/PageViewTest.php
./core/modules/node/lib/Drupal/node/Tests/SummaryLengthTest.php
./core/modules/openid/lib/Drupal/openid/Tests/OpenIDFunctionalTest.php
./core/modules/openid/lib/Drupal/openid/Tests/OpenIDRegistrationTest.php
./core/modules/path/lib/Drupal/path/Tests/PathAliasTest.php
./core/modules/path/lib/Drupal/path/Tests/PathLanguageTest.php
./core/modules/path/lib/Drupal/path/Tests/PathLanguageUiTest.php
./core/modules/path/lib/Drupal/path/Tests/PathTaxonomyTermTest.php
./core/modules/php/lib/Drupal/php/Tests/PhpAccessTest.php
./core/modules/php/lib/Drupal/php/Tests/PhpFilterTest.php
./core/modules/poll/lib/Drupal/poll/Tests/PollJsAddChoiceTest.php
./core/modules/poll/lib/Drupal/poll/Tests/PollTestBase.php
./core/modules/poll/lib/Drupal/poll/Tests/PollVoteCheckHostnameTest.php
./core/modules/rdf/lib/Drupal/rdf/Tests/CommentAttributesTest.php
./core/modules/search/lib/Drupal/search/Tests/SearchPreprocessLangcodeTest.php
./core/modules/system/lib/Drupal/system/Tests/Ajax/CommandsTest.php
./core/modules/system/lib/Drupal/system/Tests/Ajax/ElementValidationTest.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/EntityFormTest.php
./core/modules/system/lib/Drupal/system/Tests/Entity/EntityTranslationFormTest.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/Menu/RouterTest.php
./core/modules/system/lib/Drupal/system/Tests/System/AdminTest.php
./core/modules/system/lib/Drupal/system/Tests/System/MainContentFallbackTest.php
./core/modules/taxonomy/lib/Drupal/taxonomy/Tests/TermFieldMultipleVocabularyTest.php
./core/modules/taxonomy/lib/Drupal/taxonomy/Tests/TermFieldTest.php
./core/modules/user/lib/Drupal/user/Tests/UserCancelTest.php
./core/modules/user/lib/Drupal/user/Tests/UserLanguageTest.php
./core/modules/user/lib/Drupal/user/Tests/UserSignatureTest.php
./core/modules/user/lib/Drupal/user/Tests/UserTimeZoneTest.php
./core/modules/views/lib/Drupal/views/Tests/Plugin/AccessTest.php

admin_user

./core/modules/aggregator/lib/Drupal/aggregator/Tests/AggregatorRenderingTest.php
./core/modules/ban/lib/Drupal/ban/Tests/IpAddressBlockingTest.php
./core/modules/block/lib/Drupal/block/Tests/BlockAdminThemeTest.php
./core/modules/block/lib/Drupal/block/Tests/BlockCacheTest.php
./core/modules/block/lib/Drupal/block/Tests/BlockHiddenRegionTest.php
./core/modules/block/lib/Drupal/block/Tests/BlockHtmlIdTest.php
./core/modules/block/lib/Drupal/block/Tests/BlockInvalidRegionTest.php
./core/modules/block/lib/Drupal/block/Tests/BlockLanguageTest.php
./core/modules/block/lib/Drupal/block/Tests/BlockTest.php
./core/modules/block/lib/Drupal/block/Tests/BlockUserAccountSettingsTest.php
./core/modules/block/lib/Drupal/block/Tests/NewDefaultThemeBlocksTest.php
./core/modules/block/lib/Drupal/block/Tests/NonDefaultBlockAdminTest.php
./core/modules/book/lib/Drupal/book/Tests/BookTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentAnonymousTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentApprovalTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentBlockTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentContentRebuildTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentCSSTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentFieldsTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentInterfaceTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentLanguageTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentNewIndicatorTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentNodeAccessTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentPagerTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentPreviewTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentStatisticsTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentTestBase.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentThreadingTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentTokenReplaceTest.php
./core/modules/contact/lib/Drupal/contact/Tests/ContactPersonalTest.php
./core/modules/contact/lib/Drupal/contact/Tests/ContactSitewideTest.php
./core/modules/field/modules/options/lib/Drupal/options/Tests/OptionsFieldUITest.php
./core/modules/field/modules/options/lib/Drupal/options/Tests/OptionsWidgetsTest.php
./core/modules/field/modules/text/lib/Drupal/text/Tests/TextFieldTest.php
./core/modules/field_ui/lib/Drupal/field_ui/Tests/AlterTest.php
./core/modules/field_ui/lib/Drupal/field_ui/Tests/FieldUiTestBase.php
./core/modules/file/lib/Drupal/file/Tests/FileFieldPathTest.php
./core/modules/file/lib/Drupal/file/Tests/FileFieldTestBase.php
./core/modules/file/lib/Drupal/file/Tests/FileFieldWidgetTest.php
./core/modules/file/lib/Drupal/file/Tests/FilePrivateTest.php
./core/modules/file/lib/Drupal/file/Tests/FileTokenReplaceTest.php
./core/modules/filter/lib/Drupal/filter/Tests/FilterAdminTest.php
./core/modules/filter/lib/Drupal/filter/Tests/FilterDefaultFormatTest.php
./core/modules/filter/lib/Drupal/filter/Tests/FilterFormatAccessTest.php
./core/modules/filter/lib/Drupal/filter/Tests/FilterHooksTest.php
./core/modules/filter/lib/Drupal/filter/Tests/FilterSecurityTest.php
./core/modules/forum/lib/Drupal/forum/Tests/ForumNodeAccessTest.php
./core/modules/forum/lib/Drupal/forum/Tests/ForumTest.php
./core/modules/image/lib/Drupal/image/Tests/ImageFieldDisplayTest.php
./core/modules/image/lib/Drupal/image/Tests/ImageFieldTestBase.php
./core/modules/language/lib/Drupal/language/Tests/LanguageBrowserDetectionUnitTest.php
./core/modules/language/lib/Drupal/language/Tests/LanguageConfigurationElementTest.php
./core/modules/language/lib/Drupal/language/Tests/LanguageConfigurationTest.php
./core/modules/language/lib/Drupal/language/Tests/LanguageCustomLanguageConfigurationTest.php
./core/modules/language/lib/Drupal/language/Tests/LanguageListTest.php
./core/modules/language/lib/Drupal/language/Tests/LanguageNegotiationInfoTest.php
./core/modules/language/lib/Drupal/language/Tests/LanguageSwitchingTest.php
./core/modules/language/lib/Drupal/language/Tests/LanguageUILanguageNegotiationTest.php
./core/modules/locale/lib/Drupal/locale/Tests/LocaleCompareTest.php
./core/modules/locale/lib/Drupal/locale/Tests/LocaleContentTest.php
./core/modules/locale/lib/Drupal/locale/Tests/LocaleExportTest.php
./core/modules/locale/lib/Drupal/locale/Tests/LocaleFileImportStatus.php
./core/modules/locale/lib/Drupal/locale/Tests/LocaleImportFunctionalTest.php
./core/modules/locale/lib/Drupal/locale/Tests/LocalePathTest.php
./core/modules/locale/lib/Drupal/locale/Tests/LocalePluralFormatTest.php
./core/modules/locale/lib/Drupal/locale/Tests/LocaleTranslationTest.php
./core/modules/menu/lib/Drupal/menu/Tests/MenuNodeTest.php
./core/modules/node/lib/Drupal/node/Tests/NodeAccessFieldTest.php
./core/modules/node/lib/Drupal/node/Tests/NodeAdminTest.php
./core/modules/node/lib/Drupal/node/Tests/NodeBlockFunctionalTest.php
./core/modules/node/lib/Drupal/node/Tests/NodeBlockTest.php
./core/modules/node/lib/Drupal/node/Tests/NodeFieldMultilingualTestCase.php
./core/modules/node/lib/Drupal/node/Tests/NodeTitleTest.php
./core/modules/node/lib/Drupal/node/Tests/PageEditTest.php
./core/modules/openid/lib/Drupal/openid/Tests/OpenIDFunctionalTest.php
./core/modules/php/lib/Drupal/php/Tests/PhpTestBase.php
./core/modules/picture/lib/Drupal/picture/Tests/PictureAdminUITest.php
./core/modules/picture/lib/Drupal/picture/Tests/PictureFieldDisplayTest.php
./core/modules/poll/lib/Drupal/poll/Tests/PollBlockTest.php
./core/modules/poll/lib/Drupal/poll/Tests/PollTestBase.php
./core/modules/poll/lib/Drupal/poll/Tests/PollTranslateTest.php
./core/modules/poll/lib/Drupal/poll/Tests/PollVoteCheckHostnameTest.php
./core/modules/rdf/lib/Drupal/rdf/Tests/CommentAttributesTest.php
./core/modules/rdf/lib/Drupal/rdf/Tests/RdfaMarkupTest.php
./core/modules/search/lib/Drupal/search/Tests/SearchBlockTest.php
./core/modules/search/lib/Drupal/search/Tests/SearchCommentTest.php
./core/modules/shortcut/lib/Drupal/shortcut/Tests/ShortcutSetsTest.php
./core/modules/shortcut/lib/Drupal/shortcut/Tests/ShortcutTestBase.php
./core/modules/simpletest/lib/Drupal/simpletest/Tests/BrokenSetUpTest.php
./core/modules/simpletest/lib/Drupal/simpletest/Tests/InstallationProfileModuleTestsTest.php
./core/modules/simpletest/lib/Drupal/simpletest/Tests/MissingCheckedRequirementsTest.php
./core/modules/simpletest/lib/Drupal/simpletest/Tests/OtherInstallationProfileModuleTestsTest.php
./core/modules/syslog/lib/Drupal/syslog/Tests/SyslogTest.php
./core/modules/system/lib/Drupal/system/Tests/Batch/PageTest.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/Menu/BreadcrumbTest.php
./core/modules/system/lib/Drupal/system/Tests/Menu/RouterTest.php
./core/modules/system/lib/Drupal/system/Tests/Menu/TrailTest.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/CronRunTest.php
./core/modules/system/lib/Drupal/system/Tests/System/DateFormatsLanguageTest.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/Update/UpdateScriptTest.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/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/ThemeTest.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/taxonomy/lib/Drupal/taxonomy/Tests/VocabularyUnitTest.php
./core/modules/tracker/lib/Drupal/tracker/Tests/TrackerTest.php
./core/modules/translation/lib/Drupal/translation/Tests/TranslationTest.php
./core/modules/update/lib/Drupal/update/Tests/UpdateContribTest.php
./core/modules/update/lib/Drupal/update/Tests/UpdateCoreTest.php
./core/modules/update/lib/Drupal/update/Tests/UpdateUploadTest.php
./core/modules/user/lib/Drupal/user/Tests/UserAdminTest.php
./core/modules/user/lib/Drupal/user/Tests/UserCancelTest.php
./core/modules/user/lib/Drupal/user/Tests/UserLanguageCreationTest.php
./core/modules/user/lib/Drupal/user/Tests/UserLanguageTest.php
./core/modules/user/lib/Drupal/user/Tests/UserPermissionsTest.php
./core/modules/user/lib/Drupal/user/Tests/UserRegistrationTest.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/views/lib/Drupal/views/Tests/Handler/AreaTest.php
./core/modules/views/lib/Drupal/views/Tests/Handler/HandlerTest.php
./core/modules/views/lib/Drupal/views/Tests/Plugin/AccessTest.php
./core/modules/views/lib/Drupal/views/Tests/Plugin/ArgumentDefaultTest.php
./core/modules/views/lib/Drupal/views/Tests/Plugin/DisplayFeedTest.php
./core/modules/views/lib/Drupal/views/Tests/Plugin/ExposedFormTest.php
./core/modules/views/lib/Drupal/views/Tests/Plugin/PagerTest.php

Lars Toomre’s picture

Lars Toomre CreditAttribution: Lars Toomre commented

This is a much bigger list than I originally recall. Be careful here on what needs to be changed.

I think you need to grep for instance for '->big_user' and '->admin_user' to catch those instances which need to be changed.

kbasarab’s picture

kbasarab CreditAttribution: kbasarab commented

#3 was grepped with 'web_user', 'admin_user' and 'big_user'. You are right Lars. In changing for '\->admin_user' the list does get shorter. Basically cutting it in half.

grep -rl "\->web_user" ./*

->web_user

./core/modules/book/lib/Drupal/book/Tests/BookTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentActionsTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentBlockTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentContentRebuildTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentCSSTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentFieldsTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentInterfaceTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentLinksTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentNewIndicatorTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentNodeAccessTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentNodeChangesTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentPreviewTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentRssTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentStatisticsTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentTestBase.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentThreadingTest.php
./core/modules/config/lib/Drupal/config/Tests/ConfigImportUITest.php
./core/modules/contact/lib/Drupal/contact/Tests/ContactPersonalTest.php
./core/modules/field/modules/email/lib/Drupal/email/Tests/EmailFieldTest.php
./core/modules/field/modules/link/lib/Drupal/link/Tests/LinkFieldTest.php
./core/modules/field/modules/link/lib/Drupal/link/Tests/LinkFieldUITest.php
./core/modules/field/modules/number/lib/Drupal/number/Tests/NumberFieldTest.php
./core/modules/field/modules/options/lib/Drupal/options/Tests/OptionsWidgetsTest.php
./core/modules/field/modules/text/lib/Drupal/text/Tests/TextFieldTest.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/node/lib/Drupal/node/Tests/NodeAccessPagerTest.php
./core/modules/node/lib/Drupal/node/Tests/NodeAccessRebuildTest.php
./core/modules/node/lib/Drupal/node/Tests/NodeBlockFunctionalTest.php
./core/modules/node/lib/Drupal/node/Tests/NodeSaveTest.php
./core/modules/node/lib/Drupal/node/Tests/PageEditTest.php
./core/modules/openid/lib/Drupal/openid/Tests/OpenIDFunctionalTest.php
./core/modules/path/lib/Drupal/path/Tests/PathLanguageTest.php
./core/modules/poll/lib/Drupal/poll/Tests/PollVoteCheckHostnameTest.php
./core/modules/rdf/lib/Drupal/rdf/Tests/CommentAttributesTest.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/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/user/lib/Drupal/user/Tests/UserSignatureTest.php
./core/modules/views/lib/Drupal/views/Tests/Plugin/AccessTest.php

grep -rl "\->admin_user" ./*

->admin_user

./core/modules/aggregator/lib/Drupal/aggregator/Tests/AggregatorRenderingTest.php
./core/modules/block/lib/Drupal/block/Tests/BlockCacheTest.php
./core/modules/block/lib/Drupal/block/Tests/BlockHtmlIdTest.php
./core/modules/block/lib/Drupal/block/Tests/BlockTest.php
./core/modules/book/lib/Drupal/book/Tests/BookTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentAnonymousTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentApprovalTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentBlockTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentContentRebuildTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentCSSTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentFieldsTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentInterfaceTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentNewIndicatorTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentNodeAccessTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentPagerTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentPreviewTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentStatisticsTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentTestBase.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentThreadingTest.php
./core/modules/comment/lib/Drupal/comment/Tests/CommentTokenReplaceTest.php
./core/modules/contact/lib/Drupal/contact/Tests/ContactPersonalTest.php
./core/modules/field/modules/text/lib/Drupal/text/Tests/TextFieldTest.php
./core/modules/file/lib/Drupal/file/Tests/FileFieldPathTest.php
./core/modules/file/lib/Drupal/file/Tests/FileFieldTestBase.php
./core/modules/file/lib/Drupal/file/Tests/FileFieldWidgetTest.php
./core/modules/file/lib/Drupal/file/Tests/FilePrivateTest.php
./core/modules/file/lib/Drupal/file/Tests/FileTokenReplaceTest.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/ImageFieldDisplayTest.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/lib/Drupal/menu/Tests/MenuNodeTest.php
./core/modules/node/lib/Drupal/node/Tests/NodeAccessFieldTest.php
./core/modules/node/lib/Drupal/node/Tests/NodeAdminTest.php
./core/modules/node/lib/Drupal/node/Tests/NodeBlockFunctionalTest.php
./core/modules/node/lib/Drupal/node/Tests/NodeTitleTest.php
./core/modules/node/lib/Drupal/node/Tests/PageEditTest.php
./core/modules/picture/lib/Drupal/picture/Tests/PictureAdminUITest.php
./core/modules/picture/lib/Drupal/picture/Tests/PictureFieldDisplayTest.php
./core/modules/poll/lib/Drupal/poll/Tests/PollVoteCheckHostnameTest.php
./core/modules/rdf/lib/Drupal/rdf/Tests/CommentAttributesTest.php
./core/modules/search/lib/Drupal/search/Tests/SearchCommentTest.php
./core/modules/shortcut/lib/Drupal/shortcut/Tests/ShortcutSetsTest.php
./core/modules/shortcut/lib/Drupal/shortcut/Tests/ShortcutTestBase.php
./core/modules/simpletest/lib/Drupal/simpletest/Tests/InstallationProfileModuleTestsTest.php
./core/modules/simpletest/lib/Drupal/simpletest/Tests/OtherInstallationProfileModuleTestsTest.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/Menu/BreadcrumbTest.php
./core/modules/system/lib/Drupal/system/Tests/Menu/TrailTest.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/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/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/TokenReplaceTest.php
./core/modules/taxonomy/lib/Drupal/taxonomy/Tests/VocabularyLanguageTest.php
./core/modules/taxonomy/lib/Drupal/taxonomy/Tests/VocabularyTest.php
./core/modules/translation/lib/Drupal/translation/Tests/TranslationTest.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/views/lib/Drupal/views/Tests/Plugin/AccessTest.php

grep -rl "\->big_user" ./*

->big_user

./core/modules/color/lib/Drupal/color/Tests/ColorTest.php
./core/modules/dblog/lib/Drupal/dblog/Tests/DBLogTest.php
./core/modules/menu/lib/Drupal/menu/Tests/MenuTest.php

Lars Toomre’s picture

Lars Toomre CreditAttribution: Lars Toomre commented

From looking at these before, I suspect the three big_user cases can be changed to use a class property 'adminUser'. If so, please do as it is much more politically correct.

Lars Toomre’s picture

Lars Toomre CreditAttribution: Lars Toomre commented

As a first guess, I would suggest breaking this issue up into a series of patches that probably can be worked through in this meta issue. As a start, I would suggest trying to create incremental patches that address 25-50 changes. I am happy to review what patches are submitted.

Perhaps such an incremental patch might include all user or taxonomy Test classes? And then some reasonable grouping of what remains?

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Status: Active » Postponed

Yes, break it up logically into 25-50 changes -- sounds good.

But let's postpone until after feature freeze. We've been asked not to do massive clean-up patches until then. Thanks!

zijing07’s picture

zijing07 CreditAttribution: zijing07 commented
Issue summary: View changes
FileSize
change-big-user-name-pattern.patch1.37 KB

Just change the big_user name pattern in ColorTest.php

cilefen’s picture

cilefen CreditAttribution: cilefen commented
Status: Postponed » Needs review
cilefen’s picture

cilefen CreditAttribution: cilefen commented
Status: Needs review » Needs work

@zijing07 This is a meta issue. We must open sub-issues to address each section of tests.

cilefen’s picture

cilefen CreditAttribution: cilefen commented
Issue summary: View changes

@zijing07: please post the color module patch here: #2379863: Clean-up color module test members - ensure property definition and use of camelCase naming convention

tibbsa’s picture

tibbsa CreditAttribution: tibbsa commented
Issue summary: View changes
GPrince17’s picture

GPrince17 CreditAttribution: GPrince17 commented

It's been two years since this list was updated. Here is an updated list of instances that need correction.

Files using web_user: (command: grep -rl "\->web_user" ./*)

./core/modules/book/src/Tests/BookTest.php
./core/modules/comment/src/Tests/CommentActionsTest.php
./core/modules/comment/src/Tests/CommentBlockTest.php
./core/modules/comment/src/Tests/CommentCSSTest.php
./core/modules/comment/src/Tests/CommentFieldsTest.php
./core/modules/comment/src/Tests/CommentInterfaceTest.php
./core/modules/comment/src/Tests/CommentLinksAlterTest.php
./core/modules/comment/src/Tests/CommentLinksTest.php
./core/modules/comment/src/Tests/CommentNewIndicatorTest.php
./core/modules/comment/src/Tests/CommentNodeAccessTest.php
./core/modules/comment/src/Tests/CommentNodeChangesTest.php
./core/modules/comment/src/Tests/CommentPagerTest.php
./core/modules/comment/src/Tests/CommentPreviewTest.php
./core/modules/comment/src/Tests/CommentRssTest.php
./core/modules/comment/src/Tests/CommentStatisticsTest.php
./core/modules/comment/src/Tests/CommentTestBase.php
./core/modules/comment/src/Tests/CommentThreadingTest.php
./core/modules/comment/src/Tests/CommentTitleTest.php
./core/modules/config/src/Tests/ConfigImportAllTest.php
./core/modules/config/src/Tests/ConfigImportUITest.php
./core/modules/config/src/Tests/ConfigImportUploadTest.php
./core/modules/contact/src/Tests/ContactPersonalTest.php
./core/modules/field/src/Tests/Boolean/BooleanFieldTest.php
./core/modules/field/src/Tests/Email/EmailFieldTest.php
./core/modules/field/src/Tests/FieldImportDeleteUninstallUiTest.php
./core/modules/field/src/Tests/Number/NumberFieldTest.php
./core/modules/field/src/Tests/String/StringFieldTest.php
./core/modules/filter/src/Tests/FilterAdminTest.php
./core/modules/filter/src/Tests/FilterFormatAccessTest.php
./core/modules/filter/src/Tests/FilterHtmlImageSecureTest.php
./core/modules/forum/src/Tests/ForumTest.php
./core/modules/language/src/Tests/LanguageUrlRewritingTest.php
./core/modules/link/src/Tests/LinkFieldTest.php
./core/modules/link/src/Tests/LinkFieldUITest.php
./core/modules/node/src/Tests/NodeAccessLanguageAwareCombinationTest.php
./core/modules/node/src/Tests/NodeAccessLanguageAwareTest.php
./core/modules/node/src/Tests/NodeAccessPagerTest.php
./core/modules/node/src/Tests/NodeAccessRebuildTest.php
./core/modules/node/src/Tests/NodeFormButtonsTest.php
./core/modules/node/src/Tests/NodeSaveTest.php
./core/modules/node/src/Tests/NodeTypeRenameConfigImportTest.php
./core/modules/node/src/Tests/PageEditTest.php
./core/modules/options/src/Tests/OptionsWidgetsTest.php
./core/modules/path/src/Tests/PathLanguageTest.php
./core/modules/rdf/src/Tests/CommentAttributesTest.php
./core/modules/system/src/Tests/Ajax/FormValuesTest.php
./core/modules/system/src/Tests/Ajax/MultiFormTest.php
./core/modules/system/src/Tests/Entity/EntityOperationsTest.php
./core/modules/system/src/Tests/Entity/EntityRevisionsTest.php
./core/modules/system/src/Tests/File/ConfigTest.php
./core/modules/system/src/Tests/Form/ElementsVerticalTabsTest.php
./core/modules/system/src/Tests/Form/RebuildTest.php
./core/modules/system/src/Tests/Form/StorageTest.php
./core/modules/system/src/Tests/Menu/BreadcrumbTest.php
./core/modules/system/src/Tests/System/AdminTest.php
./core/modules/system/src/Tests/System/MainContentFallbackTest.php
./core/modules/text/src/Tests/TextFieldTest.php
./core/modules/user/src/Tests/UserPictureTest.php
./core/modules/user/src/Tests/UserSignatureTest.php
./core/modules/views/src/Tests/Plugin/AccessTest.php

files using admin_user: (command: grep -rl "\->admin_user" ./*)

./core/modules/block/src/Tests/BlockCacheTest.php
./core/modules/book/src/Tests/BookTest.php
./core/modules/ckeditor/src/Tests/CKEditorAdminTest.php
./core/modules/comment/src/Tests/CommentActionsTest.php
./core/modules/comment/src/Tests/CommentAdminTest.php
./core/modules/comment/src/Tests/CommentAnonymousTest.php
./core/modules/comment/src/Tests/CommentBlockTest.php
./core/modules/comment/src/Tests/CommentCSSTest.php
./core/modules/comment/src/Tests/CommentFieldsTest.php
./core/modules/comment/src/Tests/CommentInterfaceTest.php
./core/modules/comment/src/Tests/CommentNewIndicatorTest.php
./core/modules/comment/src/Tests/CommentNodeAccessTest.php
./core/modules/comment/src/Tests/CommentNonNodeTest.php
./core/modules/comment/src/Tests/CommentPagerTest.php
./core/modules/comment/src/Tests/CommentPreviewTest.php
./core/modules/comment/src/Tests/CommentStatisticsTest.php
./core/modules/comment/src/Tests/CommentTestBase.php
./core/modules/comment/src/Tests/CommentThreadingTest.php
./core/modules/comment/src/Tests/CommentTokenReplaceTest.php
./core/modules/comment/src/Tests/CommentTranslationUITest.php
./core/modules/config_translation/src/Tests/ConfigTranslationUiTest.php
./core/modules/config_translation/src/Tests/ConfigTranslationUiThemeTest.php
./core/modules/contact/src/Tests/ContactPersonalTest.php
./core/modules/editor/src/Tests/EditorAdminTest.php
./core/modules/entity_reference/src/Tests/EntityReferenceFieldDefaultValueTest.php
./core/modules/file/src/Tests/FileFieldPathTest.php
./core/modules/file/src/Tests/FileFieldTestBase.php
./core/modules/file/src/Tests/FileFieldWidgetTest.php
./core/modules/file/src/Tests/FileListingTest.php
./core/modules/file/src/Tests/FilePrivateTest.php
./core/modules/file/src/Tests/FileTokenReplaceTest.php
./core/modules/filter/src/Tests/FilterAdminTest.php
./core/modules/filter/src/Tests/FilterFormatAccessTest.php
./core/modules/filter/src/Tests/FilterSecurityTest.php
./core/modules/forum/src/Tests/ForumTest.php
./core/modules/image/src/Tests/ImageFieldDisplayTest.php
./core/modules/image/src/Tests/ImageFieldTestBase.php
./core/modules/menu_ui/src/Tests/MenuLanguageTest.php
./core/modules/menu_ui/src/Tests/MenuNodeTest.php
./core/modules/menu_ui/src/Tests/MenuTest.php
./core/modules/node/src/Tests/NodeAccessFieldTest.php
./core/modules/node/src/Tests/NodeAccessLanguageAwareCombinationTest.php
./core/modules/node/src/Tests/NodeAccessLanguageAwareTest.php
./core/modules/node/src/Tests/NodeAdminTest.php
./core/modules/node/src/Tests/NodeFormButtonsTest.php
./core/modules/node/src/Tests/NodeTitleTest.php
./core/modules/node/src/Tests/PageEditTest.php
./core/modules/responsive_image/src/Tests/ResponsiveImageAdminUITest.php
./core/modules/responsive_image/src/Tests/ResponsiveImageFieldDisplayTest.php
./core/modules/search/src/Tests/SearchCommentTest.php
./core/modules/shortcut/src/Tests/ShortcutSetsTest.php
./core/modules/shortcut/src/Tests/ShortcutTestBase.php
./core/modules/simpletest/src/Tests/InstallationProfileModuleTestsTest.php
./core/modules/simpletest/src/Tests/OtherInstallationProfileTestsTest.php
./core/modules/system/src/Tests/Common/FormatDateTest.php
./core/modules/system/src/Tests/Form/ElementsVerticalTabsTest.php
./core/modules/system/src/Tests/Image/ToolkitSetupFormTest.php
./core/modules/system/src/Tests/Menu/BreadcrumbTest.php
./core/modules/system/src/Tests/Module/ModuleTestBase.php
./core/modules/system/src/Tests/Pager/PagerTest.php
./core/modules/system/src/Tests/System/AccessDeniedTest.php
./core/modules/system/src/Tests/System/AdminTest.php
./core/modules/system/src/Tests/System/DateTimeTest.php
./core/modules/system/src/Tests/System/FrontPageTest.php
./core/modules/system/src/Tests/System/MainContentFallbackTest.php
./core/modules/system/src/Tests/System/PageNotFoundTest.php
./core/modules/system/src/Tests/System/SiteMaintenanceTest.php
./core/modules/system/src/Tests/System/SystemAuthorizeTest.php
./core/modules/system/src/Tests/System/ThemeTest.php
./core/modules/system/src/Tests/Theme/TwigTransTest.php
./core/modules/taxonomy/src/Tests/EfqTest.php
./core/modules/taxonomy/src/Tests/LegacyTest.php
./core/modules/taxonomy/src/Tests/RssTest.php
./core/modules/taxonomy/src/Tests/TaxonomyTermIndentationTest.php
./core/modules/taxonomy/src/Tests/TermIndexTest.php
./core/modules/taxonomy/src/Tests/TermLanguageTest.php
./core/modules/taxonomy/src/Tests/TermTest.php
./core/modules/taxonomy/src/Tests/TermTranslationUITest.php
./core/modules/taxonomy/src/Tests/TokenReplaceTest.php
./core/modules/taxonomy/src/Tests/Views/TaxonomyTermViewTest.php
./core/modules/taxonomy/src/Tests/VocabularyLanguageTest.php
./core/modules/taxonomy/src/Tests/VocabularyUiTest.php
./core/modules/text/src/Tests/TextFieldTest.php
./core/modules/toolbar/src/Tests/ToolbarAdminMenuTest.php
./core/modules/toolbar/src/Tests/ToolbarHookToolbarTest.php
./core/modules/toolbar/src/Tests/ToolbarMenuTranslationTest.php
./core/modules/user/src/Tests/UserCancelTest.php
./core/modules/user/src/Tests/UserPermissionsTest.php
./core/modules/user/src/Tests/UserRoleAdminTest.php
./core/modules/user/src/Tests/UserRolesAssignmentTest.php
./core/modules/user/src/Tests/UserSignatureTest.php
./core/modules/views/src/Tests/Plugin/AccessTest.php

files using big_user: (command: grep -rl "\->big_user" ./*)

./core/modules/color/src/Tests/ColorTest.php
./core/modules/dblog/src/Tests/DbLogTest.php

GPrince17’s picture

GPrince17 CreditAttribution: GPrince17 commented
Issue summary: View changes
markat’s picture

markat CreditAttribution: markat commented
Issue summary: View changes
cilefen’s picture

cilefen CreditAttribution: cilefen commented
Issue summary: View changes
cilefen’s picture

cilefen CreditAttribution: cilefen commented
Issue summary: View changes
tibbsa’s picture

tibbsa CreditAttribution: tibbsa commented

Comment module changes are ready for review in #2380023: Clean-up Comment module Test members - ensure property definition and use of camelCase naming convention.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Issue summary: View changes

If we are going to do this issue we should also add any undefined property definitions too. See #2380023-4: Clean-up Comment module Test members - ensure property definition and use of camelCase naming convention

cilefen’s picture

cilefen CreditAttribution: cilefen commented
Issue summary: View changes
tibbsa’s picture

tibbsa CreditAttribution: tibbsa commented
Title: [meta] Clean-up Test members to use camelCase naming convention » [meta] Clean-up Test members - ensure property definition and use of camelCase naming convention

Issue title change to reflect expanded scope.

cilefen’s picture

cilefen CreditAttribution: cilefen commented
Issue summary: View changes
cilefen’s picture

cilefen CreditAttribution: cilefen commented
Issue summary: View changes
cilefen’s picture

cilefen CreditAttribution: cilefen commented
Issue summary: View changes
cilefen’s picture

cilefen CreditAttribution: cilefen commented
Issue summary: View changes
tibbsa’s picture

tibbsa CreditAttribution: tibbsa commented

One caution: Be careful about changing these when more than one level of indirection is present. For example, situations like $this->comment->comment_body. The comment_body variable will have been caught by the grep, but comment_body arises because of the entity model for comments, and is set that way by CommentManager. Changing that convention would have far-flung consequences.

Put another way, I assume we should limit changes to the Test classes themselves and not to follow this cleanup through to things outside the scope of the tests.

This type of inconsistency appears to have been noted (#2335287: [policy, no patch] Class properties don't always follow coding standards), but the conclusion there seemed to be that since comment_body is an entity field and not an "actual property" it was exempt from the coding standards. That's a distinction that isn't necessarily obvious when you look at this in code form though.

cilefen’s picture

cilefen CreditAttribution: cilefen commented

@tibbsa You are correct about the levels. We need a better egrep expression.

cilefen’s picture

cilefen CreditAttribution: cilefen commented

@tibbsa You are correct about the levels. We need a better egrep expression.

cilefen’s picture

cilefen CreditAttribution: cilefen commented
Issue summary: View changes
tibbsa’s picture

tibbsa CreditAttribution: tibbsa commented
Issue summary: View changes

Created RDF sub-issue: #2381921: Clean-up RDF module test members - ensure property definition and use of camelCase naming convention

tibbsa’s picture

tibbsa CreditAttribution: tibbsa commented
Issue summary: View changes

My suggestion is that the cleanup of the 'simpletest' module itself be done last, once everything else is in -- or at least at a point where there are no uncommitted sub-issues under development.

'Simpletest' provides TestBase, WebTestBase, etc., and changes made to these base classes will need to be carried through a great number of these derived classes. If we proceed with these patches in parallel, then all (or a great number) of the individual sub-issues may need to be re-rolled later. For example, $this->root_user, undocumented but set in WebTestBase::setUp(), is used in block, config, field_ui, language, locale, node, rdf, shortcut, simpletest, system, and views.

rpayanm’s picture

rpayanm
he/him
Primary language Spanish
CreditAttribution: rpayanm commented

+1 to #32 the child issues would be postponed until #2382195: Clean-up simpletest module test members - ensure property definition and use of camelCase naming convention.

Other question: root_user where is defined? because I can't find it.

tibbsa’s picture

tibbsa CreditAttribution: tibbsa commented

@raypanm, you're suggesting the opposite direction from what I was thinking (i.e. do simpletest first, re-roll what's been done to date, then continue). That can be done as it's unlikely that the re-rolling will be terribly complicated for anyone to do, but let's be clear as to whether we're (a) tackling 'simpletest' first (and the many changes that will make), or (b) tackling all the submodules first, and then tackling 'simpletest' itself as the last item of business.

As for WebTestBase::$root_user, look at core/modules/simpletest/src/WebTestBase.php, function setUp() (around line 762):

    // Define information about the user 1 account.
    $this->root_user = new UserSession(array(
      'uid' => 1,
      'name' => 'admin',
      'mail' => 'admin@example.com',
      'pass_raw' => $this->randomMachineName(),
    ));

Aside from needing camelCasing, this is one of the many instances where properties are not even declared prior to use, and therefore don't make it to the documentation, etc.

cilefen’s picture

cilefen CreditAttribution: cilefen commented
Issue summary: View changes
cilefen’s picture

cilefen CreditAttribution: cilefen commented

#2382765: Clean-up WebTestBase class members - ensure property definition and use of camelCase naming convention

tibbsa’s picture

tibbsa CreditAttribution: tibbsa commented
Issue summary: View changes
tibbsa’s picture

tibbsa CreditAttribution: tibbsa commented

@cilefen: Note the discussion beginning at #2381303-6: Clean-up CKEditor module test members — ensure property definition and use of camelCase naming convention. There seems to be some debate as to what is "in scope" and not on these changes. I can edit the issue summary to try to make this more clear, but I didn't think this was open to too many different interpretations as it is? That issue has been marked RTBC (again) notwithstanding some class properties which are not declared or documented prior to use.

tibbsa’s picture

tibbsa CreditAttribution: tibbsa commented
Issue summary: View changes
tibbsa’s picture

tibbsa CreditAttribution: tibbsa commented
Issue summary: View changes

#2384689: Clean-up dblog module test members - ensure property definition and use of camelCase naming convention

cilefen’s picture

cilefen CreditAttribution: cilefen commented
Issue summary: View changes
cilefen’s picture

cilefen CreditAttribution: cilefen commented
Issue summary: View changes
cilefen’s picture

cilefen CreditAttribution: cilefen commented
Issue summary: View changes
tibbsa’s picture

tibbsa CreditAttribution: tibbsa commented
Issue summary: View changes

Marking contextual module 'completed' (webchick committed to 8.0.x).

tibbsa’s picture

tibbsa CreditAttribution: tibbsa commented
Issue summary: View changes
cilefen’s picture

cilefen CreditAttribution: cilefen commented
Issue summary: View changes

It will be easier to follow issue status this way.

tibbsa’s picture

tibbsa CreditAttribution: tibbsa commented
Issue summary: View changes

Added editor and entity_reference to the list (working on these now)

cilefen’s picture

cilefen CreditAttribution: cilefen commented
Issue summary: View changes
cilefen’s picture

cilefen CreditAttribution: cilefen commented
Issue summary: View changes
tibbsa’s picture

tibbsa CreditAttribution: tibbsa commented
Issue summary: View changes
cilefen’s picture

cilefen CreditAttribution: cilefen commented
Issue summary: View changes
hussainweb’s picture

hussainweb
Primary language English
CreditAttribution: hussainweb commented
Issue summary: View changes

Adding issue for telephone module.

hussainweb’s picture

hussainweb
Primary language English
CreditAttribution: hussainweb commented
Issue summary: View changes

Adding issues for statistics and toolbar.

hussainweb’s picture

hussainweb
Primary language English
CreditAttribution: hussainweb commented
Issue summary: View changes

Added issue for shortcut module, and calling it a day.

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 commented
Issue summary: View changes
Related issues: +#2388071: Clean-up TestBase test members - ensure property definition and use of camelCase naming convention
Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 commented
Issue summary: View changes
Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 commented
Issue summary: View changes
Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 commented
Issue summary: View changes
tibbsa’s picture

tibbsa CreditAttribution: tibbsa commented

(refreshing issue status)

tibbsa’s picture

tibbsa CreditAttribution: tibbsa commented
Related issues: -#2388071: Clean-up TestBase test members - ensure property definition and use of camelCase naming convention

Removing related issue tag on TestBase/WebTestBase as these are linked as child/parent already.

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 commented
Issue summary: View changes

Added patch for system module. #2389455: Clean-up system module test members - ensure property definition and use of camelCase naming convention

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 commented
Related issues: +#1805340: Add missing type hinting to Simpletest module docblocks (excluding test base classes)

Added #1805340: Add missing type hinting to Simpletest module docblocks (excluding test base classes) as a related issue.

Also, I made a video about how you might go about reviewing patches for this meta: http://youtu.be/H6pWz_6XoiQ

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 commented
Issue summary: View changes

Added cleanup for field module.

tibbsa’s picture

tibbsa CreditAttribution: tibbsa commented
Issue summary: View changes

Added and working on #2394417: Clean-up field_ui module test members - ensure property definition and use of camelCase naming convention, #2394419: Clean-up file module test members - ensure property definition and use of camelCase naming convention, and #2394421: Clean-up filter module test members - ensure property definition and use of camelCase naming convention.

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 commented
Issue summary: View changes

Working on forum module.

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 commented
Issue summary: View changes

Working on history module. #2396657: Clean-up history module test members - ensure property definition and use of camelCase naming convention

hussainweb’s picture

hussainweb
Primary language English
CreditAttribution: hussainweb commented
Issue summary: View changes
hussainweb’s picture

hussainweb
Primary language English
CreditAttribution: hussainweb commented
Issue summary: View changes
hussainweb’s picture

hussainweb
Primary language English
CreditAttribution: hussainweb commented
Issue summary: View changes
hussainweb’s picture

hussainweb
Primary language English
CreditAttribution: hussainweb commented
Issue summary: View changes
hussainweb’s picture

hussainweb
Primary language English
CreditAttribution: hussainweb commented
Issue summary: View changes
hussainweb’s picture

hussainweb
Primary language English
CreditAttribution: hussainweb commented
Issue summary: View changes
hussainweb’s picture

hussainweb
Primary language English
CreditAttribution: hussainweb commented

I have added issues (and patches) for various modules like image, responsive_image, taxonomy, quickedit, options, path, menu_ui, locale, link, language, etc... I also added a module to the list above - config_translation and create an issue and patch for the same. Please review. :)

tibbsa’s picture

tibbsa CreditAttribution: tibbsa commented

(refresh status only)

star-szr’s picture

star-szr
he/him
Primary language English
CreditAttribution: star-szr commented
Status: Needs work » Active

This should go back to 'active' since it's a meta :)

tibbsa’s picture

tibbsa CreditAttribution: tibbsa commented

(status refresh)

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 commented
Issue summary: View changes

Updating the status of issues linked within the issue.

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 commented
Related issues: +#2081945: [Policy, no patch] Reintroduce 'private' visibility to object-oriented code.
Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 commented
Related issues: -#2081945: [Policy, no patch] Reintroduce 'private' visibility to object-oriented code.

wewps. :-)

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 commented
Issue summary: View changes

You can use this phpcs one-liner to help review these issues: https://gist.github.com/paul-m/bdb4c44ecd1de8f934cd

cilefen’s picture

cilefen CreditAttribution: cilefen commented

The good news is that there are only 3 sub-issues remaining to commit. The bad news is that egrep -rl '\$this\->[a-z]+_[a-z_]+' core/modules/*/src/Tests/ |egrep -v "(system/|field/|forum/)" or phpcs --standard=drupal --report-csv core/modules/*/src/Tests | grep "should use lowerCamel"shows that more have crept in to already-completed modules, especially node.

We should open up one issue to cover the remaining module tests once the three are committed.

And then there is the core/tests directory, which we have not yet looked into. A quick look in there with egrep -rl '\$this\->[a-z]+_[a-z_]+' core/tests/ shows only a handful of cases, so we should open one other issue for those.

cilefen’s picture

cilefen CreditAttribution: cilefen commented
Issue summary: View changes

#2463417: Clean-up remaining test members in module tests - ensure property definition and use of camelCase naming convention

cilefen’s picture

cilefen CreditAttribution: cilefen commented
Issue summary: View changes

#2463419: Clean-up Test members in core/tests - ensure property definition and use of camelCase naming convention

cilefen’s picture

cilefen CreditAttribution: cilefen commented
Status: Active » Closed (fixed)

Thank you everyone - it is done. All that remains is a commented chunk in FormTest.

$ ack  '\$this\->[a-z]+_[a-z_]+' core/modules/*/src/Tests/
core/modules/field/src/Tests/FormTest.php
233://    $this->field = $this->field_multiple;

The code was commented almost 3 years ago in #1591852: Convert field tests to PSR-0. This is closed.