Fix errors with unit tests (#54)

* Change some tests with an imaginary HPC

The "44" value caused errors in another piece of software. "192" would fix
it for the replacement HPC, but I went with "1" to hopefully work longer
than 4 years.

* Change directive tests to use real PBS settings

-J is used for job arrays, and -A is the subproject number. Now it uses -l
for application tracking and -o for stdout filename.

* Fix narwhal ncpus

npcus were 44 which were for Onyx. Narwhal has 128.
Rename pbs_gpu to pbs_script everywhere.

* Replace onyx with carpenter

* Change uit.uit. and uit.uit.yaml imports for pytest to work

Also fix a str concat error with NoneType.

* Fix under-indentation from renaming pbs_gpu

tests/test_pbs_script.py

@@ -81,14 +81,14 @@ def test_job_array_indices(self):
 
     def test_set_directive(self):
         # Call the method
-        self.pbs.set_directive('-J', 'OepnGL')
+        self.pbs.set_directive('-l', 'application=other')
 
         # Get the list of namedtuple
         ret = self.pbs._optional_directives
 
         # Test the results
-        self.assertEqual('-J', ret[0].directive)
-        self.assertEqual('OepnGL', ret[0].options)
+        self.assertEqual('-l', ret[0].directive)
+        self.assertEqual('application=other', ret[0].options)
 
     def test_get_directive(self):
         self.pbs.set_directive('-A', 'C++')
@@ -117,15 +117,15 @@ def test_get_directive_default(self):
         self.assertEqual('test', res)
 
     def test_get_directives(self):
-        self.pbs.set_directive('-J', 'OepnGL')
-        self.pbs.set_directive('-A', 'C++')
+        self.pbs.set_directive('-l', 'application=other')
+        self.pbs.set_directive('-o', 'stdout.log')
 
         # Call the method
         res = self.pbs.optional_directives
 
         # Test the result
-        self.assertEqual('OepnGL', res[0].options)
-        self.assertEqual('C++', res[1].options)
+        self.assertEqual('application=other', res[0].options)
+        self.assertEqual('stdout.log', res[1].options)
 
     def test_module_use(self):
         self.pbs.module_use('path')
@@ -146,82 +146,77 @@ def test_render_required_directives_block_not_None(self, mock_job_array_directiv
         self.assertIn('this is fake', res)
 
     def test_get_render_required_directives_block_for_narwhal_compute_node(self):
-        res = self.pbs.render_required_directives_block()
-        self.assertIn('#PBS -N ' + self.pbs.name, res)
-        self.assertIn('#PBS -A ' + self.pbs.project_id, res)
-        self.assertIn("#PBS -q " + self.pbs.queue, res)
-        self.assertIn(f'#PBS -l select={self.pbs.num_nodes}:ncpus=44:mpiprocs={self.pbs.processes_per_node}', res)
+        pbs_script = PbsScript(name='test1', project_id='P001', num_nodes=5, processes_per_node=1, max_time="20:30:30",
+                               node_type='compute', system='narwhal')
+        res = pbs_script.render_required_directives_block()
+        self.assertIn('#PBS -N ' + pbs_script.name, res)
+        self.assertIn('#PBS -A ' + pbs_script.project_id, res)
+        self.assertIn("#PBS -q " + pbs_script.queue, res)
+        self.assertIn(f'#PBS -l select={pbs_script.num_nodes}:'
+                      f'ncpus={NODE_TYPES["narwhal"]["compute"]}:'
+                      f'mpiprocs={pbs_script.processes_per_node}', res)
 
     def test_get_render_required_directives_block_for_narwhal_gpu_node(self):
-        pbs_gpu = PbsScript(name='test1', project_id='P001', num_nodes=5, processes_per_node=1, max_time="20:30:30",
-                            node_type='gpu', system='narwhal')
-        res = pbs_gpu.render_required_directives_block()
-        self.assertIn(f'#PBS -l select={pbs_gpu.num_nodes}:ncpus={NODE_TYPES["narwhal"]["compute"]}:'
-                      f'mpiprocs={pbs_gpu.processes_per_node}', res)
+        pbs_script = PbsScript(name='test1', project_id='P001', num_nodes=5, processes_per_node=1, max_time="20:30:30",
+                               node_type='gpu', system='narwhal')
+        res = pbs_script.render_required_directives_block()
+        self.assertIn(f'#PBS -l select={pbs_script.num_nodes}:ncpus={NODE_TYPES["narwhal"]["compute"]}:'
+                      f'mpiprocs={pbs_script.processes_per_node}', res)
 
     def test_get_render_required_directives_block_for_narwhal_bigmem_node(self):
-        pbs_gpu = PbsScript(name='test1', project_id='P001', num_nodes=5, processes_per_node=1, max_time="20:30:30",
-                            node_type='bigmem', system='narwhal')
-        res = pbs_gpu.render_required_directives_block()
-        self.assertIn(f'#PBS -l select={pbs_gpu.num_nodes}:ncpus={NODE_TYPES["narwhal"]["bigmem"]}:'
-                      f'mpiprocs={pbs_gpu.processes_per_node}', res)
+        pbs_script = PbsScript(name='test1', project_id='P001', num_nodes=5, processes_per_node=1, max_time="20:30:30",
+                               node_type='bigmem', system='narwhal')
+        res = pbs_script.render_required_directives_block()
+        self.assertIn(f'#PBS -l select={pbs_script.num_nodes}:ncpus={NODE_TYPES["narwhal"]["bigmem"]}:'
+                      f'mpiprocs={pbs_script.processes_per_node}', res)
 
     def test_get_render_required_directives_block_for_narwhal_transfer_node(self):
-        pbs_gpu = PbsScript(name='test1', project_id='P001', num_nodes=5, processes_per_node=1, max_time="20:30:30",
-                            node_type='transfer', system='narwhal')
-        res = pbs_gpu.render_required_directives_block()
+        pbs_script = PbsScript(name='test1', project_id='P001', num_nodes=5, processes_per_node=1, max_time="20:30:30",
+                               node_type='transfer', system='narwhal')
+        res = pbs_script.render_required_directives_block()
         self.assertIn('#PBS -l select=5:ncpus=1', res)
 
-    def test_get_render_required_directives_block_for_onyx_compute_node(self):
-        pbs_gpu = PbsScript(name='test1', project_id='P001', num_nodes=5, processes_per_node=11, max_time="20:30:30",
-                            node_type='compute', system='onyx')
-        expected = f'#PBS -l select={pbs_gpu.num_nodes}:ncpus={NODE_TYPES["onyx"]["compute"]}:' \
-                   f'mpiprocs={pbs_gpu.processes_per_node}'
+    def test_get_render_required_directives_block_for_carpenter_compute_node(self):
+        pbs_script = PbsScript(name='test1', project_id='P001', num_nodes=5, processes_per_node=24, max_time="20:30:30",
+                               node_type='compute', system='carpenter')
+        expected = f'#PBS -l select={pbs_script.num_nodes}:ncpus={NODE_TYPES["carpenter"]["compute"]}:' \
+                   f'mpiprocs={pbs_script.processes_per_node}'
 
-        res = pbs_gpu.render_required_directives_block()
+        res = pbs_script.render_required_directives_block()
         self.assertIn(expected, res)
 
-    def test_get_render_required_directives_block_for_onyx_gpu_node(self):
-        pbs_gpu = PbsScript(name='test1', project_id='P001', num_nodes=5, processes_per_node=11, max_time="20:30:30",
-                            node_type='gpu', system='onyx')
-        expected = f'#PBS -l select={pbs_gpu.num_nodes}:ncpus={NODE_TYPES["onyx"]["gpu"]}:' \
-                   f'mpiprocs={pbs_gpu.processes_per_node}:ngpus=1'
+    def test_get_render_required_directives_block_for_carpenter_gpu_node(self):
+        pbs_script = PbsScript(name='test1', project_id='P001', num_nodes=5, processes_per_node=32, max_time="20:30:30",
+                               node_type='gpu', system='carpenter')
+        expected = f'#PBS -l select={pbs_script.num_nodes}:ncpus={NODE_TYPES["carpenter"]["gpu"]}:' \
+                   f'mpiprocs={pbs_script.processes_per_node}:ngpus=1'
 
-        res = pbs_gpu.render_required_directives_block()
+        res = pbs_script.render_required_directives_block()
         self.assertIn(expected, res)
 
-    def test_get_render_required_directives_block_for_onyx_bigmem_node(self):
-        pbs_gpu = PbsScript(name='test1', project_id='P001', num_nodes=5, processes_per_node=11, max_time="20:30:30",
-                            node_type='bigmem', system='onyx')
-        expected = f'#PBS -l select={pbs_gpu.num_nodes}:ncpus={NODE_TYPES["onyx"]["bigmem"]}:' \
-                   f'mpiprocs={pbs_gpu.processes_per_node}:bigmem=1'
+    def test_get_render_required_directives_block_for_carpenter_bigmem_node(self):
+        pbs_script = PbsScript(name='test1', project_id='P001', num_nodes=5, processes_per_node=24, max_time="20:30:30",
+                               node_type='bigmem', system='carpenter')
+        expected = f'#PBS -l select={pbs_script.num_nodes}:ncpus={NODE_TYPES["carpenter"]["bigmem"]}:' \
+                   f'mpiprocs={pbs_script.processes_per_node}:bigmem=1'
 
-        res = pbs_gpu.render_required_directives_block()
+        res = pbs_script.render_required_directives_block()
         self.assertIn(expected, res)
 
-    def test_get_render_required_directives_block_for_onyx_transfer_node(self):
-        pbs_gpu = PbsScript(name='test1', project_id='P001', num_nodes=5, processes_per_node=1, max_time="20:30:30",
-                            node_type='transfer', system='onyx')
-        res = pbs_gpu.render_required_directives_block()
+    def test_get_render_required_directives_block_for_carpenter_transfer_node(self):
+        pbs_script = PbsScript(name='test1', project_id='P001', num_nodes=5, processes_per_node=1, max_time="20:30:30",
+                               node_type='transfer', system='carpenter')
+        res = pbs_script.render_required_directives_block()
         self.assertIn('PBS -l select=5:ncpus=1', res)
 
-    def test_get_render_required_directives_block_for_onyx_knl_node(self):
-        pbs_gpu = PbsScript(name='test1', project_id='P001', num_nodes=5, processes_per_node=1, max_time="20:30:30",
-                            node_type='knl', system='onyx')
-        expected = f'#PBS -l select={pbs_gpu.num_nodes}:ncpus={NODE_TYPES["onyx"]["knl"]}:' \
-                   f'mpiprocs={pbs_gpu.processes_per_node}:nmics=1'
-
-        res = pbs_gpu.render_required_directives_block()
-        self.assertIn(expected, res)
-
     def test_get_render_optional_directives_block(self):
         self.pbs.set_directive('-A', 'ADH')
-        self.pbs.set_directive('-J', 'OpenMP')
+        self.pbs.set_directive('-o', 'stdout.log')
 
         res = self.pbs.render_optional_directives_block()
 
         self.assertIn('#PBS -A ADH', res)
-        self.assertIn('#PBS -J OpenMP', res)
+        self.assertIn('#PBS -o stdout.log', res)
 
     def test_load_module(self):
         # load anaconda module
@@ -280,7 +275,7 @@ def test_render(self):
 
         self.pbs.swap_module('Anaconda', 'OpenMP')
 
-        self.pbs.set_directive('-J', 'OpenMP')
+        self.pbs.set_directive('-o', 'stdout.log')
 
         self.pbs.set_directive('-T', 'OpenGL')
 
@@ -291,12 +286,12 @@ def test_render(self):
         self.assertIn("#PBS -A P001", render_str)
         self.assertIn("#PBS -N test1", render_str)
         self.assertIn("#PBS -q debug", render_str)
-        self.assertIn("#PBS -l select=5:ncpus=44:mpiprocs=1", render_str)
+        self.assertIn("#PBS -l select=5:ncpus=192:mpiprocs=1", render_str)
         self.assertIn("#PBS -l walltime=20:30:30", render_str)
         self.assertIn('module load C++', render_str)
         self.assertIn('module unload OpenGL', render_str)
         self.assertIn('module swap Anaconda OpenMP', render_str)
-        self.assertIn('#PBS -J OpenMP', render_str)
+        self.assertIn('#PBS -o stdout.log', render_str)
         self.assertIn('#PBS -T OpenGL', render_str)
 
     @mock.patch('uit.pbs_script.PbsScript.render')

tests/test_uit.py

@@ -1,14 +1,14 @@
 import unittest
 from unittest import mock
 from pathlib import PurePosixPath
-from uit.uit import Client
+from uit import Client
 
 
 class TestUIT(unittest.TestCase):
 
     def setUp(self):
-        from uit.uit import Client
-        with mock.patch('uit.uit.Client.get_userinfo') as _:
+        from uit import Client
+        with mock.patch('uit.Client.get_userinfo') as _:
             self.client = Client(token='test_token')
             self.client.connected = True
 
@@ -33,17 +33,17 @@ def mock_import(name, *args):
             builtins.__import__ = real_import
         self.assertEqual(uit_test.has_pandas, False)
 
-    @mock.patch('uit.uit.open')
-    @mock.patch('uit.uit.yaml.safe_load')
+    @mock.patch('uit.config.open')
+    @mock.patch('uit.config.yaml.safe_load')
     def test_init_no_token(self, mock_yaml, _):
         mock_yaml.return_value = {
             'client_id': 'client_id',
             'client_secret': 'client_secret'
         }
-        Client()
+        Client(config_file='test')
 
-    @mock.patch('uit.uit.open')
-    @mock.patch('uit.uit.yaml.safe_load')
+    @mock.patch('uit.config.open')
+    @mock.patch('uit.config.yaml.safe_load')
     def test_init_no_credentials(self, mock_yaml, _):
         mock_yaml.return_value = {}
         self.assertRaises(ValueError, Client)
@@ -54,25 +54,25 @@ def test_ensure_connected(self):
             RuntimeError, self.client.call, 'cmd'
         )
 
-    @mock.patch('uit.uit.Client')
+    @mock.patch('uit.Client')
     def test_HOME(self, _):
         self.client.env.HOME = 'HOME env'
         res = self.client.HOME
         self.assertEqual(PurePosixPath('HOME env'), res)
 
-    @mock.patch('uit.uit.Client')
+    @mock.patch('uit.Client')
     def test_WORKDIR(self, _):
         self.client.env.WORKDIR = 'WORKDIR'
         res = self.client.WORKDIR
         self.assertEqual(PurePosixPath('WORKDIR'), res)
 
-    @mock.patch('uit.uit.Client')
+    @mock.patch('uit.Client')
     def test_WORKDIR2(self, _):
         self.client.env.WORKDIR2 = 'WORKDIR2'
         res = self.client.WORKDIR2
         self.assertEqual(PurePosixPath('WORKDIR2'), res)
 
-    @mock.patch('uit.uit.Client')
+    @mock.patch('uit.Client')
     def test_CENTER(self, _):
         self.client.env.CENTER = 'CENTER'
         res = self.client.CENTER
@@ -108,25 +108,25 @@ def test_userinfo(self):
     def test_username(self):
         self.assertEqual(None, self.client.username)
 
-    @mock.patch('uit.uit.Client.call')
-    @mock.patch('uit.uit.Client.put_file')
+    @mock.patch('uit.Client.call')
+    @mock.patch('uit.Client.put_file')
     def test_submit(self, _, mock_call):
         mock_call.return_value = 'J001'
 
         ret = self.client.submit(pbs_script='test_script.sh', working_dir='\\test\\workdir')
 
         self.assertEqual('J001', ret)
 
-    @mock.patch('uit.uit.Client.call')
-    @mock.patch('uit.uit.Client.put_file')
+    @mock.patch('uit.Client.call')
+    @mock.patch('uit.Client.put_file')
     def test_submit_runtime_error(self, mock_put_file, mock_call):
         mock_put_file.return_value = {'success': 'false', 'error': 'test_error'}
         mock_call.return_value = 'J001'
 
         self.assertRaises(RuntimeError, self.client.submit, pbs_script='test_script.sh', working_dir='\\test\\workdir')
 
-    @mock.patch('uit.uit.Client.call')
-    @mock.patch('uit.uit.Client.put_file')
+    @mock.patch('uit.Client.call')
+    @mock.patch('uit.Client.put_file')
     def test_submit_call_error(self, _, mock_call):
         mock_call.side_effect = RuntimeError
 

uit/testing_utils.py

@@ -7,7 +7,7 @@
 class MockClient(Client):
     def __init__(self, *args, **kwargs):
         super().__init__(token='mock_token')
-        self.connect('onyx')
+        self.connect('testhpc')
 
     @property
     def WORKDIR(self):
@@ -23,12 +23,12 @@ def get_userinfo(self):
         self._userinfo = {
             'USERNAME': 'mock_user',
             'SYSTEMS': {
-                'ONYX': {
+                'TESTHPC': {
                     'LOGIN_NODES': [
                         {
-                            'HOSTNAME': 'onyx01',
+                            'HOSTNAME': 'testhpc01',
                             'USERNAME': 'mock_user',
-                            'URLS': {'UIT': 'onyx01@mock.gov'}
+                            'URLS': {'UIT': 'testhpc01@mock.gov'}
                         },
                     ]
                 }
@@ -106,7 +106,7 @@ def list_dir(self, path=None, parse=True, as_df=False):
     name='mock_script',
     project_id='mock_project_id',
     num_nodes=1,
-    processes_per_node=44,
+    processes_per_node=1,
     max_time='00:00:01'
 )
 
@@ -116,7 +116,7 @@ def list_dir(self, path=None, parse=True, as_df=False):
     name='mock_script',
     project_id='mock_project_id',
     num_nodes=1,
-    processes_per_node=44,
+    processes_per_node=1,
     max_time='00:00:01',
     array_indices=(0, 2)
 )

uit/uit.py

@@ -110,7 +110,7 @@ def __init__(self, ca_file=None, config_file=None, client_id=None, client_secret
 
         if (self.client_id is None or self.client_secret is None) and self.token is None:
             raise ValueError('Please provide either the client_id and client_secret as kwargs, environment vars '
-                             '(UIT_ID, UIT_SECRET) or in auth config file: ' + config_file + ' OR provide an '
+                             '(UIT_ID, UIT_SECRET) or in auth config file: ' + str(config_file) + ' OR provide an '
                              'access token as a kwarg.')
 
         if session_id is None: