Skip to content

Commit

Permalink
Fix several warnings reported by test suite
Browse files Browse the repository at this point in the history
  • Loading branch information
benmwebb committed Apr 28, 2020
1 parent bed9ca2 commit d5a462d
Show file tree
Hide file tree
Showing 10 changed files with 56 additions and 33 deletions.
39 changes: 25 additions & 14 deletions python/saliweb/backend/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -225,8 +225,8 @@ def __init__(self, fh):
config = configparser.SafeConfigParser()
if not hasattr(fh, 'read'):
self._config_dir = os.path.dirname(os.path.abspath(fh))
config.readfp(open(fh), fh)
fh = open(fh)
with open(fh) as fp:
config.readfp(fp, fh)
else:
self._config_dir = None
config.readfp(fh)
Expand Down Expand Up @@ -306,7 +306,8 @@ def _populate_limits(self, config):
def _read_db_auth(self, end='back'):
filename = self.database[end + 'end_config']
config = configparser.SafeConfigParser()
config.readfp(open(filename), filename)
with open(filename) as fh:
config.readfp(fh, filename)
for key in ('user', 'passwd'):
self.database[key] = config.get(end + 'end_db', key)

Expand Down Expand Up @@ -688,7 +689,8 @@ def get_running_pid(self):
raise a :exc:`StateFileError`."""
state_file = self.config.backend['state_file']
try:
old_state = open(state_file).read().rstrip('\r\n')
with open(state_file) as fh:
old_state = fh.read().rstrip('\r\n')
except IOError:
return
if old_state.startswith('FAILED: '):
Expand Down Expand Up @@ -864,6 +866,7 @@ def _register(self, up):
except socket.error:
# Swallow exception
pass
s.close()

def _sanity_check(self):
"""Do basic sanity checking of the web service"""
Expand Down Expand Up @@ -1201,8 +1204,8 @@ def _job_state_file_done(self):
"""Return True only if the job-state file indicates the job
finished."""
try:
f = open(self._get_job_state_file())
return f.read().rstrip('\r\n') == 'DONE'
with open(self._get_job_state_file()) as f:
return f.read().rstrip('\r\n') == 'DONE'
except IOError:
return False # if the file does not exist, job is still running

Expand Down Expand Up @@ -1649,8 +1652,8 @@ def set_sge_name(self, name):
If the name is not a valid SGE name (e.g. names cannot start with
a digit) then it is mapped to one that is.
"""
name = re.sub('\s*', '', name)
if re.match('\d', name) or name.upper() in ("NONE", "ALL", "TEMPLATE"):
name = re.sub(r'\s*', '', name)
if re.match(r'\d', name) or name.upper() in ("NONE", "ALL", "TEMPLATE"):
name = 'J' + name
self._name = name

Expand Down Expand Up @@ -1737,13 +1740,14 @@ def _check_bulk_completed(cls, jobid):
# Unfortunately DRMAA1 only allows us to query individual tasks, and
# looping over all tasks in a large parallel job is very inefficient,
# so use qstat instead and parse the output.
m = re.match('(\S+)\.(\d+)\-(\d+):(\d+)$', jobid)
m = re.match(r'(\S+)\.(\d+)\-(\d+):(\d+)$', jobid)
jobid = m.group(1)
p = subprocess.Popen([cls._qstat, '-j', jobid], stdout=subprocess.PIPE,
stderr=subprocess.STDOUT, env=cls._env,
universal_newlines=True)
out = p.stdout.readlines()
ret = p.wait()
p.stdout.close()
if ret == 0:
return False
elif len(out) > 0 and out[0].startswith('Following jobs do not exist'):
Expand Down Expand Up @@ -1887,9 +1891,13 @@ def download(self, fh=None):
is written into the job directory with the same name."""
fin = urllib2.urlopen(self.url)
if fh is None:
fh = open(self.get_filename(), 'w')
outfh = open(self.get_filename(), 'w')
else:
outfh = fh
for line in fin:
fh.write(line)
outfh.write(line)
if fh is None:
outfh.close()


class SaliWebServiceRunner(Runner):
Expand Down Expand Up @@ -1917,7 +1925,8 @@ def __init__(self, url, args):

def _run(self, webservice):
"""Run the command and return a unique job ID."""
open(os.path.join(self._directory, 'job-state'), 'w').write('STARTED')
with open(os.path.join(self._directory, 'job-state'), 'w') as fh:
fh.write('STARTED')
cwd = os.getcwd()
try:
os.chdir(self._directory)
Expand All @@ -1931,7 +1940,8 @@ def _run(self, webservice):
def _get_results(cls, jobid, directory):
results = saliweb.web_service.get_results(jobid)
if results is not None:
open(os.path.join(directory, 'job-state'), 'w').write('DONE')
with open(os.path.join(directory, 'job-state'), 'w') as fh:
fh.write('DONE')
return results

@classmethod
Expand Down Expand Up @@ -1962,7 +1972,8 @@ def __init__(self):
def _run(self, webservice):
"""Run and complete immediately"""
# Make job-state file
open(os.path.join(self._directory, 'job-state'), 'w').write('DONE\n')
with open(os.path.join(self._directory, 'job-state'), 'w') as fh:
fh.write('DONE\n')
runid = 'none'
e = saliweb.backend.events._CompletedJobEvent(webservice,
self, runid, None)
Expand Down
2 changes: 1 addition & 1 deletion python/saliweb/backend/sge.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class _SGETasks(object):

def __init__(self, opts):
if '-t ' in opts:
m = re.search('-t\s+(\d+)(?:\-(\d+)(?::(\d+))?)?', opts)
m = re.search(r'-t\s+(\d+)(?:\-(\d+)(?::(\d+))?)?', opts)
if not m:
raise ValueError("Invalid -t SGE option: '%s'" % opts)
self.first = int(m.group(1))
Expand Down
6 changes: 3 additions & 3 deletions python/saliweb/build/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ def _check_directory_permissions(env):
** Please change the ownership of this directory.
""" % (dir, backend_user), file=sys.stderr)
env.Exit(1)
if not re.search('^group::.\-..*other::.\-.', out,
if not re.search(r'^group::.\-..*other::.\-.', out,
re.MULTILINE | re.DOTALL):
print("""
** Install directory %s appears to be group- or world-writable!
Expand Down Expand Up @@ -480,7 +480,7 @@ def _check_permissions(env):
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
universal_newlines=True).communicate()
if not re.search('^group::\-\-\-.*^other::\-\-\-', out,
if not re.search(r'^group::\-\-\-.*^other::\-\-\-', out,
re.MULTILINE | re.DOTALL):
print("""
** The database configuration file %s appears to be group- or world-
Expand Down Expand Up @@ -615,7 +615,7 @@ def _check_sql_username_length(env, auth, typ):
def _get_sorted_grant(grant):
"""Sort grant column rights alphabetically, so that we can match them
reliably."""
m = re.match('(.*?\()(.*)(\).*)$', grant)
m = re.match(r'(.*?\()(.*)(\).*)$', grant)
if m:
fields = m.group(2).split(',')
fields = [x.strip() for x in fields]
Expand Down
3 changes: 2 additions & 1 deletion test/backend/run-all-tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ def __init__(self, *args, **keys):
def make_site_customize(self):
"""Get coverage information on Python subprocesses"""
self.tmpdir = tempfile.mkdtemp()
open(os.path.join(self.tmpdir, 'sitecustomize.py'), 'w').write("""
with open(os.path.join(self.tmpdir, 'sitecustomize.py'), 'w') as fh:
fh.write("""
import coverage
import atexit
_cov = coverage.coverage(branch=True, data_suffix=True, auto_data=True,
Expand Down
8 changes: 5 additions & 3 deletions test/backend/test_job.py
Original file line number Diff line number Diff line change
Expand Up @@ -438,7 +438,8 @@ def test_preprocess_logging(self):
os.unlink(os.path.join(jobdir, 'preproc'))
os.unlink(os.path.join(jobdir, 'job-output'))
# All logging messages above the threshold should be in framework.log
logs = open(os.path.join(jobdir, 'framework.log')).read()
with open(os.path.join(jobdir, 'framework.log')) as fh:
logs = fh.read()
self.assertTrue(re.match(
'\d+\-\d+\-\d+ \d+:\d+:\d+,\d+ WARNING warning message\n'
'\d+\-\d+\-\d+ \d+:\d+:\d+,\d+ ERROR error message\n'
Expand Down Expand Up @@ -780,8 +781,9 @@ def test_delete_all(self):
injobdir = add_incoming_job(db, 'job2')
add_expired_job(db, 'job3')
runjobdir = add_running_job(db, 'job4', completed=False)
open(os.path.join(db.config.directories['PREPROCESSING'],
'tempfile'), 'w').write('foo')
with open(os.path.join(db.config.directories['PREPROCESSING'],
'tempfile'), 'w') as fh:
fh.write('foo')
web._delete_all_jobs()
self.assertIsNone(web.get_job_by_name('FAILED', 'job1'))
self.assertIsNone(web.get_job_by_name('INCOMING', 'job2'))
Expand Down
6 changes: 3 additions & 3 deletions test/backend/test_locked_job_dict.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@ def test_locked_job_dict(self):
"""Check the _LockedJobDict class"""
d = saliweb.backend._LockedJobDict()
self.assertRaises(KeyError, d.remove, 'bar')
self.assertEquals('foo' in d, False)
self.assertEqual('foo' in d, False)
d.add('foo')
self.assertEquals('foo' in d, True)
self.assertEqual('foo' in d, True)
d.remove('foo')
self.assertEquals('foo' in d, False)
self.assertEqual('foo' in d, False)
self.assertRaises(KeyError, d.remove, 'foo')

if __name__ == '__main__':
Expand Down
3 changes: 2 additions & 1 deletion test/backend/test_logging.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ def test_init(self):
print("test text", file=dfs)
dfs.flush()
self.assertTrue(os.path.exists('foo'))
contents = open('foo').read()
with open('foo') as fh:
contents = fh.read()
self.assertEqual(contents, 'test text\n')

if __name__ == '__main__':
Expand Down
3 changes: 2 additions & 1 deletion test/backend/test_web_service_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ def test_run(self):
time.sleep(0.05)
res = SaliWebServiceRunner._check_completed(url, d.tmpdir)
self.assertEqual(len(res), 2)
state = open(os.path.join(d.tmpdir, 'job-state')).read()
with open(os.path.join(d.tmpdir, 'job-state')) as fh:
state = fh.read()
self.assertEqual(state, 'DONE')
finally:
saliweb.web_service = old
Expand Down
16 changes: 11 additions & 5 deletions test/backend/test_webservice.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ def test_register(self):
down, addr = s.accept()
self.assertEqual(down.recv(4096), b'0/test/install/bin/service.py')

s.close()
del s
os.unlink('test-socket')

Expand Down Expand Up @@ -307,15 +308,18 @@ def test_get_running_pid(self):
# No state file -> return None
self.assertIsNone(web.get_running_pid())
# FAILED state file -> raise error
open('state_file', 'w').write('FAILED: error\n')
with open('state_file', 'w') as fh:
fh.write('FAILED: error\n')
self.assertRaises(StateFileError, web.get_running_pid)
# Running pid -> return it
ourpid = os.getpid()
open('state_file', 'w').write('%d' % ourpid)
with open('state_file', 'w') as fh:
fh.write('%d' % ourpid)
self.assertEqual(web.get_running_pid(), ourpid)
# Non-running pid -> return None
# Unlikely to have a real pid this large!
open('state_file', 'w').write('99999999')
with open('state_file', 'w') as fh:
fh.write('99999999')
self.assertIsNone(web.get_running_pid())

@testutil.run_in_tempdir
Expand All @@ -328,7 +332,8 @@ def test_filesystem_sanity_check(self):
os.mkdir('incoming')
os.mkdir('preprocessing')
# Garbage files not in job directories are fine
open('garbage-file', 'w').write('test')
with open('garbage-file', 'w') as fh:
fh.write('test')
web._filesystem_sanity_check()

@testutil.run_in_tempdir
Expand All @@ -339,7 +344,8 @@ def test_filesystem_sanity_check_garbage_files(self):
db, conf, web = self._setup_webservice('.')
web._filesystem_sanity_check()
# Make files (not directories) in job directories
open('incoming/garbage-file', 'w').write('test')
with open('incoming/garbage-file', 'w') as fh:
fh.write('test')
self.assertRaises(SanityError, web._filesystem_sanity_check)

@testutil.run_in_tempdir
Expand Down
3 changes: 2 additions & 1 deletion test/pyfrontend/run-all-tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ def __init__(self, *args, **keys):
def make_site_customize(self):
"""Get coverage information on Python subprocesses"""
self.tmpdir = tempfile.mkdtemp()
open(os.path.join(self.tmpdir, 'sitecustomize.py'), 'w').write("""
with open(os.path.join(self.tmpdir, 'sitecustomize.py'), 'w') as fh:
fh.write("""
import coverage
import atexit
_cov = coverage.coverage(branch=True, data_suffix=True, auto_data=True,
Expand Down

0 comments on commit d5a462d

Please sign in to comment.