ebook-utils / css-parser

CSS related utilities (parsing, serialization, etc) for python
GNU Lesser General Public License v3.0
31 stars 5 forks source link

test: relax assertRaisesMsg to match longer strings #8

Closed keszybz closed 3 years ago

keszybz commented 3 years ago

With python3.10, we get the following failure:

======================================================================
FAIL: test_literalname (css_parser_tests.test_property.PropertyTestCase)
Property.literalname
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/builddir/build/BUILD/css-parser-1.0.6/css_parser_tests/basetest.py", line 168, in assertRaisesMsg
    callableObj(*args, **kwargs)
AttributeError: can't set attribute 'literalname'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/builddir/build/BUILD/css-parser-1.0.6/css_parser_tests/test_property.py", line 165, in test_literalname
    self.assertRaisesMsg(AttributeError, "can't set attribute", p.__setattr__,
  File "/builddir/build/BUILD/css-parser-1.0.6/css_parser_tests/basetest.py", line 179, in assertRaisesMsg
    raise self.failureException(
AssertionError: Right exception, wrong message: got 'can't set attribute 'literalname'' instead of 'can't set attribute'

======================================================================
FAIL: test_specificity (css_parser_tests.test_selector.SelectorTestCase)
Selector.specificity
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/builddir/build/BUILD/css-parser-1.0.6/css_parser_tests/basetest.py", line 168, in assertRaisesMsg
    callableObj(*args, **kwargs)
  File "/builddir/build/BUILD/css-parser-1.0.6/css_parser_tests/test_selector.py", line 414, in _set
    def _set(): selector.specificity = 1
AttributeError: can't set attribute 'specificity'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/builddir/build/BUILD/css-parser-1.0.6/css_parser_tests/test_selector.py", line 415, in test_specificity
    self.assertRaisesMsg(AttributeError, "can't set attribute", _set)
  File "/builddir/build/BUILD/css-parser-1.0.6/css_parser_tests/basetest.py", line 179, in assertRaisesMsg
    raise self.failureException(
AssertionError: Right exception, wrong message: got 'can't set attribute 'specificity'' instead of 'can't set attribute'

----------------------------------------------------------------------

By checking if the argument matches just a substring, we can match those cases without making the code more complicated. Matching just for substrings in exception messages is pretty common.

kovidgoyal commented 3 years ago

It's not clear to me that relaxing the test would not cause false positives. A less disruptive change would be to add a keyword arg to assertRaisesMessages to allow substring matches and default it to false, explicitly turning it on only for those tests that fail. Unless all such tests fail, in which case your PR is fine.

keszybz commented 3 years ago

It's only the two matches shown in the commit message that fail like that.

If you prefer, I can rework it to have a keyword arg... I didn't think it was worth it the trouble, because if the whole test string matches, having a false positive where the string unexpectedly changes while still matching the whole older value of the string should be quite rare. I.e. if the exception message is broken / unexpectedly changed, I expect the match for the substring will fail too.

kovidgoyal commented 3 years ago

yes, I prefer a keyword arg, thanks.

keszybz commented 3 years ago

I guess python2 compat is still necessary? With a keyword-only argument this works nicely, but it's not compatible with python2 :(

diff --git css_parser_tests/basetest.py css_parser_tests/basetest.py
index 2b269064a1..0410416bfa 100644
--- css_parser_tests/basetest.py
+++ css_parser_tests/basetest.py
@@ -149,7 +149,7 @@ class BaseTestCase(unittest.TestCase):
         else:
             self.fail("%s did not raise %s" % (callsig, exception))

-    def assertRaisesMsg(self, excClass, msg, callableObj, *args, **kwargs):
+    def assertRaisesMsg(self, excClass, msg, callableObj, *args, substring_match=False, **kwargs):
         """
         Just like unittest.TestCase.assertRaises,
         but checks that the message is right too.
@@ -158,8 +158,9 @@ class BaseTestCase(unittest.TestCase):

             self.assertRaisesMsg(
                 MyException, "Exception message",
-                my_function, (arg1, arg2)
-                )
+                my_function, arg1, arg2,
+                kwarg1=val, kwarg2=val,
+                substring_match=True|False)

         from
         http://www.nedbatchelder.com/blog/200609.html#e20060905T064418
@@ -171,7 +172,7 @@ class BaseTestCase(unittest.TestCase):
             if not msg:
                 # No message provided: any message is fine.
                 return
-            elif excMsg == msg:
+            elif (msg in excMsg if substring_match else msg == excMsg):
                 # Message provided, and we got the right message: passes.
                 return
             else:
keszybz commented 3 years ago

This works here with python2.7-2.7.18-6.fc33.x86_64 and python3-3.9.1-1.fc33.x86_64.