zopefoundation / zope.exceptions

exceptions and implementations which are general purpose
https://zopeexceptions.readthedocs.io/
Other
0 stars 6 forks source link

Drop support for Python < 3.7 #28

Closed icemac closed 1 year ago

jugmac00 commented 1 year ago

Maybe add a 'pragma: no cover' there?

it should be straightforward to add a simple test for that branch, ie construct something that actually ends with a new line

icemac commented 1 year ago

Maybe add a 'pragma: no cover' there?

it should be straightforward to add a simple test for that branch, ie construct something that actually ends with a new line

It is not about having something ending in a newline but not ending in one. The existing test does not have a newline in the setup. So this if is more line a safeguard, so I decided to change the code a bit to get full coverage via a "trick".

icemac commented 1 year ago

Thank you for reviewing this PR. 😃

icemac commented 1 year ago

FWIW: Python 3.7 has gone out of security support a few days ago

Thank you @mauritsvanrees for bringing this up. I was not aware that the strong and beloved Python 3.7 is now dead. I created https://github.com/zopefoundation/meta/issues/200 to discuss how to handle end-of-life scenarios in the future.

mauritsvanrees commented 1 year ago

It is not about having something ending in a newline but not ending in one.

Aha, so that is what coverage was trying to say with 33->35 as missing: the condition on line 33 is always true in the tests, so line 34 is always executed, but there is no coverage for an immediate jump from line 33 to 35 (an invisible, empty else statement).

The existing test does not have a newline in the setup.

Actually it does. The s in the condition is the output of print_exception, which ends with a newline, for example this:

'Traceback (most recent call last):\n  File "dummy/filename.py", line 14, in dummy_function\n   - __traceback_info__: Have a Snowman: ☃\nValueError: testing\n'

I suppose it is possible that this differs per Python version.

So this if is more line a safeguard, so I decided to change the code a bit to get full coverage via a "trick".

The "trick" definitely makes sense.