Skip to content

Do not use ZeroDivisionError in tests for unrelated things #119989

Closed as not planned

Description

@sobolevn

Feature or enhancement

Refs #119066

I'm a little horrified at how often we use ZeroDivisionError in random tests that have nothing to do with arithmetic, but that's another story ...

Probably, most of them should be converted to some custom exception.

Activity

  1. added
    type-featureA feature request or enhancement
    testsTests in the Lib/test dir
    on Jun 3, 2024
  2. terryjreedy commented on Jun 3, 2024

    @terryjreedy
    Member

    Specifically, #119066 (review).
    @mdickinson What are examples of horror-inducing uses?

  3. serhiy-storchaka commented on Jun 4, 2024

    @serhiy-storchaka
    Member

    1/0 is just the simplest way to raise a rare exception. We can use raise OtherRareException or even raise CustomError, but this is more verbose.

  4. terryjreedy commented on Jun 4, 2024

    @terryjreedy
    Member

    To answer my question: I routinely use something like a a to get a SyntaxError and specifically 1/0' for a 'generic' runtime error. That said, greppping Lib/test recursively for ZeroDivisionError` gets 524 hits. Some test classes raise the long name itself. For instance:

    # test_iter, 83-5.
    class BadIterableClass:
        def __iter__(self):
            raise ZeroDivisionError
    

    Others do not:

    # test_xml_etree, 2790-2.
    class BadElementPath(str):
        def __eq__(self, o):
            raise 1/0
    

    Regardless, the multiple tests using 'Bad' classs require the long error name. I could be persuaded that defining something like class FakeErr, raising that in the Bad class, and checking for the short name in tests would be clearer, as well as involving less writing and reading.

    This strikes me as a test style proposal that is plausible but needs discussion on Discord or Discourse before possibly revising PEP8 or Devguide.

    Serhiy, only a few in test_tkinter:

    test_tkinter\test_colorchooser.py: 49:             raise ZeroDivisionError
    test_tkinter\test_colorchooser.py: 53:             self.assertRaises(ZeroDivisionError, askcolor)
    test_tkinter\test_colorchooser.py: 59:             self.assertRaises(ZeroDivisionError, askcolor)
    test_tkinter\test_messagebox.py: 18:             raise ZeroDivisionError
    test_tkinter\test_messagebox.py: 22:             self.assertRaises(ZeroDivisionError, showinfo, "Spam", "Egg Information")
    test_tkinter\test_messagebox.py: 27:             self.assertRaises(ZeroDivisionError, showinfo, "Spam", "Egg Information")
    
  5. mdickinson commented on Jun 4, 2024

    @mdickinson
    Member

    @terryjreedy @sobolevn I apologise for causing unnecessary work and brain cycles here; I think I shouldn't have made the original comment, and definitely shouldn't have used the word "horrified".

    I do think that this is a (very minor) abuse of ZeroDivisionError, but my complaint is very much at nitpick level. That is, if I were reviewing a PR that used raise ZeroDivisionError I might suggest introducing and using a custom exception, but I don't think it's worth changing existing cases of this. We have better ways to spend core developer cycles.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    testsTests in the Lib/test dirtype-featureA feature request or enhancement

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions