Clean-up simpletest module test members - ensure property definition and use of camelCase naming convention

This is session_id in WebTestBase because I am wondering what it may break, if anything.

$session_id is a bit of a pain because of $account->session_id.

Building on the above patch, this is more significant and will almost certainly break a ton things elsewhere. Locally, 'simpletest' tests were fine.

Primarily root_user -> rootUser changes across the board

Missed a few files in that last patch + langCode changes here.

Added separate issues for WebTestBase and TestBase:

• #2388181: Clean-up WebTestBase test members - ensure property definition and use of camelCase naming convention
• #2388071: Clean-up TestBase test members - ensure property definition and use of camelCase naming convention

The sub-issues for this were created on a per-module basis, not a per-class basis. Why separate these two? You could re-factor the above patch into two separate ones but why?

The TestBase/WebTestBase issues are duplicative of some of what was in the patches above. Those two should be resolved first and then we can come back to fixing anything left in SimpleTest itself.

WebTestBase and TestBase aren't tests, so I thought the tests *for the module* would be separate. Because I was horribly confused or something. I guess. Sorry.

I'll mark the class issues as closed won't fix.

Anyway, here's my review of the patch in #8:

WebTestBase still has $http_auth and $http_credentials.

I took the simpletest module to be a bit of an exception to the rule, given that all/most of the relevant Test* classes are derived from WebTestBase/TestBase, but your logic was not unreasonable either!

Where are you seeing those variables?

..drupal $ egrep -rl '\->http_auth' *
..drupal $ egrep -rl '\->http_cred' *

I can't find either one anywhere in the Drupal codebase?

Oops. Mistyped.

http://cgit.drupalcode.org/drupal/tree/core/modules/simpletest/src/WebTe...

  /**
   * HTTP authentication method.
   */
  protected $httpauth_method = CURLAUTH_BASIC;

  /**
   * HTTP authentication credentials (<username>:<password>).
   */
  protected $httpauth_credentials = NULL;

It turns out that those httpauth_* variables are actually initialized in TestBase::run(), and so need to be declared there rather than in WebTestBase. This is of course rather backward: TestBase has no use for this information. I can't quite fathom why these settings would not just be retrieved in WebTestBase::curlInitialize(), rather than being set up as class properties in TestBase::run(), but that's beyond the scope of this change.

Rerolled.

diff --git a/core/modules/system/src/Tests/Installer/SingleVisibleProfileTest.php b/core/modules/system/src/Tests/Installer/SingleVisibleProfileTest.php
index a616703..35bcf74 100644
--- a/core/modules/system/src/Tests/Installer/SingleVisibleProfileTest.php
+++ b/core/modules/system/src/Tests/Installer/SingleVisibleProfileTest.php
@@ -62,7 +62,7 @@ public function testInstalled() {
     $this->assertUrl('user/1');
     $this->assertResponse(200);
     // Confirm that we are logged-in after installation.
-    $this->assertText($this->root_user->getUsername());
+    $this->assertText($this->rootUser->getUsername());
     // Confirm that the minimal profile was installed.
     $this->assertEqual(drupal_get_profile(), 'minimal');
   }

I think TestBase::run() is more for pulling in the settings, and then WebTestBase:: curlInitialize() can act on the settings. I'd bet it works this way because the ideal is to only generate the settings object once. It could probably use some refactor love, maybe.

But I agree, it's out of scope for this issue to change things up too much, and moving the $httpauth* properties to TestBase seems like the right thing to do.

Plus, this patch accomplishes the goal of changing all the properties in all the tests and other test base classes to make sure they have camelCase property names instead of under_score.

I know this because I did a bit of analysis with phpcs and couldn't find any instances of errors for that coding standard in the tests.

Therefore... RTBC.

+++ b/core/modules/system/src/Tests/Installer/InstallerTranslationMultipleLanguageForeignTest.php
@@ -19,7 +19,7 @@ class InstallerTranslationMultipleLanguageForeignTest extends InstallerTranslati
-  protected $langcode = 'de';
+  protected $langCode = 'de';

Here and elsewhere: This should not be changed. We refer to language codes as $langcode everywhere in code, not $lang_code. So by the same logic it should be $this->langcode not $this->langCode.

This is one of those cases where we ditch our own standards of not abbreviating. In theory it should be $language_code and $this->languageCode but we've decided that that's too many characters to type.

Ah, new core commits came in between my pull and when I finished working on this. Rerolled.

Interestingly, after I applied this patch, TestBase has both ::privateFilesDirectory and ::private_files_directory.

Could be related to this: https://www.drupal.org/node/2392959

Probably needs a re-roll.

Erased the private_files_directory. There's just one occurrence of it :)

OK, so the patch in #25 seems to have dealt with the duplicate private files directory issue.

The patch applies, passes tests, and fixes the underscore property names issue according to phpcs.

langCode is nowhere to be found, and the refactoring of httpauth_* properties in #15 is still there, so we're happy.

The test failed due to being unable to apply the patch. The patch applies locally for me, so let's re-try the testbot.

So re-ups on the RTBC from #26, plus refactoring NodeAccessViewGrantsCacheContextTest which was added in #2390691: Expose node grants as cache context.

Re-rolled

phpcs says no camel case errors in test classes, which in the case of simpletest module is really all classes.

Previous RTBC before reroll in #36.

Looks like Alex has been committing others of these, so joining the club. :P

Committed and pushed to 8.0.x. Thanks!