Change API of assertFormError to take an actual form object

[Baptiste Mispelon]

The current signature of assertFormError is (response, form, field, errors, msg_prefix='') where:

• response is a response object (specifically one generated by the test client because it needs response.context)
• form is the name of a form (string) to be found in the response's context
• field is a name of a field in the form (string)
• errors is either a list of error strings, a single error string (equivalent to [error]) or None (equivalent to [])
• msg_prefix is a string added to the test failure message

Personally I've always found this response/form API to be clunky since I often want to test a form directly without having to go through the test client.
It would be a lot easier to use if we could pass a form object directly. I don't really understand why the API was implemented like this to begin with.

On top of that, now that Django uses template-based rendering for forms, formsets and widgets there are a lot of contexts to search in response.context and a lot of possibilities for clashing names (see #33346).

I would therefore propose the following new signature: assertFormError(form, field, errors, msg_prefix='') with form being an actual Form object and all other parameters staying the same.
The deprecation should be straightforward to implement by checking if form is a response object (isinstance check) or if response kwarg has been passed.

With that change assertFormsetError becomes mostly useless since one can simply do assertFormError(formset.forms[i], field_name, errors). The one usecase that wouldn't be covered is i = None + field_name=None which checks the errors list against the formset's non_form_errors().
We could either leave assertFormsetError in place as a convenience method (with a similar change to its API to accept a formset object directly) or deprecate more agressively by suggesting the user tests the output of formset.non_field_errors() directly.

[Baptiste Mispelon]

As an argument in favor of this change, may I present this lines from tests/admin_views/tests.py:6540 [1]

msg = "The formset 'inline_admin_formset' in context 22 does not contain any non-form errors."
with self.assertRaisesMessage(AssertionError, msg):
    self.assertFormsetError(response, 'inline_admin_formset', None, None, ['Error'])

Over the years as more and more templates got involved in the rendering of admin views, the exact number needed to make this test pass went from 4 to 10 to 12 to 22 (and every time, I suspect someone changed it manually to whichever new number the test failure asked for).

With the change I propose, the test would look like this:

for formset in response.context['inline_admin_formsets']:
    with self.assertRaisesMessage("The formset does not contain any non-form errors"):
        self.assertFormsetError(formset, None, None, ['error']

Though on second thought, in this case I think we can skip assertFormsetError altogether:

for i, formset in enumerate(response.context['inline_admin_formsets']):
    with self.subTest(i=i):
        self.assertEqual(formset.non_form_errors(), [])

[1] ​https://github.com/django/django/blob/8a4e5067605e608c3fcbb5ca11e0019eac8b40aa/tests/admin_views/tests.py#L6540

[Mariusz Felisiak]

Agreed, let's change the first argument to form/formset and deprecate passing a response in it.

[Baptiste Mispelon]

PR is ready for review: ​https://github.com/django/django/pull/15179

Overall the changes are backwards-compatible but there are some exceptions:

1) The failure messages are now different
2) The behavior of errors=[] is completely different
3) The behavior in the case of multiple errors is different

For 1), I don't know if our compatibility policy applies here. As noted in comment:1 the failure message were already prone to change arbitrarily as Django rendered more and more templates in its request/response cycle.

For 2), the old implementation of assertFormError (and assertFormsetError) would always pass when using errors=[]. Even if the field had no errors, or even if the field was not present on the form. I'd argue that this is a prety nasty bug and fixing it actually caught an incorrect tests in Django's own test suite [1]

For 3), the old implementation would only make sure that all the errors passed to assertFormError (and assertFormsetError) were present in the field's error. If the field happened to have extra errors the test would still pass. I changed that behavior so that the two lists must match exactly. This makes the implementation simpler (we can use a plain assertEqual and let Python give a nice diff in case of errors) and I think it's also more correct. The documentation wasn't very clear anyway so I think the change is acceptable.

As part of the ticket, I also removed the undocumented ability to use errors=None as an equivalent to errors=[]. I don't really see the usecase for that "shortcut" and "one obvious way" something something.

Finally, the PR adds a nicer repr for formsets, similar to what was done in #23167 (it made the tests easier to write).

[1] ​https://github.com/django/django/blob/2f73e5406d54cb8945e187eff302a3a3373350be/tests/admin_views/tests.py#L5558

[Mariusz Felisiak <felisiak.mariusz@…>]

In c67e1cf4:

Refs #33348 -- Deprecated passing errors=None to SimpleTestCase.assertFormError()/assertFormsetErrors().

[Mariusz Felisiak <felisiak.mariusz@…>]

In 7986028:

Refs #33348 -- Made SimpleTestCase.assertFormError()/assertFormsetErrors() raise an error for unbound forms/formsets.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 9bb13def:

Refs #33348 -- Made SimpleTestCase.assertFormsetErrors() raise an error when form_index is too big.

[Mariusz Felisiak <felisiak.mariusz@…>]

In d84cd91e:

Refs #33348 -- Improved messages raised by SimpleTestCase.assertFormError()/assertFormsetErrors().

This makes messages use BaseFormSet/BaseForm.repr() instead of
context, and adds the _assert_form_error() helper.

[Mariusz Felisiak <felisiak.mariusz@…>]

In d4c9dab:

Refs #33348 -- Fixed SimpleTestCase.assertFormError() error message raised for unbound forms.

[Mariusz Felisiak <felisiak.mariusz@…>]

In f7e0bffa:

Refs #33348 -- Made SimpleTestCase.assertFormError() raise ValueError when "field" is passed without "form_index".

[Mariusz Felisiak <felisiak.mariusz@…>]

In 50e1e7ef:

Fixed #33348 -- Changed SimpleTestCase.assertFormError()/assertFormsetErrors() to take form/formset.

Instead of taking a response object and a context name for
the form/formset, the two methods now take the object directly.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 23c8787:

Refs #33348 -- Removed support for passing errors=None to SimpleTestCase.assertFormError()/assertFormsetErrors().

Per deprecation timeline.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 71d1203b:

Refs #33348 -- Removed support for passing response object and form/formset name to SimpleTestCase.assertFormError()/assertFormSetError().

Per deprecation timeline.