Details
-
Type:
Bug
-
Status: Done
-
Resolution: Done
-
Fix Version/s: None
-
Component/s: obs_cfht
-
Labels:None
-
Team:SQuaRE
Description
All of the test* functions in tests/testButler.py were disabled if the datadir wasn't found except for testPackageName. However, that test uses the butler as well, so if there's no datadir then that test must be skipped as well. Fix is to do:
def testPackageName(self):
|
if not self.runTests:
|
return
|
self.assertEqual(self.butler.mapper.packageName, "obs_cfht")
|
Attachments
Activity
Michael Wood-Vasey I noticed that too when working on DM-4711, and adopted some changes as how obs_decam skips tests when testdata_decam is not found. They are currently in branch u/hfc/DM-4711 of obc_cfht.
Hsin-Fang Chiang
added a comment - Michael Wood-Vasey I noticed that too when working on DM-4711 , and adopted some changes as how obs_decam skips tests when testdata_decam is not found. They are currently in branch u/hfc/ DM-4711 of obc_cfht . Thanks, Tim Jenness Good point!
Thanks, Hsin-Fang Chiang . I cherry-picked your changes from u/hfc/DM-4711. They look good to me
Here's the result:
[Calm obs_cfht] python tests/testButler.py
|
tests/testButler.py:88: UserWarning: Package testdata_cfht not found
|
warnings.warn(e.message)
|
sssss.
|
----------------------------------------------------------------------
|
Ran 6 tests in 0.015s
|
|
|
OK (skipped=5)
|
[Calm obs_cfht] scons opt=3
|
scons: Reading SConscript files ...
|
EUPS integration: enabled
|
Checking who built the CC compiler...(cached) error: no result
|
CC is clang version 7.0.2
|
Checking for C++11 support
|
Checking whether the C++ compiler works... (cached) yes
|
C++11 supported with '-std=c++11'
|
Setting up environment to build package 'obs_cfht'.
|
scons: done reading SConscript files.
|
scons: Building targets ...
|
scons: Nothing to be done for `python'.
|
running tests/testButler.py... passed
|
scons: done building targets.
|
Michael Wood-Vasey
added a comment - Thanks, Tim Jenness Good point!
Thanks, Hsin-Fang Chiang . I cherry-picked your changes from u/hfc/ DM-4711 . They look good to me
Here's the result:
[Calm obs_cfht] python tests/testButler.py
tests/testButler.py:88: UserWarning: Package testdata_cfht not found
warnings.warn(e.message)
sssss.
----------------------------------------------------------------------
Ran 6 tests in 0.015s
OK (skipped=5)
[Calm obs_cfht] scons opt=3
scons: Reading SConscript files ...
EUPS integration: enabled
Checking who built the CC compiler...(cached) error: no result
CC is clang version 7.0.2
Checking for C++11 support
Checking whether the C++ compiler works... (cached) yes
C++11 supported with '-std=c++11'
Setting up environment to build package 'obs_cfht'.
scons: done reading SConscript files.
scons: Building targets ...
scons: Nothing to be done for `python'.
running tests/testButler.py... passed
scons: done building targets.
quick review
Do you agree with the way `unittest.SkipTest` is invoked as the right way to handle this?
Michael Wood-Vasey
added a comment - quick review
Do you agree with the way `unittest.SkipTest` is invoked as the right way to handle this? These changes look good to me.
Merged into master.
Michael Wood-Vasey
added a comment - Merged into master. 
Tests should not be disabled, they should be skipped. Returning without action indicates that the test has passed. What we want is the test harness to realise that a load of tests have been skipped (with the reason). This will be very helpful if we aren't expecting tests to be skipped.