DM-35039: Add tests for CfhtIsrTask to obs_cfht #107
Conversation
tests/test_isr.py
Outdated
| testDataDirectory = lsst.utils.getPackageDir(testDataPackage) | ||
| except LookupError as e: | ||
| warnings.warn(e.args[0]) | ||
| raise unittest.SkipTest("Skipping tests as testdata_cfht is not setup") |
There was a problem hiding this comment.
Normally we'd ask for the test data package in the preamble and use a unittest.skipIf decorator around the entire test class. This is more efficient than having every test run the same code that will always fail.
tests/test_isr.py
Outdated
| detector = self.camera[0] | ||
|
|
||
| # Get the override. | ||
| self.configPath = os.path.join(lsst.utils.getPackageDir("obs_lsst"), "config", |
There was a problem hiding this comment.
I don't think we should be depending on obs_lsst in this test.
tests/test_isr.py
Outdated
| @@ -0,0 +1,140 @@ | |||
| # | |||
There was a problem hiding this comment.
I don't think the blank line in the beginning is required.
tests/test_isr.py
Outdated
| self.assertEqual(md['RDNOISEA'], amplifiers[0].getReadNoise()) | ||
| self.assertEqual(md['RDNOISEB'], amplifiers[1].getReadNoise()) | ||
|
|
||
| def imageHandler(self, location, detector=None): |
There was a problem hiding this comment.
This should perhaps be a staticmethod, since none of self's attributes are used. Also, this could go either before or after setUp(Class) method, in case we want to add additional tests later.
| class CfhtIsrTestCase(lsst.utils.tests.TestCase): | ||
| """Test for the CFHT IsrTask wrapper.""" | ||
|
|
||
| def setUp(self): |
There was a problem hiding this comment.
Since there's only one test case, it doesn't make any difference now, but if you foresee other test cases making use of the exposure and camera attributes, make it a setUpClass method so that it doesn't have to read in the images for every test case.
Add unit test coverage for CfhtIsrTask.