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 suite is brittle around global sysconfig._config_vars state. #231

Closed
jaraco opened this issue Mar 2, 2024 · 1 comment · Fixed by #233 or #234
Closed

Test suite is brittle around global sysconfig._config_vars state. #231

jaraco opened this issue Mar 2, 2024 · 1 comment · Fixed by #233 or #234

Comments

@jaraco
Copy link
Member

jaraco commented Mar 2, 2024

I applied that diff, but unfortunately, that led to more failures in test_cc_overrides_ldshared_for_cxx_correctly... and while debugging the issue, I found that test is itself fragile - it fails differently depending on whether it's run in isolation or not (even if before the patch).

 distutils 765bbbd7 @ tox -e py311 -- -p no:cov -k test_cc_overrides_ldshared_for_cxx_correctly
.pkg-cpython311: _optional_hooks> python '/Users/jaraco/Library/Application Support/pipx/venvs/tox/lib/python3.12/site-packages/pyproject_api/_backend.py' True setuptools.build_meta
.pkg-cpython311: get_requires_for_build_editable> python '/Users/jaraco/Library/Application Support/pipx/venvs/tox/lib/python3.12/site-packages/pyproject_api/_backend.py' True setuptools.build_meta
.pkg-cpython311: build_editable> python '/Users/jaraco/Library/Application Support/pipx/venvs/tox/lib/python3.12/site-packages/pyproject_api/_backend.py' True setuptools.build_meta
py311: install_package> python -I -m pip install --force-reinstall --no-deps /Users/jaraco/code/pypa/distutils/.tox/.tmp/package/20/distutils-0.1.dev3944+g765bbbd-0.editable-py3-none-any.whl
py311: commands[0]> pytest -p no:cov -k test_cc_overrides_ldshared_for_cxx_correctly
============================================================== test session starts ===============================================================
platform darwin -- Python 3.11.8, pytest-8.0.2, pluggy-1.4.0
cachedir: .tox/py311/.pytest_cache
rootdir: /Users/jaraco/code/pypa/distutils
configfile: pytest.ini
plugins: enabler-3.0.0, checkdocs-2.10.1, pyfakefs-5.3.5, ruff-0.2.1
collected 478 items / 477 deselected / 1 selected                                                                                                

distutils/tests/test_unixccompiler.py F                                                                                                    [100%]

==================================================================== FAILURES ====================================================================
_________________________________________ TestUnixCCompiler.test_cc_overrides_ldshared_for_cxx_correctly _________________________________________

self = <distutils.tests.test_unixccompiler.TestUnixCCompiler object at 0x1031f8b50>

    @pytest.mark.skipif('platform.system == "Windows"')
    def test_cc_overrides_ldshared_for_cxx_correctly(self):
        """
        Ensure that setting CC env variable also changes default linker
        correctly when building C++ extensions.
    
        pypa/distutils#126
        """
    
        def gcv(v):
            if v == 'LDSHARED':
                return 'gcc-4.2 -bundle -undefined dynamic_lookup '
            elif v == 'LDCXXSHARED':
                return 'g++-4.2 -bundle -undefined dynamic_lookup '
            elif v == 'CXX':
                return 'g++-4.2'
            elif v == 'CC':
                return 'gcc-4.2'
            return ''
    
        def gcvs(*args, _orig=sysconfig.get_config_vars):
            if args:
                return list(map(sysconfig.get_config_var, args))
            return _orig()
    
        sysconfig.get_config_var = gcv
        sysconfig.get_config_vars = gcvs
        with mock.patch.object(
            self.cc, 'spawn', return_value=None
        ) as mock_spawn, mock.patch.object(
            self.cc, '_need_link', return_value=True
        ), mock.patch.object(
            self.cc, 'mkpath', return_value=None
        ), EnvironmentVarGuard() as env:
            env['CC'] = 'ccache my_cc'
            env['CXX'] = 'my_cxx'
            del env['LDSHARED']
>           sysconfig.customize_compiler(self.cc)

distutils/tests/test_unixccompiler.py:275: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

compiler = <distutils.tests.test_unixccompiler.compiler_wrapper.<locals>.CompilerWrapper object at 0x1031f8f50>

    def customize_compiler(compiler):  # noqa: C901
        """Do any platform-specific customization of a CCompiler instance.
    
        Mainly needed on Unix, so we can plug in the information that
        varies across Unices and is stored in Python's Makefile.
        """
        if compiler.compiler_type == "unix":
            if sys.platform == "darwin":
                # Perform first-time customization of compiler-related
                # config vars on OS X now that we know we need a compiler.
                # This is primarily to support Pythons from binary
                # installers.  The kind and paths to build tools on
                # the user system may vary significantly from the system
                # that Python itself was built on.  Also the user OS
                # version and build tools may not support the same set
                # of CPU architectures for universal builds.
                global _config_vars
                # Use get_config_var() to ensure _config_vars is initialized.
                if not get_config_var('CUSTOMIZED_OSX_COMPILER'):
                    import _osx_support
    
                    _osx_support.customize_compiler(_config_vars)
>                   _config_vars['CUSTOMIZED_OSX_COMPILER'] = 'True'
E                   TypeError: 'NoneType' object does not support item assignment

distutils/sysconfig.py:291: TypeError
============================================================ short test summary info =============================================================
FAILED distutils/tests/test_unixccompiler.py::TestUnixCCompiler::test_cc_overrides_ldshared_for_cxx_correctly - TypeError: 'NoneType' object does not support item assignment
======================================================= 1 failed, 477 deselected in 0.16s ========================================================
py311: exit 1 (0.30 seconds) /Users/jaraco/code/pypa/distutils> pytest -p no:cov -k test_cc_overrides_ldshared_for_cxx_correctly pid=70507
.pkg-cpython311: _exit> python '/Users/jaraco/Library/Application Support/pipx/venvs/tox/lib/python3.12/site-packages/pyproject_api/_backend.py' True setuptools.build_meta
  py311: FAIL code 1 (0.86=setup[0.56]+cmd[0.30] seconds)
  evaluation failed :( (0.90 seconds)

Probably we should figure out why that test is reliant on other tests and fix that too.

Originally posted by @jaraco in #228 (comment)

The issue seems to be surfaced by this change, where the fallback value is now '', which allows the darwin-specific behavior to be reached:

global _config_vars
# Use get_config_var() to ensure _config_vars is initialized.
if not get_config_var('CUSTOMIZED_OSX_COMPILER'):
import _osx_support
_osx_support.customize_compiler(_config_vars)
_config_vars['CUSTOMIZED_OSX_COMPILER'] = 'True'

That code attempts to mutate a global variable, which, if it wasn't initialized by another test, will lead to the error.

I believe the proposed diff is safer and more correct. We'll want to fix that undesirable entanglement.

Originally posted by @jaraco in #228 (comment)

@jaraco
Copy link
Member Author

jaraco commented Mar 2, 2024

Here's another case where the problem could have been encountered except that the test ensures that the global variable is initialized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant