Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test_functools unexpected failures when C _functoolsmodule is missing #90805

Closed
sobolevn opened this issue Feb 5, 2022 · 7 comments · Fixed by #109275
Closed

test_functools unexpected failures when C _functoolsmodule is missing #90805

sobolevn opened this issue Feb 5, 2022 · 7 comments · Fixed by #109275
Labels
3.10 only security fixes 3.11 only security fixes tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error

Comments

@sobolevn
Copy link
Member

sobolevn commented Feb 5, 2022

BPO 46647
Nosy @rhettinger, @tiran, @shihai1991, @sobolevn
PRs
  • bpo-46647: make sure test_functools works with and without _functoolsmodule #31141
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2022-02-08.04:43:59.900>
    created_at = <Date 2022-02-05.10:46:54.761>
    labels = ['type-bug', 'tests', '3.10', '3.11']
    title = '`test_functools` unexpected failures when C `_functoolsmodule` is missing'
    updated_at = <Date 2022-02-08.04:43:59.899>
    user = 'https://github.com/sobolevn'

    bugs.python.org fields:

    activity = <Date 2022-02-08.04:43:59.899>
    actor = 'sobolevn'
    assignee = 'none'
    closed = True
    closed_date = <Date 2022-02-08.04:43:59.900>
    closer = 'sobolevn'
    components = ['Tests']
    creation = <Date 2022-02-05.10:46:54.761>
    creator = 'sobolevn'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 46647
    keywords = ['patch']
    message_count = 5.0
    messages = ['412565', '412572', '412573', '412767', '412809']
    nosy_count = 4.0
    nosy_names = ['rhettinger', 'christian.heimes', 'shihai1991', 'sobolevn']
    pr_nums = ['31141']
    priority = 'normal'
    resolution = 'wont fix'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue46647'
    versions = ['Python 3.10', 'Python 3.11']

    Linked PRs

    @sobolevn
    Copy link
    Member Author

    sobolevn commented Feb 5, 2022

    Reproduction steps:

    1. Add to Setup.local:
    *disabled*
    _functoolsmodule
    
    1. .configure && make -j. Then, ensure that this module is not available:
    » ./python.exe -c 'import _functools'                                         
    Traceback (most recent call last):
      File "<string>", line 1, in <module>
    ModuleNotFoundError: No module named '_functools'
    
    1. Run test_functools:
    ======================================================================
    ERROR: test_bad_cmp (test.test_functools.TestCmpToKeyC)
    ----------------------------------------------------------------------
    Traceback (most recent call last):
      File "/Users/sobolev/Desktop/cpython/Lib/test/test_functools.py", line 905, in test_bad_cmp
        key = self.cmp_to_key(cmp1)
              ^^^^^^^^^^^^^^^^^^^^^
    TypeError: cmp_to_key() takes 1 positional argument but 2 were given
    
    ======================================================================
    ERROR: test_cmp_to_key (test.test_functools.TestCmpToKeyC)
    ----------------------------------------------------------------------
    Traceback (most recent call last):
      File "/Users/sobolev/Desktop/cpython/Lib/test/test_functools.py", line 869, in test_cmp_to_key
        key = self.cmp_to_key(cmp1)
              ^^^^^^^^^^^^^^^^^^^^^
    TypeError: cmp_to_key() takes 1 positional argument but 2 were given
    
    ======================================================================
    ERROR: test_cmp_to_key_arguments (test.test_functools.TestCmpToKeyC)
    ----------------------------------------------------------------------
    Traceback (most recent call last):
      File "/Users/sobolev/Desktop/cpython/Lib/test/test_functools.py", line 885, in test_cmp_to_key_arguments
        key = self.cmp_to_key(mycmp=cmp1)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^
    TypeError: cmp_to_key() got multiple values for argument 'mycmp'
    
    ======================================================================
    ERROR: test_hash (test.test_functools.TestCmpToKeyC)
    ----------------------------------------------------------------------
    Traceback (most recent call last):
      File "/Users/sobolev/Desktop/cpython/Lib/test/test_functools.py", line 941, in test_hash
        key = self.cmp_to_key(mycmp)
              ^^^^^^^^^^^^^^^^^^^^^^
    TypeError: cmp_to_key() takes 1 positional argument but 2 were given
    
    ======================================================================
    ERROR: test_obj_field (test.test_functools.TestCmpToKeyC)
    ----------------------------------------------------------------------
    Traceback (most recent call last):
      File "/Users/sobolev/Desktop/cpython/Lib/test/test_functools.py", line 920, in test_obj_field
        key = self.cmp_to_key(mycmp=cmp1)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^
    TypeError: cmp_to_key() got multiple values for argument 'mycmp'
    
    ======================================================================
    ERROR: test_sort_int (test.test_functools.TestCmpToKeyC)
    ----------------------------------------------------------------------
    Traceback (most recent call last):
      File "/Users/sobolev/Desktop/cpython/Lib/test/test_functools.py", line 926, in test_sort_int
        self.assertEqual(sorted(range(5), key=self.cmp_to_key(mycmp)),
                                              ^^^^^^^^^^^^^^^^^^^^^^
    TypeError: cmp_to_key() takes 1 positional argument but 2 were given
    
    ======================================================================
    ERROR: test_sort_int_str (test.test_functools.TestCmpToKeyC)
    ----------------------------------------------------------------------
    Traceback (most recent call last):
      File "/Users/sobolev/Desktop/cpython/Lib/test/test_functools.py", line 934, in test_sort_int_str
        values = sorted(values, key=self.cmp_to_key(mycmp))
                                    ^^^^^^^^^^^^^^^^^^^^^^
    TypeError: cmp_to_key() takes 1 positional argument but 2 were given
    
    ======================================================================
    ERROR: test_pickle (test.test_functools.TestPartialC)
    ----------------------------------------------------------------------
    Traceback (most recent call last):
      File "/Users/sobolev/Desktop/cpython/Lib/test/test_functools.py", line 258, in test_pickle
        f_copy = pickle.loads(pickle.dumps(f, proto))
                              ^^^^^^^^^^^^^^^^^^^^^^
    _pickle.PicklingError: Can't pickle <class 'functools.partial'>: it's not the same object as functools.partial
    
    ======================================================================
    ERROR: test_recursive_pickle (test.test_functools.TestPartialC)
    ----------------------------------------------------------------------
    Traceback (most recent call last):
      File "/Users/sobolev/Desktop/cpython/Lib/test/test_functools.py", line 343, in test_recursive_pickle
        pickle.dumps(f, proto)
        ^^^^^^^^^^^^^^^^^^^^^^
    _pickle.PicklingError: Can't pickle <class 'functools.partial'>: it's not the same object as functools.partial
    
    ======================================================================
    ERROR: test_iterator_usage (test.test_functools.TestReduceC)
    ----------------------------------------------------------------------
    Traceback (most recent call last):
      File "/Users/sobolev/Desktop/cpython/Lib/test/test_functools.py", line 843, in test_iterator_usage
        self.assertEqual(self.reduce(add, SequenceClass(5)), 10)
                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      File "/Users/sobolev/Desktop/cpython/Lib/functools.py", line 249, in reduce
        it = iter(sequence)
             ^^^^^^^^^^^^^^
    TypeError: 'builtin_function_or_method' object is not iterable
    
    ======================================================================
    ERROR: test_reduce (test.test_functools.TestReduceC)
    ----------------------------------------------------------------------
    Traceback (most recent call last):
      File "/Users/sobolev/Desktop/cpython/Lib/test/test_functools.py", line 794, in test_reduce
        self.assertEqual(self.reduce(add, ['a', 'b', 'c'], ''), 'abc')
                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    TypeError: reduce() takes from 2 to 3 positional arguments but 4 were given
    
    ======================================================================
    FAIL: test_disallow_instantiation (test.test_functools.TestCmpToKeyC)
    ----------------------------------------------------------------------
    TypeError: type() takes 1 or 3 arguments
    
    During handling of the above exception, another exception occurred:
    
    Traceback (most recent call last):
      File "/Users/sobolev/Desktop/cpython/Lib/test/test_functools.py", line 955, in test_disallow_instantiation
        support.check_disallow_instantiation(
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      File "/Users/sobolev/Desktop/cpython/Lib/test/support/__init__.py", line 2121, in check_disallow_instantiation
        testcase.assertRaisesRegex(TypeError, msg, tp, *args, **kwds)
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    AssertionError: "cannot create 'type' instances" does not match "type() takes 1 or 3 arguments"
    
    ======================================================================
    FAIL: test_attributes_unwritable (test.test_functools.TestPartialC)
    ----------------------------------------------------------------------
    Traceback (most recent call last):
      File "/Users/sobolev/Desktop/cpython/Lib/test/test_functools.py", line 402, in test_attributes_unwritable
        self.assertRaises(AttributeError, setattr, p, 'func', map)
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    AssertionError: AttributeError not raised by setattr
    
    ======================================================================
    FAIL: test_attributes_unwritable (test.test_functools.TestPartialCSubclass)
    ----------------------------------------------------------------------
    Traceback (most recent call last):
      File "/Users/sobolev/Desktop/cpython/Lib/test/test_functools.py", line 402, in test_attributes_unwritable
        self.assertRaises(AttributeError, setattr, p, 'func', map)
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    AssertionError: AttributeError not raised by setattr
    
    ----------------------------------------------------------------------
    Ran 249 tests in 0.690s
    
    FAILED (failures=3, errors=11)
    test test_functools failed
    test_functools failed (11 errors, 3 failures)
    
    == Tests result: FAILURE ==
    
    1 test failed:
        test_functools
    
    Total duration: 1.3 sec
    Tests result: FAILURE
    

    List of individual problems:

    1. This function is defined assuming that c_functools always has .lru_cache:
      @c_functools.lru_cache()
      def c_cached_func(x, y):
      return 3 * x + y
    2. TestLRUC is never skipped:
      class TestLRUC(TestLRU, unittest.TestCase):
      module = c_functools
      cached_func = c_cached_func,
      I think it should be, because there's no need to test _lru_cache_wrapper twice for just python implementation (default if _functools is missing)
    3. All similar modules tend to use fresh= in import_fresh_module, for example:
      py_typing = import_helper.import_fresh_module('typing', blocked=['_typing'])
      c_typing = import_helper.import_fresh_module('typing', fresh=['_typing'])
      But, test_functools does not do this:
      c_functools = import_helper.import_fresh_module('functools')
      So, even if _functools is missing, c_functools will not be None, it will still be functools.py module! And this causes multiple unexpected test failures above

    Related:

    I will send a patch for this in a moment.

    @sobolevn sobolevn added 3.10 only security fixes 3.11 only security fixes tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error labels Feb 5, 2022
    @tiran
    Copy link
    Member

    tiran commented Feb 5, 2022

    _functoolsmodule is a core bootstrap module and defined in Modules/Setup.bootstrap. It has no external dependencies. There is no reason and no point to disable the module.

    We can safely assume that all modules defined in Modules/Setup.bootstrap are always available. (Maybe except for pwd, but that is a different story.)

    I propose to close the bug and PR as wontfix.

    @sobolevn
    Copy link
    Member Author

    sobolevn commented Feb 5, 2022

    Cristian, in this case - is there a reason to keep skipUnless(c_functools) around?

    If we are sure that it is always available - then it should be always tested.

    We either should have:

    1. Cleanly defined skips that work (this PR)
    2. Unconditional coverage

    Or do you think that some middle-ground is possible?

    @shihai1991
    Copy link
    Member

    _functoolsmodule is a core bootstrap module and defined in Modules/Setup.bootstrap. It has no external dependencies. There is no reason and no point to disable the module.

    +1.

    Cristian, in this case - is there a reason to keep skipUnless(c_functools) around?
    If we are sure that it is always available - then it should be always tested.

    Hm. Personally, I suggest to keep the skipUnless to aovid the code churn.
    Or maybe you have other cases to show the functools module will missing unexpectly?

    @sobolevn
    Copy link
    Member Author

    sobolevn commented Feb 8, 2022

    Or maybe you have other cases to show the functools module will missing unexpectly?

    No, I can't think of any :)

    Your argument about code churn also makes sense.
    But, if we ever are going to refactor this test module, this is something to remember of.

    Thanks everyone!

    @sobolevn sobolevn closed this as completed Feb 8, 2022
    @sobolevn sobolevn closed this as completed Feb 8, 2022
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @sobolevn
    Copy link
    Member Author

    Reopened: #108638 (comment)

    @sobolevn sobolevn reopened this Aug 29, 2023
    sobolevn added a commit to sobolevn/cpython that referenced this issue Aug 29, 2023
    miss-islington pushed a commit to miss-islington/cpython that referenced this issue Sep 11, 2023
    …toolsmodule (pythonGH-108644)
    
    (cherry picked from commit baa6dc8)
    
    Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
    miss-islington pushed a commit to miss-islington/cpython that referenced this issue Sep 11, 2023
    …toolsmodule (pythonGH-108644)
    
    (cherry picked from commit baa6dc8)
    
    Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
    serhiy-storchaka pushed a commit that referenced this issue Sep 11, 2023
    …ctoolsmodule (GH-108644) (GH-109274)
    
    (cherry picked from commit baa6dc8)
    
    Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
    Yhg1s pushed a commit that referenced this issue Sep 12, 2023
    …ctoolsmodule (GH-108644) (#109275)
    
    gh-90805: Make sure test_functools works with and without _functoolsmodule (GH-108644)
    (cherry picked from commit baa6dc8)
    
    Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
    @hugovk
    Copy link
    Member

    hugovk commented Nov 10, 2023

    Thanks!

    @hugovk hugovk closed this as completed Nov 10, 2023
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.10 only security fixes 3.11 only security fixes tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    4 participants