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

Support unittesting via 'python setup.py test' #193

Merged
merged 8 commits into from
Oct 9, 2015
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
169 changes: 153 additions & 16 deletions lib/vsc/install/shared_setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,19 +24,46 @@
import shutil
import sys
import re

try:
from unittest.suite import TestSuite
except ImportError:
from unittest import TestSuite
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the fallback is for older Python versions or something? add a comment to clarify please

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, py2.6 ism

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need for the exception for py2.7?

-bash-4.1$ python -c "import sys; print sys.version; from unittest import TestSuite"
2.6.6 (r266:84292, Jul 22 2015, 16:47:47) 
[GCC 4.4.7 20120313 (Red Hat 4.4.7-16)]
-bash-4.1$ module load Python/2.7.9-foss-2015a
-bash-4.1$ python -c "import sys; print sys.version; from unittest import TestSuite"
2.7.9 (default, Sep  1 2015, 15:59:22) 
[GCC 4.9.2]
-bash-4.1$ 
-bash-4.1$ python -c "import sys; print sys.version; from unittest.suite import TestSuite as T1; from unittest import TestSuite as T2; print T1 is T2"
2.7.9 (default, Sep  1 2015, 15:59:22) 
[GCC 4.9.2]
True

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, probably some init magic


from distutils import log # also for setuptools
from distutils.dir_util import remove_tree

from setuptools.command.test import test as TestCommand
from setuptools.command.test import ScanningLoader
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if setuptools is not available? It's not a part of the standard Python library...


# 0 : WARN (default), 1 : INFO, 2 : DEBUG
log.set_verbosity(2)

has_setuptools = None
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, setuptools is there since we hard import it above?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i cleaned up this setuptools legacy code a bit


# available authors
ag = ('Andy Georges', '[email protected]')
eh = ('Ewan Higgs', '[email protected]')
jt = ('Jens Timmermans', '[email protected]')
kh = ('Kenneth Hoste', '[email protected]')
kw = ('Kenneth Waegeman', '[email protected]')
lm = ('Luis Fernando Munoz Meji?as', '[email protected]')
sdw = ('Stijn De Weirdt', '[email protected]')
wdp = ('Wouter Depypere', '[email protected]')
wp = ('Ward Poelmans', '[email protected]')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe keep them ordered alphabetically?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, are you using a different alphabet than the rest of us? ;)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are you? (and are you suggestion emacs' sort-fields is wrong 😄 )

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some alphabet, different sorting key (variable name vs actual name of person), so nvm :)


# FIXME: do we need this here? it won;t hurt, but still ...
REGEXP_REMOVE_SUFFIX = re.compile(r'(\.(?:py|sh|pl))$')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do I dare ask about this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i added a proper comment


# We do need all setup files to be included in the source dir if we ever want to install
# the package elsewhere.
EXTRA_SDIST_FILES = ['setup.py']

# Put unittests under this directory
DEFAULT_TEST_SUITE = 'test'

URL_GH_HPCUGENT = 'https://github.com/hpcugent/%(name)s'
URL_GHUGENT_HPCUGENT = 'https://github.ugent.be/hpcugent/%(name)s'

def find_extra_sdist_files():
"""Looks for files to append to the FileList that is used by the egg_info."""
Expand Down Expand Up @@ -122,20 +149,6 @@ class vsc_bdist_rpm_egg_info(vsc_egg_info):
has_setuptools = False


# available authors
ag = ('Andy Georges', '[email protected]')
jt = ('Jens Timmermans', '[email protected]')
kh = ('Kenneth Hoste', '[email protected]')
lm = ('Luis Fernando Munoz Meji?as', '[email protected]')
sdw = ('Stijn De Weirdt', '[email protected]')
wdp = ('Wouter Depypere', '[email protected]')
kw = ('Kenneth Waegeman', '[email protected]')
eh = ('Ewan Higgs', '[email protected]')


# FIXME: do we need this here? it won;t hurt, but still ...
REGEXP_REMOVE_SUFFIX = re.compile(r'(\.(?:py|sh|pl))$')

class vsc_install_scripts(install_scripts):
"""Create the (fake) links for mympirun also remove .sh and .py extensions from the scripts."""

Expand Down Expand Up @@ -166,14 +179,131 @@ def find_package_modules (self, package, package_dir):


class vsc_bdist_rpm(bdist_rpm):
""" Custom class to build the RPM, since the __inti__.py cannot be included for the packages that have namespace spread across all of the machine."""
"""Custom class to build the RPM, since the __init__.py cannot be included for the packages that have namespace spread across all of the machine."""
def run(self):
log.error("vsc_bdist_rpm = %s" % (self.__dict__))
SHARED_TARGET['cmdclass']['egg_info'] = vsc_bdist_rpm_egg_info # changed to allow removal of files
self.run_command('egg_info') # ensure distro name is up-to-date
orig_bdist_rpm.run(self)


# private class variables to communicate
# between VscScanningLoader and VscTestCommand
# stored in __builtin__ because the .run_tests
# reload and cleanup in the modules
import __builtin__
if not hasattr(__builtin__,'__test_filter'):
setattr(__builtin__, '__test_filter', {
'module': None,
'function': None,
'allowmods': [],
})


def filter_testsuites(testsuites):
"""(Recursive) filtering of (suites of) tests"""
test_filter = getattr(__builtin__,'__test_filter')['function']
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

space after ,


res = type(testsuites)()

for t in testsuites:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't use single-letter variables outside of list comprehensions please

# t is either a test or testsuite of more tests
if isinstance(t, TestSuite):
res.addTest(filter_testsuites(t))
else:
if re.search(test_filter, t._testMethodName):
res.addTest(t)
return res


class VscScanningLoader(ScanningLoader):
def loadTestsFromModule(self, module):
"""Allow for filter"""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needs a better docstring, should provide a better description of what loadTestsFromModule does/returns?

testsuites = ScanningLoader.loadTestsFromModule(self, module)
test_filter = getattr(__builtin__,'__test_filter')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

space after ,


res = testsuites

if test_filter['module'] is not None:
mod = module.__name__
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

avoid confusion between mod and module, use modname instead of mod?

if mod in test_filter['allowmods']:
# a parent name space
pass
elif re.search(test_filter['module'], mod):
if test_filter['function'] is not None:
res = filter_testsuites(testsuites)
# add parents (and module itself)
pms = mod.split('.')
for pm_idx in range(len(pms)):
pm = '.'.join(pms[:pm_idx])
if not pm in test_filter['allowmods']:
test_filter['allowmods'].append(pm)
else:
res = type(testsuites)()

return res


class VscTestCommand(TestCommand):
"""
The cmdclass for testing
"""
user_options = TestCommand.user_options + [
('test-filterf=', 'f', "Regex filter on test function names"),
('test-filterm=', 'F', "Regex filter on test (sub)modules"),
]

def initialize_options(self):
TestCommand.initialize_options(self)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing docstring

self.test_filterm = None
self.test_filterf = None
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what magic takes care of populating these if -f or -F are used?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setuptools

self.test_loader = "vsc.install.shared_setup:VscScanningLoader"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

magic string?

combine VscScanningLoader.__name__ with whatever you need to use to determine the module name you're in?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok


def run_tests(self):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing docstring

getattr(__builtin__,'__test_filter').update({
'function': self.test_filterf,
'module': self.test_filterm,
})

# should be setup.py
setup_py = os.path.abspath(sys.argv[0])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can verify whether it is, by checking os.path.basename(setup_py)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, what if someone uses it from a setup_whatever.py to test branches?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On 09/10/15 09:48, Stijn De Weirdt wrote:

In lib/vsc/install/shared_setup.py
#193 (comment):

  • ]
  • def initialize_options(self):
  •    TestCommand.initialize_options(self)
    
  •    self.test_filterm = None
    
  •    self.test_filterf = None
    
  •    self.test_loader = "vsc.install.shared_setup:VscScanningLoader"
    
  • def run_tests(self):
  •    getattr(**builtin**,'__test_filter').update({
    
  •        'function': self.test_filterf,
    
  •        'module': self.test_filterm,
    
  •    })
    
  •    # should be setup.py
    
  •    setup_py = os.path.abspath(sys.argv[0])
    

hmm, what if someone uses it from a |setup_whatever.py| to test branches?

Then the check will fail. You can make it a bit more flexible if you
need to:

os.path.basename(setup_py).startswith('setup')
os.path.basename(setup_py).endswith('.py')

log.info('run_tests from %s' % setup_py)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we really need inspect here? it is kind of slow, and does not work if the .py file is not available (like when you only have the .pyc or pyo files)
why not use sys.argv[0]? Seems to do the same thing to me in these cases:

[jens@exile temp]$ cat foo.py 
import bla
[jens@exile temp]$ cat bla.py
import inspect
import sys
import os
print inspect.stack()[-1][1]
print sys.argv[0] 
print os.path.dirname(sys.argv[0])
print os.path.dirname(inspect.stack()[-1][1])
[jens@exile temp]$ python foo.py 
foo.py
foo.py


[jens@exile temp]$ python bla.py
bla.py
bla.py


[jens@exile temp]$ mv foo.py test/
[jens@exile temp]$ vim bla.py
[jens@exile temp]$ mv bla.py test/
[jens@exile temp]$ python test/bla.py 
test/bla.py
test/bla.py
test
test
[jens@exile temp]$ python test/foo.py 
test/foo.py
test/foo.py
test
test

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i pulled the inspect code form stackoverflow, rereading it, they tried to solve something more complicated, so i'll revert to sys.argv[0]

base_dir = os.path.dirname(setup_py)

# make a lib dir to trick setup.py to package this properly
# and git ignore empty dirs, so recreate it if necessary
lib_dir = os.path.join(base_dir, 'lib')
if not os.path.exists(lib_dir):
os.mkdir(lib_dir)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no cleanup?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no really, this is simply an empty directory that is missing because git doesn't handle empty dirs.
but i'll add the cleanup for good practise.


test_dir = os.path.join(base_dir, DEFAULT_TEST_SUITE)
if os.path.isdir(test_dir):
sys.path.insert(0, test_dir)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wait, shouldn't this add base_dir rather than test_dir to sys.path?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, should add testdir for modules in test dir that are no unittests but are used in unittests

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

else:
raise Exception("Can't find location of testsuite directory %s in %s" % (DEFAULT_TEST_SUITE, base_dir))

# make sure we can import the script as a module
scripts_dir = os.path.join(base_dir, 'bin')
if os.path.isdir(scripts_dir):
sys.path.insert(0, scripts_dir)

# and now the ugly bits: cleanup and restore vsc namespace
# we use vsc namespace tools very early
# need to make sure they are picked up from the paths as specified above
# not to mix with installed and already loaded modules
loaded_vsc_modules = [mod for mod in sys.modules.keys() if mod == 'vsc' or mod.startswith('vsc.')]
reload_vsc_modules = []
for mod in loaded_vsc_modules:
if hasattr(sys.modules[mod], '__file__'):
# only actual modules
reload_vsc_modules.append(mod)
del(sys.modules[mod])
# reimport
for mod in reload_vsc_modules:
__import__(mod)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how do you know whether you're importing the right stuff here?

because of the sys.path hack at the top?

maybe check here whether things are being imported where you expect from (use base_dir)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the problem is i don't know which one of the vsc modules that should be loaded from the repo

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, right


TestCommand.run_tests(self)

# shared target config
SHARED_TARGET = {
'url': '',
Expand All @@ -184,6 +314,8 @@ def run(self):
"egg_info": vsc_egg_info,
"bdist_rpm": vsc_bdist_rpm,
},
'cmdclass': {'test': VscTestCommand},
'test_suite': DEFAULT_TEST_SUITE,
}


Expand Down Expand Up @@ -265,7 +397,7 @@ def build_setup_cfg_for_bdist_rpm(target):
setup_cfg.close()


def action_target(target, setupfn=setup, extra_sdist=[]):
def action_target(target, setupfn=setup, extra_sdist=[], urltemplate=None):
# EXTRA_SDIST_FILES.extend(extra_sdist)

do_cleanup = True
Expand All @@ -283,6 +415,11 @@ def action_target(target, setupfn=setup, extra_sdist=[]):
if do_cleanup:
cleanup()

if urltemplate:
target['url'] = urltemplate % target
if 'github' in urltemplate:
target['download_url'] = "%s/tarball/master" % target['url']
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

flesh out master into a branch named argument?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


build_setup_cfg_for_bdist_rpm(target)
x = parse_target(target)

Expand Down
19 changes: 10 additions & 9 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,35 +32,36 @@
@author: Andy Georges (Ghent University)
@author: Kenneth Hoste (Ghent University)
"""

# vsc-base setup.py needs vsc.install, which is currently shipped as part of vsc-base
# vsc.install doesn't require vsc-base, so we could move it to it's own repo and only
# have this hack in the setup.py of vsc.install (and set it as build_requires)
# until then...
import os
import sys

sys.path.insert(0, os.path.join(os.path.dirname(os.path.abspath(__file__)), "lib"))


import vsc.install.shared_setup as shared_setup
from vsc.install.shared_setup import ag, kh, jt, sdw
from vsc.install.shared_setup import ag, kh, jt, sdw, URL_GH_HPCUGENT

def remove_bdist_rpm_source_file():
"""List of files to remove from the (source) RPM."""
return []

shared_setup.remove_extra_bdist_rpm_files = remove_bdist_rpm_source_file
shared_setup.SHARED_TARGET.update({
'url': 'https://github.com/hpcugent/vsc-base',
'download_url': 'https://github.com/hpcugent/vsc-base',
'zip_safe': True,
})


PACKAGE = {
'name': 'vsc-base',
'version': '2.3.0',
'version': '2.3.1',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no more version?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wdpypere: psst, there's a line below this one ;)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice catch, restored and bumped

'author': [sdw, jt, ag, kh],
'maintainer': [sdw, jt, ag, kh],
'packages': ['vsc', 'vsc.install', 'vsc.utils'],
'scripts': ['bin/logdaemon.py', 'bin/startlogdaemon.sh', 'bin/bdist_rpm.sh', 'bin/optcomplete.bash'],
'install_requires' : ['setuptools'],
'zip_safe': True,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we sure about this? We have vsc-utils that also install in the same namespace, or am I mixing things up?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i didn't add this...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the diff you did :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

look a few lines back

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ageorges: shouldn't matter whether vsc-utils touches vsc/utils too, vsc-base by itself works just fine when installed as a zipped egg

However, other stuff that requires it as a dep doesn't support using it when it's installed as a zipped egg yet cough EasyBuild cough

So, for now, Jenkins shouldn't install it as a zipped egg after a successful merge/test, i.e. it should use easy_install -U

}

if __name__ == '__main__':
shared_setup.action_target(PACKAGE)
shared_setup.action_target(PACKAGE, urltemplate=URL_GH_HPCUGENT)
18 changes: 11 additions & 7 deletions test/runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
import os
import sys

if __name__ == '__main__':
sys.path.insert(0, os.path.dirname(os.path.dirname(__file__)))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was kicked out in a previous PR, why add it again?

if it's really needed (it shouldn't), please add a clear comment here

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it will break again if we remove the hack from setup.py

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This hack is not (really) related to the one in setup.py, I think...

This one is for the import tests below, the one in setup.py is for import vsc.install (since the latter insert .../lib)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is for people who run the tests with python test/runner.py, as is clear from the if ... above

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you avoid this hack by using relative imports? import .docs ad tdo, etc?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how would relative imports pickup something differently then what sys.path sets as order?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't relative imports imply only looking in the dir where the current module is located?

not really sure here, trying to find alternatives, the hack just feels wrong (but I guess it's not a blocker)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the whole runner.py can be removed 😄

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, even better, but then we should bump to 2.4?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok


import test.asyncprocess as a
import test.dateandtime as td
import test.docs as tdo
Expand All @@ -22,11 +25,12 @@

suite = unittest.TestSuite([x.suite() for x in (a, td, tg, tf, te, tm, trest, trun, tt, topt, wrapt, tdo)])

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stdweird: Jenkins relies on having this available

try:
import xmlrunner
rs = xmlrunner.XMLTestRunner(output="test-reports").run(suite)
except ImportError, err:
rs = unittest.TextTestRunner().run(suite)
if __name__ == '__main__':
try:
import xmlrunner
rs = xmlrunner.XMLTestRunner(output="test-reports").run(suite)
except ImportError, err:
rs = unittest.TextTestRunner().run(suite)

if not rs.wasSuccessful():
sys.exit(1)
if not rs.wasSuccessful():
sys.exit(1)