-
Notifications
You must be signed in to change notification settings - Fork 51
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
Conversation
One or more tests FAILed. See https://jenkins1.ugent.be/job/vsc-base-pr-builder/336/console for more details. |
All tests PASSed. See https://jenkins1.ugent.be/job/vsc-base-pr-builder/337/console for more details. |
retest this please |
All tests PASSed. See https://jenkins1.ugent.be/job/vsc-base-pr-builder/338/console for more details. |
last tests ran with adapted jenkins to use |
|
||
|
||
PACKAGE = { | ||
'name': 'vsc-base', | ||
'version': '2.3.0', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no more version?
There was a problem hiding this comment.
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 ;)
There was a problem hiding this comment.
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
wdp = ('Wouter Depypere', '[email protected]') | ||
kw = ('Kenneth Waegeman', '[email protected]') | ||
eh = ('Ewan Higgs', '[email protected]') | ||
wp = ('Ward Poelmans', '[email protected]') |
There was a problem hiding this comment.
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?
All tests PASSed. See https://jenkins1.ugent.be/job/vsc-base-pr-builder/339/console for more details. |
@@ -2,6 +2,9 @@ | |||
import os | |||
import sys | |||
|
|||
if __name__ == '__main__': | |||
sys.path.insert(0, os.path.dirname(os.path.dirname(__file__))) |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 test
s below, the one in setup.py
is for import vsc.install
(since the latter insert .../lib
)
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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 😄
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
All tests PASSed. See https://jenkins1.ugent.be/job/vsc-base-pr-builder/340/console for more details. |
One or more tests FAILed. See https://jenkins1.ugent.be/job/vsc-base-pr-builder/341/console for more details. |
All tests PASSed. See https://jenkins1.ugent.be/job/vsc-base-pr-builder/342/console for more details. |
@boegel @JensTimmerman i added support to filter tests |
try: | ||
from unittest.suite import TestSuite | ||
except ImportError: | ||
from unittest import TestSuite |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, py2.6 ism
self.test_filterf = None | ||
self.test_loader = "vsc.install.shared_setup:VscScanningLoader" | ||
|
||
def run_tests(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing docstring
@stdweird: fully reviewed, mostly minor remarks and things that are not fully clear (to me) I still feel there's a lot of stuff being done here, which makes it feel like we're missing something, it's too complicated. Although, the filter support does add quite a bit the code in here, of course. |
One or more tests FAILed. See https://jenkins1.ugent.be/job/vsc-base-pr-builder/344/console for more details. |
retest this please |
All tests PASSed. See https://jenkins1.ugent.be/job/vsc-base-pr-builder/345/console for more details. |
from unittest.suite import TestSuite | ||
except ImportError: | ||
# py2.6 | ||
from unittest import TestSuite |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
All tests PASSed. See https://jenkins1.ugent.be/job/vsc-base-pr-builder/346/console for more details. |
All tests PASSed. See https://jenkins1.ugent.be/job/vsc-base-pr-builder/347/console for more details. |
from setuptools.command.install_scripts import install_scripts | ||
# egg_info uses sdist directly through manifest_maker | ||
from setuptools.command.sdist import sdist | ||
from setuptools.command.egg_info import egg_info |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sort these?
All tests PASSed. See https://jenkins1.ugent.be/job/vsc-base-pr-builder/348/console for more details. |
Going in, let's see what's broken. ;-) |
Support unittesting via 'python setup.py test'
|
||
try: | ||
import xmlrunner | ||
rs = xmlrunner.XMLTestRunner(output="test-reports").run(suite) |
There was a problem hiding this comment.
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
fixes #192