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

execute permission on python files in test? #119

Closed
repagh opened this issue May 11, 2020 · 1 comment · Fixed by #125
Closed

execute permission on python files in test? #119

repagh opened this issue May 11, 2020 · 1 comment · Fixed by #125

Comments

@repagh
Copy link
Member

repagh commented May 11, 2020

In the CMakeLists.txt for the tests I see:

install(FILES ${PYSOURCE}
        PERMISSIONS OWNER_READ OWNER_WRITE OWNER_EXECUTE
                    GROUP_READ GROUP_EXECUTE
                    WORLD_READ WORLD_EXECUTE
        DESTINATION slycot/tests)

Why are these files installed with execute permission? On Linux/OSX this is not needed, is it a Windows thing, or something specific to pytest?

@bnavigator
Copy link
Collaborator

bnavigator commented May 11, 2020

Many test files have a __main__ section so that one could run them directly from the command line.

~/src/Slycot/slycot/ > ./tests/test_mb.py 
......
----------------------------------------------------------------------
Ran 6 tests in 0.006s

OK

This needs to be consistent with a proper shebang in the first line and the executable bits. That is not always true right now.

Also note that when packaging a RPM and installing executable python scripts into ${python_sitearch} (e.g. /usr/lib64/python3.8/site-packages/slycot/tests/), that produces RPMlint warnings and is not considered good practice.

On my system, when running python3 setup.py build, the executable bits do not propagate from _skbuild/*/cmake-install to _skbuild/*/setuptools anyway:

~/src/Slycot/ > ls -l _skbuild/linux-x86_64-3.8/cmake-install/slycot/tests 
total 88
-rwxr-xr-x 1 greiner users     0 May  6 03:01 __init__.py
drwxr-xr-x 1 greiner users  1046 May 11 17:37 __pycache__
-rwxr-xr-x 1 greiner users  3062 May  6 03:03 test_ab08n.py
-rwxr-xr-x 1 greiner users  4091 May  6 03:03 test_ag08bd.py
-rwxr-xr-x 1 greiner users   631 May 11 14:03 test_examples.py
-rwxr-xr-x 1 greiner users  3200 May 11 14:03 test_exceptions.py
-rwxr-xr-x 1 greiner users 12932 May 11 20:28 test_mb.py
-rwxr-xr-x 1 greiner users  1912 May 11 14:03 test_mc.py
-rwxr-xr-x 1 greiner users  8712 May 11 17:20 test_sb.py
-rwxr-xr-x 1 greiner users  1531 May  6 03:03 test_sg02ad.py
-rwxr-xr-x 1 greiner users  2613 May  6 03:03 test_sg03ad.py
-rwxr-xr-x 1 greiner users  6821 May 11 14:03 test_tb05ad.py
-rwxr-xr-x 1 greiner users  8729 May  6 03:03 test_td04ad.py
-rwxr-xr-x 1 greiner users  2573 May  6 03:03 test_tg01ad.py
-rwxr-xr-x 1 greiner users  4403 May  6 03:03 test_tg01fd.py
~/src/Slycot/ > ls -l _skbuild/linux-x86_64-3.8/setuptools/lib/slycot/tests
total 88
-rw-r--r-- 1 greiner users     0 May  6 03:01 __init__.py
-rw-r--r-- 1 greiner users  3062 May  6 03:03 test_ab08n.py
-rw-r--r-- 1 greiner users  4091 May  6 03:03 test_ag08bd.py
-rw-r--r-- 1 greiner users   631 May 11 14:03 test_examples.py
-rw-r--r-- 1 greiner users  3200 May 11 14:03 test_exceptions.py
-rw-r--r-- 1 greiner users 12932 May 11 20:28 test_mb.py
-rw-r--r-- 1 greiner users  1912 May 11 14:03 test_mc.py
-rw-r--r-- 1 greiner users  8712 May 11 17:20 test_sb.py
-rw-r--r-- 1 greiner users  1531 May  6 03:03 test_sg02ad.py
-rw-r--r-- 1 greiner users  2613 May  6 03:03 test_sg03ad.py
-rw-r--r-- 1 greiner users  6821 May 11 14:03 test_tb05ad.py
-rw-r--r-- 1 greiner users  8729 May  6 03:03 test_td04ad.py
-rw-r--r-- 1 greiner users  2573 May  6 03:03 test_tg01ad.py
-rw-r--r-- 1 greiner users  4403 May  6 03:03 test_tg01fd.py

My recommendation from a linux package maintainer point of view is to remove the shebangs and executable bits of the test files. Executing them individually is still possible by calling python <testfile> or pytest -k <testspec> (the latter bypassing the main section)

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.

2 participants