Opened 5 months ago
Last modified 4 months ago
#36280 assigned Cleanup/optimization
Replace cm.exception.messages with assertRaisesMessage() in tests
Reported by: | Tim Graham | Owned by: | Georgii (George) Randiuk |
---|---|---|---|
Component: | Core (Other) | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | David Bogar | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | yes | UI/UX: | no |
Description (last modified by )
I haven't audited each instance, but most likely many of the uses of cm.exception.messages
could be made more consistent with the rest of the Django test suite by instead using self.assertRaisesMessage()
.
tests/composite_pk/test_models.py: self.assertSequenceEqual(ctx.exception.messages, messages) tests/composite_pk/test_models.py: ctx.exception.messages, ("User with this Email already exists.",) tests/validation/test_error_messages.py: self.assertEqual(cm.exception.messages, expected) tests/auth_tests/test_validators.py: self.assertEqual(cm.exception.messages, [msg_too_short]) tests/auth_tests/test_validators.py: cm.exception.messages, ["This password is too common.", msg_too_short] tests/auth_tests/test_validators.py: self.assertEqual(cm.exception.messages, [expected_error % 8]) tests/auth_tests/test_validators.py: self.assertEqual(cm.exception.messages, [expected_error % 3]) tests/auth_tests/test_validators.py: self.assertEqual(cm.exception.messages, [expected_error % "username"]) tests/auth_tests/test_validators.py: self.assertEqual(cm.exception.messages, [expected_error % "email address"]) tests/auth_tests/test_validators.py: self.assertEqual(cm.exception.messages, [expected_error % "first name"]) tests/auth_tests/test_validators.py: self.assertEqual(cm.exception.messages, [expected_error % "first name"]) tests/auth_tests/test_validators.py: cm.exception.messages, ["The password is too similar to the username."] tests/auth_tests/test_validators.py: self.assertEqual(cm.exception.messages, [expected_error]) tests/auth_tests/test_validators.py: self.assertEqual(cm.exception.messages, [expected_error]) tests/auth_tests/test_validators.py: self.assertEqual(cm.exception.messages, [expected_error]) tests/auth_tests/test_validators.py: self.assertEqual(cm.exception.messages, [expected_error]) tests/forms_tests/tests/test_error_messages.py: self.assertEqual(cm.exception.messages, expected) tests/forms_tests/tests/test_validators.py: self.assertEqual(2, len(e.exception.messages)) tests/test_exceptions/test_validation_error.py: self.assertEqual(sorted(exception.messages), []) tests/test_exceptions/test_validation_error.py: self.assertEqual(sorted(exception.messages), ["E1", "E2"]) tests/test_exceptions/test_validation_error.py: self.assertEqual(sorted(exception.messages), ["E1", "E2", "E3", "E4"]) tests/postgres_tests/test_array.py: cm.exception.messages[0], tests/postgres_tests/test_array.py: cm.exception.messages[0], "Nested arrays must have the same length." tests/postgres_tests/test_array.py: cm.exception.messages[0], tests/postgres_tests/test_array.py: cm.exception.messages[0], tests/postgres_tests/test_array.py: cm.exception.messages[0], tests/postgres_tests/test_array.py: cm.exception.messages[0], tests/postgres_tests/test_array.py: cm.exception.messages[0], tests/postgres_tests/test_array.py: self.assertEqual(cm.exception.messages[0], "This field is required.") tests/postgres_tests/test_array.py: cm.exception.messages, tests/postgres_tests/test_array.py: cm.exception.messages, tests/postgres_tests/test_hstore.py: self.assertEqual(cm.exception.messages[0], "Could not load JSON data.") tests/postgres_tests/test_hstore.py: self.assertEqual(cm.exception.messages[0], "Some keys were missing: b") tests/postgres_tests/test_hstore.py: self.assertEqual(cm.exception.messages[0], "Some unknown keys were provided: c") tests/postgres_tests/test_hstore.py: self.assertEqual(cm.exception.messages[0], "Foobar") tests/postgres_tests/test_hstore.py: self.assertEqual(cm.exception.messages[0], "Some unknown keys were provided: c") tests/postgres_tests/test_ranges.py: self.assertEqual(cm.exception.messages[0], msg) tests/postgres_tests/test_ranges.py: self.assertEqual(cm.exception.messages[0], msg) tests/postgres_tests/test_ranges.py: cm.exception.messages[0], tests/postgres_tests/test_ranges.py: self.assertEqual(cm.exception.messages[0], "Enter two whole numbers.") tests/postgres_tests/test_ranges.py: self.assertEqual(cm.exception.messages[0], "Enter a whole number.") tests/postgres_tests/test_ranges.py: self.assertEqual(cm.exception.messages[0], "Enter a whole number.") tests/postgres_tests/test_ranges.py: self.assertEqual(cm.exception.messages[0], "This field is required.") tests/postgres_tests/test_ranges.py: cm.exception.messages[0], tests/postgres_tests/test_ranges.py: self.assertEqual(cm.exception.messages[0], "Enter two numbers.") tests/postgres_tests/test_ranges.py: self.assertEqual(cm.exception.messages[0], "Enter a number.") tests/postgres_tests/test_ranges.py: self.assertEqual(cm.exception.messages[0], "Enter a number.") tests/postgres_tests/test_ranges.py: self.assertEqual(cm.exception.messages[0], "This field is required.") tests/postgres_tests/test_ranges.py: cm.exception.messages[0], tests/postgres_tests/test_ranges.py: self.assertEqual(cm.exception.messages[0], "Enter two valid dates.") tests/postgres_tests/test_ranges.py: self.assertEqual(cm.exception.messages[0], "Enter a valid date.") tests/postgres_tests/test_ranges.py: self.assertEqual(cm.exception.messages[0], "Enter a valid date.") tests/postgres_tests/test_ranges.py: self.assertEqual(cm.exception.messages[0], "This field is required.") tests/postgres_tests/test_ranges.py: cm.exception.messages[0], tests/postgres_tests/test_ranges.py: self.assertEqual(cm.exception.messages[0], "Enter two valid date/times.") tests/postgres_tests/test_ranges.py: self.assertEqual(cm.exception.messages[0], "Enter a valid date/time.") tests/postgres_tests/test_ranges.py: self.assertEqual(cm.exception.messages[0], "Enter a valid date/time.") tests/postgres_tests/test_ranges.py: self.assertEqual(cm.exception.messages[0], "This field is required.")
Change History (9)
follow-up: 2 comment:1 by , 5 months ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 5 months ago
Replying to Sarah Boyce:
Agree that most can be updated
Likely that the ones that check the error code (cm.exception.code
) may need to stay the same
I assume the same goes for assertEqual's that validate two lists of errors? Since, assertRaisesMessage validates against a string, not a list of them. Otherwise, we would need to write a new assert for that specifically.
comment:3 by , 5 months ago
Owner: | set to |
---|---|
Status: | new → assigned |
comment:4 by , 5 months ago
Description: | modified (diff) |
---|---|
Has patch: | set |
Patch needs improvement: | set |
comment:5 by , 5 months ago
Hi! I noticed the current patch for this ticket is marked as needing improvement.
I’d love to help move this forward — would it be alright if I tried improving the patch or opened a new PR based on the feedback so far?
Just want to make sure it’s okay before proceeding. Thanks in advance!
— Yahya
comment:6 by , 5 months ago
No, you should wait at least a month before considering a patch abandoned due to inactivity.
comment:7 by , 4 months ago
Cc: | added |
---|---|
Triage Stage: | Accepted → Ready for checkin |
comment:8 by , 4 months ago
Triage Stage: | Ready for checkin → Accepted |
---|
The patch is still marked as needs improvement.
comment:9 by , 4 months ago
The following pattern used to test assertions looks like it could also be covered by this ticket, maybe with the exception of tests for assertRaisesMessage
itself?
try: self.assertRedirects(response, "/get_view/") except AssertionError as e: self.assertIn( "Response redirected to '/get_view/?var=value', expected '/get_view/'", str(e), )
e.g. in test_client_regress.tests.*
, where this pattern is used extensively and interchangeably with assertRaisesMessage
.
Agree that most can be updated
Likely that the ones that check the error code (
cm.exception.code
) may need to stay the same