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

Some more camelCasing, some undeclared vars, and the removal of a bunch of web_user/admin_user properties that served no purpose.

Patch rerolled

@subhojit777: I think you uploaded the same patch as #8 again.

This is a reroll of #8. I think #13 was the same patch as #8 as I couldn't apply it manually.

man!!! I rerolled the patch but didnt obtained the diff, and uploaded the old one. Here is the rerolled patch that should have been uploaded in #13

The patch in #16 is not a correct reroll as it brings back a property which was removed in HEAD (hence the conflict). See the current source of ThemeTest.php and you will find that the property $strictConfigSchema is not there, nor in the patch in #8. That patch only adds the $adminUser property whereas the patch in #16 adds two.

@Mile23: Yes, it seems like a random failure. And it's not just a docblock, but also a property definition which was removed in HEAD but came back because of the conflict. I will attach patch in #15 again to avoid confusion. I have named it 24 to keep it consistent but it is the same patch as #15.

Go this result on execution of command egrep -rl '\$this\->[a-z]+_[a-z_]+' core/modules/system/src/Tests/

core/modules/system/src/Tests/DrupalKernel/DrupalKernelTest.php
core/modules/system/src/Tests/Image/ToolkitGdTest.php
core/modules/system/src/Tests/System/DateFormatsLockedTest.php
core/modules/system/src/Tests/File/HtaccessUnitTest.php
core/modules/system/src/Tests/File/ConfigTest.php
core/modules/system/src/Tests/Installer/InstallerTest.php
core/modules/system/src/Tests/Installer/SingleVisibleProfileTest.php
core/modules/system/src/Tests/Installer/DistributionProfileTest.php
core/modules/system/src/Tests/Update/UpdateScriptTest.php
core/modules/system/src/Tests/Session/SessionTest.php

Manually checked those files but variables with underscores were only local variables, so no problem with that. Patch looks good and applied cleanly on the HEAD.

Moving this to RTBC

It was a random failure - database connection failed.

Back to RTBC as per #25 after random fail.

Rerolling

Have to look back at this and see what is going on. I could not find a previous commit to which patch #31 would apply, so there might be something funky with that.

Guess what? I rerolled again. :P

The last RTBC was over a month ago as per #25. I am letting it be to Needs review as lot might have changed.

Sorry about that, Mile23.
Attached is a rerolled one that applies to 8.0.x.

working on it now

The patch is no longer applicable. Will have to reroll it first.

Seeing if the testbot is able to apply the patch. Will resolve the errors from #47 after that.

Tried to cover all the errors mentioned. Finger's crossed!

Hope, I am doing it right this time :-)

The test passes at local, giving it another try.

I am not sure why the test is failing. I did a quick egrep -rl '\$this\->[a-z]+_[a-z_]+' core/modules/system/src/Tests/ and found out the one file was missed out from the implementation.
I do not think if that was causing the test to fail, but I think it will be worth trying to fix it.

+++ b/core/modules/system/src/Tests/Update/UpdateScriptTest.php
@@ -23,17 +23,29 @@ class UpdateScriptTest extends WebTestBase {
-    $this->update_url = $GLOBALS['base_url'] . '/update.php';
-    $this->update_user = $this->drupalCreateUser(array('administer software updates', 'access site in maintenance mode'));
-    // Make sure updates for new entity type definitions are processed.
-    \Drupal::service('entity.definition_update_manager')->applyUpdates();
+    $this->updateUrl = $GLOBALS['base_url'] . '/update.php';
+    $this->updateUser = $this->drupalCreateUser(array('administer software updates', 'access site in maintenance mode'));

You lost a line here in the test, that's causing those test fails. Probably a merge that went wrong.

Hint: Review patches like this with "git diff --color-words='[^[:space:]]|([[:alnum:]]|UTF_8_GUARD)+'" ( I have an alias for this), that makes it much easier to catch the difference there.

@Berdir Thank you for the review! And also for the tip on reviewing, very helpful. I have also created an alias for the same, and will use it from now on.

Retrying the patch after adding the missed line.

Ta Da! It feels so good! Hope this goes through. I will make any changes suggested.

Looking into this. @Mile23, are you still looking at this? I checked on IRC but couldn't find you.

Trying a reroll from #63 again.

Fixing the failure.

Rerolling. Hope this finds its way in soon!

Committed f0c731f and pushed to 8.0.x. Thanks!

Thanks for adding the beta evaluation to the issue summary.

+++ b/core/modules/system/src/Tests/Cache/ClearTest.php
@@ -17,8 +17,8 @@
 class ClearTest extends CacheTestBase {

Let's get a followup to remove CacheTestBase and merge its functionality into ClearTest.

Also someone should have created followups to document the undocumented properties.

Created a followup at #2464383: Remove CacheTestBase and merge its functionality into ClearTest.