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.
- core/modules/views/lib/Drupal/views/Tests/Wizard/ItemsPerPageTest.php
- core/modules/views/lib/Drupal/views/Tests/Wizard/BasicTest.php
- core/modules/block/lib/Drupal/block/Tests/NewDefaultThemeBlocksTest.php
- core/modules/views/lib/Drupal/views/Tests/UI/OverrideDisplaysTest.php
- core/modules/user/lib/Drupal/user/Tests/UserBlocksTests.php
- core/modules/translation/lib/Drupal/translation/Tests/TranslationTest.php
- core/modules/system/lib/Drupal/system/Tests/System/AccessDeniedTest.php
- core/modules/system/lib/Drupal/system/Tests/Upgrade/BlockUpgradePathTest.php
- core/modules/system/lib/Drupal/system/Tests/Menu/TrailTest.php
- core/modules/system/lib/Drupal/system/Tests/Menu/BreadcrumbTest.php
- core/modules/statistics/lib/Drupal/statistics/Tests/StatisticsReportsTest.php
- core/modules/search/lib/Drupal/search/Tests/SearchConfigSettingsFormTest.php
- core/modules/search/lib/Drupal/search/Tests/SearchBlockTest.php
- core/modules/poll/lib/Drupal/poll/Tests/PollBlockTest.php
- core/modules/openid/lib/Drupal/openid/Tests/OpenIDTestBase.php
- core/modules/node/lib/Drupal/node/Tests/NodeBlockTest.php
- core/modules/node/lib/Drupal/node/Tests/NodeBlockFunctionalTest.php
- core/modules/menu/lib/Drupal/menu/Tests/MenuTest.php
- core/modules/language/lib/Drupal/language/Tests/LanguageUILanguageNegotiationTest.php
- core/modules/language/lib/Drupal/language/Tests/LanguageSwitchingTest.php
- core/modules/forum/lib/Drupal/forum/Tests/ForumNodeAccessTest.php
- core/modules/forum/lib/Drupal/forum/Tests/ForumBlockTest.php
- core/modules/comment/lib/Drupal/comment/Tests/CommentBlockTest.php
- core/modules/book/lib/Drupal/book/Tests/BookTest.php
- core/modules/block/lib/Drupal/block/Tests/BlockInvalidRegionTest.php
- core/modules/block/lib/Drupal/block/Tests/BlockHtmlIdTest.php
- core/modules/block/lib/Drupal/block/Tests/BlockHiddenRegionTest.php
- core/modules/block/lib/Drupal/block/Tests/BlockCacheTest.php
- core/modules/aggregator/lib/Drupal/aggregator/Tests/ImportOpmlTest.php
- core/modules/aggregator/lib/Drupal/aggregator/Tests/AggregatorRenderingTest.php
Comment | File | Size | Author |
---|---|---|---|
#20 | simpletest-1872540-20.patch | 60.15 KB | xjm |
#20 | interdiff.txt | 3.58 KB | xjm |
#16 | simpletest-1872540-16.patch | 59.64 KB | xjm |
#16 | interdiff.txt | 848 bytes | xjm |
#15 | interdiff.txt | 22.44 KB | xjm |
Comments
Comment #0.0
xjmUpdated issue summary.
Comment #0.1
xjmUpdated issue summary.
Comment #1
xjm33 uses of nearly-identical code sounds like a reason to add
WebTestBase::addBlock()
to me.Comment #2
xjmI'll work on this one.
Comment #2.0
xjmUpdated issue summary.
Comment #2.1
xjmUpdated issue summary.
Comment #3
xjmAttached 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.Comment #4
xjmOh--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 likedrupalAddBlock()
even though it's not quite as accurate.Comment #6
tim.plunkettdrupalPlaceBlock()? Then again, who types out stuff like drupalCreateBlockInstance(), even now I just copy/pasted that :)
Comment #7
xjmI 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.Comment #8
xjmAttached renames the method, fixes the test failures, and adds some wholly out-of-scope documentation cleanups because seeing "HTML id" causes me emotional distress.
Comment #8.0
xjmUpdated issue summary.
Comment #9
xjmI think I'm also going to switch the order of arguments; the settings are customized far more frequently than the theme.
Comment #11
xjmAttatched does that and converts blocks from 16 on.
Comment #12.0
(not verified) CreditAttribution: commentedUpdated issue summary.
Comment #13
xjmThis should be everything.
Comment #15
xjmActual interdiff.
Comment #16
xjmComment #17
xjmRe-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.
Comment #18
tim.plunkettWithout 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.
Comment #20
xjmAttached resolves the remaining test failures locally.
Comment #21
tim.plunkettAwesome. Thanks so much, this makes that other issue so much more sane.
Comment #22
xjmNote for reviewers: I willfully renamed
NodeBlockTest
toNodeSyndicateBlockTest
, 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. :)Comment #23
xjmOops, xpost!
Comment #24
webchickCommitted and pushed to 8.x. Thanks!
Comment #25
xjmComment #27
xjmComment #27.0
xjmUpdated issue summary.