Problem/Motivation

This is a followup issue for #1535868: Convert all blocks into plugins and #1871696: Convert block instances to configuration entities to resolve architectural issues.

Several of the migrated block tests create block instances with drupalPost(), because there is not currently a clean API for creating block instances programmatically.

Proposed resolution

Where appropriate (when the block configuration form itself is not being tested), change the tests listed below so that blocks are created programmatically. It might also be worthwhile to add boilerplate code for creating a particular block, since the same pattern is repeated many times.

  1. core/modules/views/lib/Drupal/views/Tests/Wizard/ItemsPerPageTest.php
  2. core/modules/views/lib/Drupal/views/Tests/Wizard/BasicTest.php
  3. core/modules/block/lib/Drupal/block/Tests/NewDefaultThemeBlocksTest.php
  4. core/modules/views/lib/Drupal/views/Tests/UI/OverrideDisplaysTest.php
  5. core/modules/user/lib/Drupal/user/Tests/UserBlocksTests.php
  6. core/modules/translation/lib/Drupal/translation/Tests/TranslationTest.php
  7. core/modules/system/lib/Drupal/system/Tests/System/AccessDeniedTest.php
  8. core/modules/system/lib/Drupal/system/Tests/Upgrade/BlockUpgradePathTest.php
  9. core/modules/system/lib/Drupal/system/Tests/Menu/TrailTest.php
  10. core/modules/system/lib/Drupal/system/Tests/Menu/BreadcrumbTest.php
  11. core/modules/statistics/lib/Drupal/statistics/Tests/StatisticsReportsTest.php
  12. core/modules/search/lib/Drupal/search/Tests/SearchConfigSettingsFormTest.php
  13. core/modules/search/lib/Drupal/search/Tests/SearchBlockTest.php
  14. core/modules/poll/lib/Drupal/poll/Tests/PollBlockTest.php
  15. core/modules/openid/lib/Drupal/openid/Tests/OpenIDTestBase.php
  16. core/modules/node/lib/Drupal/node/Tests/NodeBlockTest.php
  17. core/modules/node/lib/Drupal/node/Tests/NodeBlockFunctionalTest.php
  18. core/modules/menu/lib/Drupal/menu/Tests/MenuTest.php
  19. core/modules/language/lib/Drupal/language/Tests/LanguageUILanguageNegotiationTest.php
  20. core/modules/language/lib/Drupal/language/Tests/LanguageSwitchingTest.php
  21. core/modules/forum/lib/Drupal/forum/Tests/ForumNodeAccessTest.php
  22. core/modules/forum/lib/Drupal/forum/Tests/ForumBlockTest.php
  23. core/modules/comment/lib/Drupal/comment/Tests/CommentBlockTest.php
  24. core/modules/book/lib/Drupal/book/Tests/BookTest.php
  25. core/modules/block/lib/Drupal/block/Tests/BlockInvalidRegionTest.php
  26. core/modules/block/lib/Drupal/block/Tests/BlockHtmlIdTest.php
  27. core/modules/block/lib/Drupal/block/Tests/BlockHiddenRegionTest.php
  28. core/modules/block/lib/Drupal/block/Tests/BlockCacheTest.php
  29. core/modules/aggregator/lib/Drupal/aggregator/Tests/ImportOpmlTest.php
  30. core/modules/aggregator/lib/Drupal/aggregator/Tests/AggregatorRenderingTest.php
Files: 
CommentFileSizeAuthor
#20 simpletest-1872540-20.patch60.15 KBxjm
PASSED: [[SimpleTest]]: [MySQL] 50,574 pass(es).
[ View ]
#20 interdiff.txt3.58 KBxjm
#16 simpletest-1872540-16.patch59.64 KBxjm
FAILED: [[SimpleTest]]: [MySQL] 50,361 pass(es), 12 fail(s), and 8 exception(s).
[ View ]
#16 interdiff.txt848 bytesxjm
#15 interdiff.txt22.44 KBxjm
#13 simpletest-1872540-13.patch59.64 KBxjm
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/system/lib/Drupal/system/Tests/Upgrade/BlockUpgradePathTest.php.
[ View ]
#13 interdiff.txt24.24 KBxjm
#11 simpletest-1872540-11.patch39 KBxjm
FAILED: [[SimpleTest]]: [MySQL] 48,469 pass(es), 8 fail(s), and 0 exception(s).
[ View ]
#11 interdiff.txt24.24 KBxjm
#8 simpletest-1872540-8.patch20.49 KBxjm
FAILED: [[SimpleTest]]: [MySQL] 50,478 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#8 interdiff.txt3.13 KBxjm
#3 simpletest-1872540-3.patch19.04 KBxjm
FAILED: [[SimpleTest]]: [MySQL] 50,406 pass(es), 9 fail(s), and 1 exception(s).
[ View ]

Comments

Issue summary:View changes

Updated issue summary.

Issue summary:View changes

Updated issue summary.

33 uses of nearly-identical code sounds like a reason to add WebTestBase::addBlock() to me.

Assigned:Unassigned» xjm
Status:Postponed» Active

I'll work on this one.

Issue summary:View changes

Updated issue summary.

Issue summary:View changes

Updated issue summary.

Status:Active» Needs review
StatusFileSize
new19.04 KB
FAILED: [[SimpleTest]]: [MySQL] 50,406 pass(es), 9 fail(s), and 1 exception(s).
[ View ]

Attached converts #24 through #32. I think the first step is to convert all the tests to use this method, and then we can make drupalCreateBlockInstance() add the block instance programmatically rather than through the UI once block instances afre configuration entities.

+++ b/core/modules/simpletest/lib/Drupal/simpletest/WebTestBase.phpundefined
@@ -340,6 +340,78 @@ protected function drupalCreateContentType($settings = array()) {
+   * @return ??|false

Oh--right now this is the array returned by BlockBase::getConfig().

drupalCreateBlockInstance() is a lot of typing; I'm wondering if we could get away with something like drupalAddBlock() even though it's not quite as accurate.

Status:Needs review» Needs work

The last submitted patch, simpletest-1872540-3.patch, failed testing.

drupalPlaceBlock()? Then again, who types out stuff like drupalCreateBlockInstance(), even now I just copy/pasted that :)

I think drupalPlaceBlock() works. I considered it before and rejected it because it doesn't place existing instances in a different region, but at worst people will read the documentation and realize it doesn't do that. +1.

Status:Needs work» Needs review
StatusFileSize
new3.13 KB
new20.49 KB
FAILED: [[SimpleTest]]: [MySQL] 50,478 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Attached renames the method, fixes the test failures, and adds some wholly out-of-scope documentation cleanups because seeing "HTML id" causes me emotional distress.

Issue summary:View changes

Updated issue summary.

I think I'm also going to switch the order of arguments; the settings are customized far more frequently than the theme.

Status:Needs review» Needs work

The last submitted patch, simpletest-1872540-8.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new24.24 KB
new39 KB
FAILED: [[SimpleTest]]: [MySQL] 48,469 pass(es), 8 fail(s), and 0 exception(s).
[ View ]

Attatched does that and converts blocks from 16 on.

Status:Needs review» Needs work

The last submitted patch, simpletest-1872540-11.patch, failed testing.

Issue summary:View changes

Updated issue summary.

Status:Needs work» Needs review
StatusFileSize
new24.24 KB
new59.64 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/system/lib/Drupal/system/Tests/Upgrade/BlockUpgradePathTest.php.
[ View ]

This should be everything.

Status:Needs review» Needs work

The last submitted patch, simpletest-1872540-13.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new22.44 KB

Actual interdiff.

StatusFileSize
new848 bytes
new59.64 KB
FAILED: [[SimpleTest]]: [MySQL] 50,361 pass(es), 12 fail(s), and 8 exception(s).
[ View ]

Title:Create block instances programmatically in block testsProvide a test helper method for creating block instances

Re-scoping. It'll be much easier to convert just this one method to use the API following #1871696: Convert block instances to configuration entities to resolve architectural issues, and it will make writing additional tests easier as well.

Priority:Normal» Major

Without this, #1871696: Convert block instances to configuration entities to resolve architectural issues would have to duplicate all of this work, and with this, it would mean that issue only has to update one helper method.

The bot's almost done with this, and I'm going to review it now, but since this effectively blocks that critical, I'm promoting this.

Status:Needs review» Needs work

The last submitted patch, simpletest-1872540-16.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new3.58 KB
new60.15 KB
PASSED: [[SimpleTest]]: [MySQL] 50,574 pass(es).
[ View ]

Attached resolves the remaining test failures locally.

Status:Needs review» Reviewed & tested by the community

Awesome. Thanks so much, this makes that other issue so much more sane.

Status:Reviewed & tested by the community» Needs review

Note for reviewers: I willfully renamed NodeBlockTest to NodeSyndicateBlockTest, even though it's a bit out of scope, because it was misleading and it confused me and the test is getting half-rewritten anyway. :)

Status:Needs review» Reviewed & tested by the community

Oops, xpost!

Status:Reviewed & tested by the community» Fixed

Committed and pushed to 8.x. Thanks!

Issue tags:+Block plugins

Status:Fixed» Closed (fixed)

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

Assigned:xjm» Unassigned

Issue summary:View changes

Updated issue summary.