4.3.0: test_ctrl_c_clears fails

[dvzrv]

Hi! While trying to package 4.3.0 for Arch Linux I ran into test failures due to urwid:

=================================== FAILURES ===================================
______________________________ test_ctrl_c_clears ______________________________

default_formatter = <todoman.formatters.DefaultFormatter object at 0x7f2808959810>
todo_factory = <function todo_factory.<locals>.inner at 0x7f280880a700>

    def test_ctrl_c_clears(default_formatter, todo_factory):
        todo = todo_factory()
        editor = TodoEditor(todo, [todo.list], default_formatter)

        # Simulate that ctrl+c gets pressed, since we can't *really* do that
        # trivially inside unit tests.
>       with mock.patch(
            "urwid.main_loop.MainLoop.run", side_effect=KeyboardInterrupt
        ), mock.patch(
            "urwid.main_loop.MainLoop.stop",
        ) as mocked_stop:

tests/test_ui.py:147:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
/usr/lib/python3.11/unittest/mock.py:1421: in __enter__
    self.target = self.getter()
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

name = 'urwid.main_loop.MainLoop'

    def resolve_name(name):
        """
        Resolve a name to an object.

        It is expected that `name` will be a string in one of the following
        formats, where W is shorthand for a valid Python identifier and dot stands
        for a literal period in these pseudo-regexes:

        W(.W)*
        W(.W)*:(W(.W)*)?

        The first form is intended for backward compatibility only. It assumes that
        some part of the dotted name is a package, and the rest is an object
        somewhere within that package, possibly nested inside other objects.
        Because the place where the package stops and the object hierarchy starts
        can't be inferred by inspection, repeated attempts to import must be done
        with this form.

        In the second form, the caller makes the division point clear through the
        provision of a single colon: the dotted name to the left of the colon is a
        package to be imported, and the dotted name to the right is the object
        hierarchy within that package. Only one import is needed in this form. If
        it ends with the colon, then a module object is returned.

        The function will return an object (which might be a module), or raise one
        of the following exceptions:

        ValueError - if `name` isn't in a recognised format
        ImportError - if an import failed when it shouldn't have
        AttributeError - if a failure occurred when traversing the object hierarchy
                         within the imported package to get to the desired object.
        """
        global _NAME_PATTERN
        if _NAME_PATTERN is None:
            # Lazy import to speedup Python startup time
            import re
            dotted_words = r'(?!\d)(\w+)(\.(?!\d)(\w+))*'
            _NAME_PATTERN = re.compile(f'^(?P<pkg>{dotted_words})'
                                       f'(?P<cln>:(?P<obj>{dotted_words})?)?$',
                                       re.UNICODE)

        m = _NAME_PATTERN.match(name)
        if not m:
            raise ValueError(f'invalid format: {name!r}')
        gd = m.groupdict()
        if gd.get('cln'):
            # there is a colon - a one-step import is all that's needed
            mod = importlib.import_module(gd['pkg'])
            parts = gd.get('obj')
            parts = parts.split('.') if parts else []
        else:
            # no colon - have to iterate to find the package boundary
            parts = name.split('.')
            modname = parts.pop(0)
            # first part *must* be a module/package.
            mod = importlib.import_module(modname)
            while parts:
                p = parts[0]
                s = f'{modname}.{p}'
                try:
                    mod = importlib.import_module(s)
                    parts.pop(0)
                    modname = s
                except ImportError:
                    break
        # if we reach this point, mod is the module, already imported, and
        # parts is the list of parts in the object hierarchy to be traversed, or
        # an empty list if just the module is wanted.
        result = mod
        for p in parts:
>           result = getattr(result, p)
E           AttributeError: module 'urwid' has no attribute 'main_loop'

/usr/lib/python3.11/pkgutil.py:715: AttributeError
=============================== warnings summary ===============================
tests/test_cli.py: 10 warnings
tests/test_ui.py: 27 warnings
  /usr/lib/python3.11/site-packages/urwid/wimp.py:267: PendingDeprecationWarning: only for backwards compatibility.You may also use the new standard container property `focus_position` to get the focus.
    self._w.focus_col = 0

tests/test_cli.py: 7 warnings
tests/test_ui.py: 12 warnings
  /usr/lib/python3.11/site-packages/urwid/signals.py:168: DeprecationWarning: Don't use user_arg argument, use user_args instead.
    warnings.warn(

../../../../usr/lib/python3.11/site-packages/_pytest/cacheprovider.py:451
  /usr/lib/python3.11/site-packages/_pytest/cacheprovider.py:451: PytestCacheWarning: could not create cache path /dev/.pytest_cache/v/cache/nodeids: [Errno 13] Permission denied: '/dev/.pytest_cache'
    config.cache.set("cache/nodeids", sorted(self.cached_nodeids))

../../../../usr/lib/python3.11/site-packages/_pytest/cacheprovider.py:405
  /usr/lib/python3.11/site-packages/_pytest/cacheprovider.py:405: PytestCacheWarning: could not create cache path /dev/.pytest_cache/v/cache/lastfailed: [Errno 13] Permission denied: '/dev/.pytest_cache'
    config.cache.set("cache/lastfailed", self.lastfailed)

../../../../usr/lib/python3.11/site-packages/_pytest/stepwise.py:56
  /usr/lib/python3.11/site-packages/_pytest/stepwise.py:56: PytestCacheWarning: could not create cache path /dev/.pytest_cache/v/cache/stepwise: [Errno 13] Permission denied: '/dev/.pytest_cache'
    session.config.cache.set(STEPWISE_CACHE_DIR, [])

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
=========================== short test summary info ============================
FAILED ../../../../dev/tests/test_ui.py::test_ctrl_c_clears - AttributeError:...
======= 1 failed, 190 passed, 5 skipped, 3 xfailed, 59 warnings in 4.66s =======

Due to CVEs in ncurses I had to upgrade urwid to a commit past its last release:
https://gitlab.archlinux.org/archlinux/packaging/packages/python-urwid/-/commits/main

Full build/test logs:
todoman-4.3.0-1-x86_64-build.log
todoman-4.3.0-1-x86_64-check.log

[WhyNotHugo]

Looking at the patches applied on arch with git log release-2.1.2..3c21a429d7275aa79069c7a331fdb3de9138d2b2 -p, I suspect that this commit introduces the issue.

commit d1710f0983e86dc5ec06efec6e94f3b6d204bbb3
Author: Alexey Stepanov <[email protected]>
Date:   Tue Apr 18 17:35:19 2023 +0200

    [BREAKING CHANGE] Refactoring: Split event loop in several modules (#537)
    
    * [BREAKING CHANGE] Refactoring: Split event loop in several modules
    
    * `urwid.main_loop` is split into multiple modules which is easier to maintain
    * `urwid.compat` is not used anymore and removed
    * `TornadoEventLoop`, `GLibEventLoop`, `TwistedEventLoop` and `TrioEventLoop`
      accessible ONLY if required dependencies installed
      (like: Tornado installed -> `TornadoEventLoop` is accessible for import)
    * `TornadoEventLoop` use the same idle logic as `AsyncioLoop`:
      tornado.ioloop.IOLoop is asyncio based.
    * Trio < 0.15 is not supported. Version 0.15 was released almost 3 years ago.
    * Tornado < 5.0 is not supported. Tornado 5.0 was released 5 years ago.
    
    * Remove useless shebang
    
    * `EventLoop` should be real abstract
    * add new module docstrings
    
    * Fix docstrings
    
    * remove unneeded import
    
    ---------
    
    Co-authored-by: Aleksei Stepanov <[email protected]>

I'm not sure what to do here. Is urwid-upstream planning a new release with breaking changes to fix the CVEs?

[dvzrv]

I think they are working on things actively atm, but it appears they are also introducing some breakage (sometimes accidentally).

I'll disable the test for now.

[WhyNotHugo]

Leaving the test disabled for now sounds good to me.

I don't think that I'll address this until upstream provides a new release (given that it's a moving target and it's unclear if it will contain breaking changes or not).

Looks like this only affects the tests anyway.