xfail symlink tests in hole2

[IAlibay]

Expected behavior

All tests should pass.

Actual behavior

On Windows it's quite common for folks to get permission errors on symlink creation leading to test failures.

Since it's a known failure, it would be good if we just xfailed the relevant tests.

Code to reproduce the behavior

run tests on win 10 or 11 (in my case using anaconda prompt, not sure about @jbarnoud)

Current version of MDAnalysis

• Which version are you using? 2.1.0
• Which version of Python (python -V)? 3.7-3.10
• Which operating system? Win 10-11

[jbarnoud]

(in my case using anaconda prompt, not sure about @jbarnoud)

I am running with miniconda in windows terminal.

The test failures are:

________________ TestCheckAndFixLongFilename.test_symlink_file ________________
[gw6] win32 -- Python 3.7.11 C:\Users\trash\miniconda3\envs\test210-37\python.exe

self = <MDAnalysisTests.analysis.test_hole2.TestCheckAndFixLongFilename object at 0x0000023845C255C8>
tmpdir = local('C:\\Users\\trash\\AppData\\Local\\Temp\\pytest-of-trash\\pytest-1\\popen-gw6\\test_symlink_file0')

    @pytest.mark.skipif(os.name == 'nt' and sys.maxsize <= 2**32,
                        reason="OSError: symbolic link privilege not held")
    def test_symlink_file(self, tmpdir):
        long_name = 'a'*10 + self.filename

        with tmpdir.as_cwd():
>           fixed = check_and_fix_long_filename(long_name)

C:\Users\trash\miniconda3\envs\test210-37\lib\site-packages\MDAnalysisTests\analysis\test_hole2.py:96:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

filename = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.pdb'
tmpdir = '.', max_length = 70, make_symlink = True

    def check_and_fix_long_filename(filename, tmpdir=os.path.curdir,
                                    max_length=70,
                                    make_symlink=True):
        """Return a file name suitable for HOLE.

        HOLE is limited to filenames <= ``max_length``. This method

        1. returns `filename` if HOLE can process it
        2. returns a relative path (see :func:`os.path.relpath`) if that shortens the
            path sufficiently
        3. creates a symlink to `filename` (:func:`os.symlink`) in a safe temporary
            directory and returns the path of the symlink.

        Parameters
        ----------
        filename : str
            file name to be processed
        tmpdir : str, optional
            By default the temporary directory is created inside the current
            directory in order to keep that path name short. This can be changed
            with the `tmpdir` keyword (e.g. one can use "/tmp"). The default is
            the current directory :data:`os.path.curdir`.

        Returns
        -------
        str
            path to the file that has a length less than
            ``max_length``

        Raises
        ------
        RuntimeError
            If none of the tricks for filename shortening worked. In this case,
            manually rename the file or recompile your version of HOLE.
        """

        if len(filename) <= max_length:
            return filename

        msg = ('HOLE will not read {} '
               'because it has more than {} characters.')
        logger.debug(msg.format(filename, max_length))

        # try a relative path
        new_name = os.path.relpath(filename)
        if len(new_name) <= max_length:
            msg = 'Using relative path: {} -> {}'
            logger.debug(msg.format(filename, new_name))
            return new_name

        if make_symlink:
            # shorten path by creating a symlink inside a safe temp dir
            _, ext = os.path.splitext(filename)
            dirname = tempfile.mkdtemp(dir=tmpdir)
            newname = os.path.join(dirname, os.path.basename(filename))
            if len(newname) > max_length:
                fd, newname = tempfile.mkstemp(suffix=ext, dir=dirname)
                os.close(fd)
                os.unlink(newname)

            if len(newname) > max_length:
                newname = os.path.relpath(newname)

            if len(newname) <= max_length:
>               os.symlink(filename, newname)
E               OSError: symbolic link privilege not held

C:\Users\trash\miniconda3\envs\test210-37\lib\site-packages\MDAnalysis\analysis\hole2\utils.py:139: OSError

and

____________ TestHydrogenBondAnalysisIdeal.test_logging_step_not_1 ____________
[gw5] win32 -- Python 3.7.11 C:\Users\trash\miniconda3\envs\test210-37\python.exe

self = <MDAnalysisTests.analysis.test_hydrogenbonds_analysis.TestHydrogenBondAnalysisIdeal object at 0x0000022C45736848>
universe = <Universe with 6 atoms>
caplog = <_pytest.logging.LogCaptureFixture object at 0x0000022C510C4748>

    def test_logging_step_not_1(self, universe, caplog):
        hbonds = HydrogenBondAnalysis(universe, **self.kwargs)
        # using step 2
        hbonds.run(step=2)

        caplog.set_level(logging.WARNING)
        hbonds.lifetime(tau_max=2, intermittency=1)

        warning = \
            "Autocorrelation: Hydrogen bonds were computed with step > 1."
>       assert any(warning in rec.getMessage() for rec in caplog.records)
E       assert False
E        +  where False = any(<generator object TestHydrogenBondAnalysisIdeal.test_logging_step_not_1.<locals>.<genexpr> at 0x0000022C572B2AC8>)

C:\Users\trash\miniconda3\envs\test210-37\lib\site-packages\MDAnalysisTests\analysis\test_hydrogenbonds_analysis.py:251: AssertionError

[orbeckst]

If we cannot create symlinks (under some conditions) on Windows then that would imply that the code check_and_fix_long_filename() is buggy under Windows. In this case the failure is actually correct and we should be thinking about fixing the code. Or am I misunderstanding why the test fails under Windows?

[IAlibay]

I'll tag @lilyminium here - I can't remember the specifics but I thought Lily mentioned that the issue is something that's down to the install/environment not the code itself, maybe we should fail more graciously though.

[lilyminium]

Ummm, it looks like the test already predicts the symlink permissions error. I think if you ran with admin permissions then you can symlink. https://www.scivision.dev/windows-symbolic-link-permission-enable/ also has something about running in "Developer mode", although that might (?) require 3.8+. It's hard to say without knowing more about Windows but IMO this should be treated as a bug, the file should be copied instead of symlinked, and just deleted after.

Annoyingly, I think this function might have been required in the first place because Windows had a (weirdly short, too) max path length.

install/environment not the code itself

From the looks of the xfail I thought this was a problem only for win32 and also expected virtually no one to use win32. I don't think tests would be run with admin permissions either, but you probably don't want to run Python like that anyway....