inconsistent escaping in assertRaisesMessage test output

[Chris Jerdonek]

If assertRaisesMessage fails with differing exception text, then the test output escapes the two strings differently in the display.

For example--

def test(self):
    with self.assertRaisesMessage(Exception, "two words"):
        raise Exception("no, three words")

yields--

----------------------------------------------------------------------
Exception: no, three words

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "***.py", line 178, in test
    raise Exception("no, three words")
AssertionError: "two\ words" does not match "no, three words"

Observe the spurious slash ("\") before the space. The test output is especially annoying if the string is long and has many spaces. It can look something like--

\{\
\ \ \ \ \"

[Tim Graham]

I don't see an easy way to address this issue. The expected message is passed through re.escape(expected_message) before it's passed to six.assertRaisesRegex. It seems we would have to reimplement large parts of six.assertRaisesRegex in order to unescape the pattern in the error message.

[Chris Jerdonek]

Since only a plain string comparison is needed, is there a reason the method needs to be using assertRaisesRegex? For example, couldn't the implementation bypass assertRaisesRegex and go something like this? (This is just an idea so shouldn't be taken as is.)

@contextmanager
def assertRaisesMessage(self, expected_exception, expected_message):
    with self.assertRaises(expected_exception) as cm:
        yield
    err = cm.exception
    self.assertEqual(str(err), expected_message)

An implementation like this would have the further advantage that in the case of a string mismatch the test output would show where the strings differ by using unittest's assertEqual, which is better equipped for this and has this added functionality.

[Tim Graham]

The attached patch passes the test suite, however, it doesn't fail correctly when assertRaisesMessage() is used in a non-context manager context. If this issue can be solved, I don't see a reason not to use that approach.

[Chris Jerdonek]

What about something like this?

@contextmanager
def _assert_raises_message_cm(self, expected_exception, expected_message):
    with self.assertRaises(expected_exception) as cm:
        yield cm
    err = cm.exception
    self.assertEqual(str(err), expected_message)

def assertRaisesMessage(self, expected_exception, expected_message, _callable=None, *args, **kwargs):
    cm = self._assert_raises_message_cm(expected_exception, expected_message)
    if _callable is None:
        # Then return the context manager.
        return cm
    # Otherwise, call the callable inside the context manager.
    with cm:
        _callable(*args, **kwargs)

[Chris Jerdonek]

(Actually, if callable is supposed to be positional-only, then it could be pulled as the first element of *args.)

[Tim Graham]

Yes, that seems to work. Thanks for the help. ​PR for review.

[Chris Jerdonek]

Thanks. I'm not sure if you prefer using GitHub's inline commenting feature. But my only comment is: are you really supposed to be using assertIn() here? The method name and documentation suggest that an exact match is required (and that was my understanding before). If it really should be assertIn(), perhaps the documentation can be updated to clarify?

[Chris Jerdonek]

From the docstring: "Asserts that the message in a raised exception matches the passed value."

[Tim Graham]

Yes, that's the current behavior since it uses a regular expression match. Some tests in Django's test suite rely on this, so I don't think changing it would be a good idea. I'll update the docs.

[Tim Graham <timograham@…>]

In 1c7c782:

Fixed #25188 -- Improved message raised by SimpleTestCase.assertRaisesMessage().

Thanks Chris Jerdonek for the suggestion and help with the patch.