Trying to add tests for #582584: form_required_marker() isn't passed the form element (theme_hook definition is wrong) led me to look closely at FormsTestCase::testRequiredFields() from modules/simpletest/tests/form.test. It's broken in various ways:

A) It never calls form_clear_error() so it's often operating on stale form error messages.

B) Every form in there is failing with 'form_token' => 'Validation error, please try again. If this error persists, please contact the site administrator.' -- seems like the way we're building/executing the forms is bogus in some way.

C) It'd be nice to test that if each of those dummy form elements is *not* marked required that you do *not* see a form error when trying to submit them with empty input.

Moving this to a separate bug report so as not to cram too many changes into #582584.

CommentFileSizeAuthor
#1 582956-1.required_form_tests.patch5.96 KBdww

Comments

dww’s picture

dww
we/he/they
commented
Assigned: Unassigned » dww
Status: Active » Needs review
StatusFileSize
new 582956-1.required_form_tests.patch5.96 KB
dww’s picture

dww
we/he/they
commented

Note: patch #1 still doesn't address (B). Not sure if it really matters. I'd like chx or another FAPI expert to comment.

damien tournoud’s picture

damien tournoud commented
Status: Needs review » Reviewed & tested by the community

Makes sense and works. I'm a little bit concerned about those tests: Form API is too big a beast to try unit-testing it likes this (or we end up with (B) :p). We should rewrite those tests at one point.

dww’s picture

dww
we/he/they
commented

Gentle ping to committers: this is holding up progress on other patches I'd like to work on... Thanks. ;)

dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD.

Status: Fixed » Closed (fixed)

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