Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Problem/Motivation
This is a followup issue for #1535868: Convert all blocks into plugins.
- Currently, the Custom Block module has no test coverage of its own.
- Some test coverage for custom blocks is still in the parent Block module's test namespace.
- There is no BlockTestBase.
- BlockTest runs slowly and includes many unrelated tests. Some are related to basic block module functionality, whereas others are related to custom blocks. The custom block module is enabled for all these tests, which is unnecessary overhead.
- BlockTest also includes some helper methods and setup that could be useful to other tests and, once improved, could reduce test run time for tests that use them.
Proposed resolution
- Split
BlockTest
intoCustomBlockTest
,BlockTest
, andBlockTestBase
. - Move helper code to remove defined block instances into
BlockTestBase::setUp()
, to reduce the risk of false positives in block tests.
Remaining tasks
Related issue
Comment | File | Size | Author |
---|---|---|---|
#24 | drupal-block-test-1874598-23.patch | 12.16 KB | mojzis |
#22 | drupal-block-test-1874598-22.patch | 12.15 KB | mojzis |
#21 | block-test-1874598-21.patch | 8.48 KB | mojzis |
#10 | block-test-1874598-10.patch | 21.86 KB | xjm |
#10 | interdiff.txt | 7.32 KB | xjm |
Comments
Comment #1
xjmSome or perhaps all of
BlockTest
is actually custom block test coverage.Comment #2
xjmHad my nose in
BlockTest
half of today, so I'll roll a patch that moves the existing tests.Comment #5
xjmComment #6
xjmWe probably want to wait for #1872540: Provide a test helper method for creating block instances to work on this further.
Comment #8
xjmOops, looks like I accidentally removed a helper:
Comment #8.0
xjmUpdated issue summary.
Comment #9
xjmThinking about it, I'd like to move the code first and commit that since patches that move code are a pain to reroll. Then we can work on improving the test coverage in a followup issue once #1872540: Provide a test helper method for creating block instances is in.
Comment #10
xjmMake that "accidentally removed two helpers". The second should actually just be part of the setup method of a
BlockTestBase
class, though.Comment #11
xjm#10: block-test-1874598-10.patch queued for re-testing.
Comment #11.0
xjmUpdated issue summary.
Comment #11.1
xjmUpdated issue summary.
Comment #12
xjmPostponing this on #1871696: Convert block instances to configuration entities to resolve architectural issues and #1884758: Remove testing profile default blocks, which will do the same thing the old helper did in BlockTest, but better.
Comment #13
xjmNo, really.
Comment #14
larowlanAlso #1871772: Convert custom blocks to content entities will add CRUD/hook tests for Custom Block
Comment #15
xjmComment #16
tim.plunkett#10: block-test-1874598-10.patch queued for re-testing.
Comment #18
tim.plunkett#10: block-test-1874598-10.patch queued for re-testing.
Comment #20
xjmAlright, rerolling this now.
Comment #21
mojzis CreditAttribution: mojzis commentedreroll :
- removed completely the creation of CustomBlockTest - something has already been done there
- function removeDefaultBlocks has allready been removed
slight change in the Base::setup()
- formats are now entities, using the newer code
Comment #22
mojzis CreditAttribution: mojzis commentedoops, forgot the base class ...
Comment #24
mojzis CreditAttribution: mojzis commentedone more time :)
Comment #25
mojzis CreditAttribution: mojzis commentedpls review :)
Comment #27
xjmThis looks good -- waiting for the bot to RTBC, though. :)
Comment #28
mojzis CreditAttribution: mojzis commentednow we have the remaining task of testing the addition of a block from the custom_block module to a region. I am not sure where that should reside - whether in the tests for custom_block (because a block has to be created first) or here (because this is testing block addition :). Should i create the follow-up issue ?
Comment #29
xjmYeah, let's do that in a followup issue. Thanks!
Comment #30
xjm#24: drupal-block-test-1874598-23.patch queued for re-testing.
Comment #31
xjmComment #32
webchickThis looks like a useful abstraction.
Committed and pushed to 8.x. Thanks!
Comment #34
chrisjlee CreditAttribution: chrisjlee commentedComment #34.0
chrisjlee CreditAttribution: chrisjlee commentedUpdated issue summary.