Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.In system/tests/modules/design_test we have a module that does not appear to be enabled by any tests at the moment. Do we need it?
Since this is incorporating work from #1987688: [Followup] Convert design_test_category_page() to a new style controller suggested commit message is "Issue #2037569 by tstoeckler, ParisLiakos: Add tests for design_test module."
| Comment | File | Size | Author |
|---|---|---|---|
| #15 | design_test-2037569-15.patch | 18.33 KB | tim.plunkett |
| #13 | 2037569-13-design_test-tests.patch | 5.44 KB | pfrenssen |
| #8 | 2032031-7.patch | 1.06 KB | wildflower_0002 |
| #6 | design_test_followup-23.patch | 952 bytes | tstoeckler |
Comments
Comment #1
alexpotttagging
Comment #2
tstoecklerdesign_test.module was added for manual testing of certain page elements, such as nested vertical tabs, whether they look OK and whether they work properly. That's why it's not being used by any test. It got broken recently (or even before that), but I'm not sure if we want to add tests for it, that seems a little extreme, IMO.
Closing this for now.
Comment #3
alexpottWell the fact that we've broken this tells me that we should have tests for it.
The test should enable to the module. And tested that expected functionality provided by design_test_hook_menu works as advertised.
Comment #4
alexpottIn order to do this the patch should incorporate the follow up fox from #1987688: [Followup] Convert design_test_category_page() to a new style controller as the module currently does not work :)
Comment #5
alexpottTagging
Comment #6
tstoecklerYeah, I guess I agree. :-)
Re-uploading the patch from the referenced issue here, so that it doesn't get lost. This was done by me and @ParisLiakos.
Sending for a quick date with the bot. still needs tests, as @alexpott mentioned.
Comment #7
tstoecklerNeeds work for tests, now that bot has picked it up.
Comment #8
wildflower_0002 CreditAttribution: wildflower_0002 commentedrefactor views_get_view on ExposedFormTest.php
Comment #10
tstoecklerWrong issue? :-)
Comment #11
lokapujya#6: design_test_followup-23.patch queued for re-testing.
Comment #12
pfrenssenI will write some initial tests.
Comment #13
pfrenssenWrote a test for
design_test/page/list-operations, there are two more pages that need tests:design_test/form/detailsanddesign_test/form/fieldset.Fixed a notice in the module and added an id to the table on the 'list operations' page so it can be targetted more easily in the test.
Comment #14
tim.plunkettThis is either a major or the whole thing needs to be removed. We're actively removing D8 functionality like page callbacks, and this module uses them, and we'd never know otherwise.
Comment #15
tim.plunkettThis has just fallen too far behind. It's in contrib already, it can stay there.
Comment #16
dawehnerIt is not used anywhere and it blocks the routing conversions as it has theme callbacks and everything.
Comment #17
nod_design_test is around to make some manual tests easier. Sort of a poor man's frontend testing environment.
Because it's such an imprtant we shouldn't have a half-baked solution in core to do frontend testing. The JS maintainers had a core convo in Prague about the topic and we're slowly gearing up to come up with a proper solution.
In the meantime https://drupal.org/project/testswarm can be used for those things, and it comes with some automated tests so you don't have to click around so much.
+1 on the removal from core.
Comment #18
webchickHad a long talk on IRC about this patch tonight.
I can definitely see the module's utility. It's basically a "smoke screen" for front-end problems to help catch things that would be very difficult to find just in random clicking around. The question being raised here is whether it belongs in core, and is worth expending effort porting to the new router system and maintaining on an ongoing basis by the core dev team.
git blame shows that it was introduced as part of #1168246: Freedom For Fieldsets! Long Live The DETAILS., and I can see how it would've been really helpful for that issue.
OTOH, it seems pretty clear that this module hasn't really been used since then for its intended purpose. For one, it's been sitting around busted since at least July. For another, the one huge design shift we've had ongoing efforts around (Seven Style Guide) hasn't used this module at all, and has instead been using Lewis Nyman's tensile framework for fleshing out problems. And in D7, people seem to have coalesced around the https://drupal.org/project/styleguide module, with 3500+ installs (as opposed to https://drupal.org/project/design with 45 installs).
So given that we don't have any code touching this right now to check if it's functioning (and as a result it's been broken for months with no one realizing), the module itself is either too hidden or not broad enough to have been actually used as a test case in subsequent design efforts, the contrib ecosystem seems to have settled on a different solution, and the folks driving front-end testing wish the core solution to be more holistic than this, I think that's enough argumentation for me to justify this module's removal from core. It also helps expedite the menu.inc removal efforts, which is a big plus.
Therefore, committed and pushed to 8.x. If someone could post a reverse-diff to the https://drupal.org/project/design queue, that'd be a nice thing to do, to get the project started with a 8.x branch.
Comment #19.0
(not verified) CreditAttribution: commentedAdding info