Add test coverage for admin/structure/block/demo

Looks nice, only one thing spotted:

+++ b/core/modules/block/src/Tests/BlockDemoTest.php
@@ -0,0 +1,66 @@
+ * Contains of Drupal\block\Tests\BlockDemoTest.

This should be 'Contains \Drupal...' not 'Contains of Drupal'

+++ b/core/modules/block/src/Tests/BlockDemoTest.php
@@ -0,0 +1,66 @@
+    // Install default theme and confirm we have access to the demo page for it.
+    $config = \Drupal::config('system.theme');
+    $default_theme = $config->get('default');
+    \Drupal::service('theme_handler')->install(array($default_theme));

The default theme is already installed.

+++ b/core/modules/block/src/Tests/BlockDemoTest.php
@@ -0,0 +1,66 @@
+    $edit['admin_theme'] = $default_theme;
+    $this->drupalPostForm('admin/appearance', $edit, t('Save configuration'));
...
+    $edit['admin_theme'] = 'bartik';
+    $this->drupalPostForm('admin/appearance', $edit, t('Save configuration'));
...
+    $edit['admin_theme'] = 'seven';
+    $this->drupalPostForm('admin/appearance', $edit, t('Save configuration'));

Why do we need to set the admin theme?

Updated patch from #6 based on comments from alexpott in #8. No need to install default theme. No need to set admin theme. Refactored code to loop through all available themes and perform assertions.

Added beta evaluation

I think, we should use the short array synatx (as new patch policy).

I updated the patch with the short array syntax.

Updated the patch so it includes the whole file now.

Hided the wrong patch.

Nice. Pretty sure this can be in 8.1.x because it's a test.

Restarting the testbot for the patch in #17. The test passes locally.

Here's a review:

1. +++ b/core/modules/block/src/Tests/BlockDemoTest.php
   @@ -0,0 +1,67 @@
   +/**
   + * @file
   + * Contains \Drupal\block\Tests\BlockDemoTest.
   + */
   

   We don't do 'Contains' anymore, so remove the @file section.

2. +++ b/core/modules/block/src/Tests/BlockDemoTest.php
   @@ -0,0 +1,67 @@
   +    $config = \Drupal::config('system.theme');
   

   Should be $this->container->get('config.factory')->get('system.theme');

3. +++ b/core/modules/block/src/Tests/BlockDemoTest.php
   @@ -0,0 +1,67 @@
   +      \Drupal::service('theme_handler')->install([$theme]);
   

   Should be $this->container->get() instead of \Drupal::service().

Hi Mile23,

Thanks for suggestions.

I have worked on changes mentioned above. Added patch and interdiff.

That's a F/R can we bump this to 8.2.x and directly write the BTB instead of WTB ?

Unassigning because no work has been done on it for almost 4 weeks. Also, I'm hoping that someone will be able to review this at the Drupal North 2016 code sprint tomorrow.

@Ashish.Dalvi, if you are still working on this issue, please re-assign it to yourself.

Not sure what the abbreviation "BTB" means, but I think "F/R" = "Feature Request" and "WTB" = "Web Test Base".

Right, BrowserTestBase and WebTestBase.

Not sure about what 'F/R' means because this isn't a feature request. Maybe @naveenvalecha can clarify.

It should be 8.1.x because it's a test improvement. The new test should pass both 8.1.x and 8.2.x.

@Mile23,
Yup F/R refers to the feature request.
however if its not a FR then its good to review.

Attached is a new patch with just one newline added for code style. Otherwise looked good.

We're not adding more WTB tests b/c it would be the duplicate efforts than in conversion

//Naveen

Looks good and works, marking RTBC.

Committed/pushed to 8.4.x and cherry-picked to 8.3.x, thanks!