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."

#15 design_test-2037569-15.patch18.33 KBtim.plunkett
PASSED: [[SimpleTest]]: [MySQL] 58,416 pass(es).
[ View ]
#13 2037569-13-design_test-tests.patch5.44 KBpfrenssen
PASSED: [[SimpleTest]]: [MySQL] 58,908 pass(es).
[ View ]
#8 2032031-7.patch1.06 KBwildflower_0002
FAILED: [[SimpleTest]]: [MySQL] 57,564 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#6 design_test_followup-23.patch952 byteststoeckler
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch design_test_followup-23_0.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]


Issue tags:+Novice


Status:Active» Closed (works as designed)

design_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.

Title:Remove unused design_test moduleTest design_test module
Status:Closed (works as designed)» Active

Well 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.

In 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 :)

Title:Test design_test module Add tests for design_test module
Issue tags:+Needs tests


Status:Active» Needs review
new952 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch design_test_followup-23_0.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Yeah, 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.

Status:Needs review» Needs work

Needs work for tests, now that bot has picked it up.

Status:Needs work» Needs review
new1.06 KB
FAILED: [[SimpleTest]]: [MySQL] 57,564 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

refactor views_get_view on ExposedFormTest.php

Status:Needs review» Needs work

The last submitted patch, 2032031-7.patch, failed testing.

Wrong issue? :-)

Status:Needs work» Needs review

#6: design_test_followup-23.patch queued for re-testing.

Assigned:Unassigned» pfrenssen
Status:Needs review» Needs work

I will write some initial tests.

Assigned:pfrenssen» Unassigned
Status:Needs work» Needs review
new5.44 KB
PASSED: [[SimpleTest]]: [MySQL] 58,908 pass(es).
[ View ]

Wrote a test for design_test/page/list-operations, there are two more pages that need tests: design_test/form/details and design_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.

Title: Add tests for design_test moduleAdd tests for design_test module
Priority:Normal» Major

This 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.

Title:Add tests for design_test moduleRemove design_test module
Issue tags:-Needs tests, -Novice
new18.33 KB
PASSED: [[SimpleTest]]: [MySQL] 58,416 pass(es).
[ View ]

This has just fallen too far behind. It's in contrib already, it can stay there.

Status:Needs review» Reviewed & tested by the community

It is not used anywhere and it blocks the routing conversions as it has theme callbacks and everything.

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 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.

Status:Reviewed & tested by the community» Fixed

Had 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 module, with 3500+ installs (as opposed to 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 removal efforts, which is a big plus.

Therefore, committed and pushed to 8.x. If someone could post a reverse-diff to the queue, that'd be a nice thing to do, to get the project started with a 8.x branch.

Status:Fixed» Closed (fixed)

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

Issue summary:View changes

Adding info