From 1dc207646cb9482a01e8b356658ab6767df60a4b Mon Sep 17 00:00:00 2001 From: Caleb Harris Date: Sat, 6 Sep 2025 09:22:44 -0500 Subject: [PATCH 1/2] Updates code to adhere to PEP8. Also adjusts Tox only check the sciunit source code and tests. --- sciunit2/cli.py | 2 +- sciunit2/command/diff.py | 28 ++++++++---- sciunit2/command/export.py | 90 ++++++++++++++++++++++++++------------ sciunit2/command/given.py | 6 ++- sciunit2/command/remove.py | 4 +- sciunit2/command/rm.py | 2 +- sciunit2/records.py | 9 ++-- sciunit2/wget.py | 2 +- sciunit2/workspace.py | 6 ++- tests/test_archiver.py | 6 +-- tests/test_copy.py | 3 +- tox.ini | 2 +- 12 files changed, 106 insertions(+), 54 deletions(-) diff --git a/sciunit2/cli.py b/sciunit2/cli.py index 82e3427..4a0e924 100644 --- a/sciunit2/cli.py +++ b/sciunit2/cli.py @@ -80,7 +80,7 @@ def main(): def _main(args): - if platform.system().startswith('Linux') == False: + if not platform.system().startswith('Linux'): err1('Platform is not supported') sys.exit(1) diff --git a/sciunit2/command/diff.py b/sciunit2/command/diff.py index 831410b..44c0c6c 100644 --- a/sciunit2/command/diff.py +++ b/sciunit2/command/diff.py @@ -31,13 +31,20 @@ def run(self, args): # exclusive performs a lock on the berkeleydb instance with emgr.exclusive(): orig1 = emgr.get(args[0]).cmd - diffdir = os.path.join(repo.location, 'Diff') # repo.location is the project dir + # repo.location is the project dir + diffdir = os.path.join(repo.location, 'Diff') repo.checkout_Diff(args[0]) orig2 = emgr.get(args[1]).cmd repo.checkout_Diff(args[1]) - cmd = quoted_format('rsync -nai --delete {0}/ {1}/', args[0], args[1]) - p = subprocess.Popen(cmd, shell=True, cwd=diffdir, stderr=PIPE, stdout=PIPE) + cmd = quoted_format( + 'rsync -nai --delete {0}/ {1}/', args[0], args[1]) + p = subprocess.Popen( + cmd, + shell=True, + cwd=diffdir, + stderr=PIPE, + stdout=PIPE) out, err = p.communicate() p_return_code = p.wait() if p_return_code != 0: @@ -50,9 +57,12 @@ def run(self, args): output = "Difference in e1 and e2:\n" + \ "Files only in e1:\n" + '\n'.join(new_e1) + "\n\n" + \ "Files only in e2:\n" + '\n'.join(new_e2) + "\n\n" + \ - "Files with changed size:\n" + '\n'.join(size_changed) + "\n\n" + \ - "Files with changed modified time:\n" + '\n'.join(time_changed) + "\n\n" + \ - "Files with changed permissions:\n" + '\n'.join(perms_changed) + "\n\n" + "Files with changed size:\n" + \ + '\n'.join(size_changed) + "\n\n" + \ + "Files with changed modified time:\n" + \ + '\n'.join(time_changed) + "\n\n" + \ + "Files with changed permissions:\n" + \ + '\n'.join(perms_changed) + "\n\n" except Exception: output = "error executing diff command!" @@ -68,7 +78,7 @@ def run(self, args): Files with changed permissions: Detail on output format of rsync could be found at: - https://linux.die.net/man/1/rsync + https://linux.die.net/man/1/rsync """ @staticmethod def parse_rsync(out): @@ -82,7 +92,7 @@ def parse_rsync(out): perms_changed = [] for line in lines: splits = line.split() - # The length of splits is two for regular files and directories + # The length of splits is two for regular files and directories # and 4 for symlinks # assert len(splits) == 2 YXcstpoguax = splits[0].strip() @@ -115,7 +125,7 @@ def parse_rsync(out): time_changed.append(file_name) if file_perms == 'p': # permissions of file changed perms_changed.append(file_name) - else: # rest of the cases not handled right now + else: # rest of the cases not handled right now continue return new_e1, new_e2, size_changed, time_changed, perms_changed diff --git a/sciunit2/command/export.py b/sciunit2/command/export.py index 329c7d3..ffc98e4 100644 --- a/sciunit2/command/export.py +++ b/sciunit2/command/export.py @@ -117,7 +117,8 @@ def find_pkg_list(site_package_dirs): for (dirpath, dirnames, filenames) in os.walk(package_path): version_file = None version = None - version_file_list = list(filter(lambda file: 'version' in file, filenames)) + version_file_list = list( + filter(lambda file: 'version' in file, filenames)) if version_file_list: version_file = version_file_list[0] else: @@ -129,12 +130,15 @@ def find_pkg_list(site_package_dirs): with open(os.path.join(dirpath, version_file)) as f: lines = f.readlines() for line in lines: - version_line_list = re.findall(r'^_*version_*\s*=', line) + version_line_list = re.findall( + r'^_*version_*\s*=', line) if version_line_list: version_split = line.split('=') - if version_split and len(version_split) >= 2: + if version_split and len( + version_split) >= 2: version = version_split[1].strip() - version = version.strip('"').strip('\'') + version = version.strip( + '"').strip('\'') version = str(version) # confirm its a number if not version[:1].isdigit(): @@ -184,7 +188,8 @@ class ExportCommand(AbstractCommand): @property def usage(self): return [('export [virtualenv]', - 'Exports the dependencies of into requirements.txt.\n' + 'Exports the dependencies of ' + ' into requirements.txt.\n' 'Optionally creates new virtualenv instance and ' 'installs all python dependencies in it.\n')] @@ -221,7 +226,8 @@ def run(self, args): log_file = "provenance.cde-root.1.log" log_file_path = cde_pkg_dir + "/" + log_file cde_root_dir = cde_pkg_dir + "/cde-root" - site_pkg_dirs, python_path, py_version = python_paths(log_file_path, cde_root_dir) + site_pkg_dirs, python_path, py_version = python_paths( + log_file_path, cde_root_dir) if not site_pkg_dirs: print('Aborting export! Environment paths not found') return None @@ -233,55 +239,84 @@ def run(self, args): pkg_str = write_req_file(py_version, pkg_list, req_file) if create_venv: retcode = 0 - # 1. install virtualenv using pip(assuming pip is installed) + # 1. install virtualenv using pip(assuming pip is + # installed) try: - retcode ^= subprocess.call(sys.executable + ' -m pip install' + - ' --force-reinstall' + - ' virtualenv &>' + output_log, - shell=True, executable='/bin/bash') + retcode ^= subprocess.call( + sys.executable + + ' -m pip install' + + ' --force-reinstall' + + ' virtualenv &>' + + output_log, + shell=True, + executable='/bin/bash') except CalledProcessError as err: print(err.stderr) print('Aborting export! Error installing virtualenv') - print('Detailed output log can be found in: ' + output_log) + print( + 'Detailed output log can be found in: ' + + output_log) return None if retcode != 0: print('Aborting export! Error installing virtualenv') - print('Detailed output log can be found in: ' + output_log) + print( + 'Detailed output log can be found in: ' + + output_log) return None # 2. create new environment using virtualenv and # install all Python dependencies in it print('Creating new virtual environment...') - retcode ^= subprocess.call('`which virtualenv` ' + env_name - + ' &>>' + output_log, shell=True, executable='/bin/bash') + retcode ^= subprocess.call( + '`which virtualenv` ' + + env_name + + ' &>>' + + output_log, + shell=True, + executable='/bin/bash') if retcode == 0: print('Installing Python packages...') env_python_path = env_name + '/bin/python' for pkg in tqdm(pkg_str.splitlines()): - cmd = env_python_path + ' -m pip install ' + pkg + ' &>> ' + output_log - subprocess.call(cmd, shell=True, executable='/bin/bash') + cmd = env_python_path + ' -m pip install ' + \ + pkg + ' &>> ' + output_log + subprocess.call( + cmd, shell=True, executable='/bin/bash') else: print('Aborting export! Error installing dependencies') - print('Detailed output log can be found in: ' + output_log) + print( + 'Detailed output log can be found in: ' + + output_log) return None if retcode != 0: print('Aborting export!') - print('Detailed output log can be found in: ' + output_log) + print( + 'Detailed output log can be found in: ' + + output_log) return None # 3. create new folder in cwd and bring all code+data # in /home/ there _mkdir_p(data_dir) - subprocess.call('cp -r ' + user_dir + ' ' + data_dir, shell=True) - print("A new virtual environment '" + env_name + "' successfully " + - "created with your Python packages.\n" - "Detailed output log can be found in: " + output_log) - print('You can activate the virtual environment as follows:\n' - '\tsource ' + env_name + '/bin/activate') + subprocess.call( + 'cp -r ' + user_dir + ' ' + data_dir, shell=True) + print( + "A new virtual environment '" + + env_name + + "' successfully " + + "created with your Python packages.\n" + "Detailed output log can be found in: " + + output_log) + print( + 'You can activate the virtual ' + 'environment as follows:\n' + '\tsource ' + env_name + '/bin/activate') print('Your code and data have been copied to the dir: ' + data_dir + '\n') print('To deactivate the environment, enter: deactivate') - print('Make sure to run the following command from your shell once:') + print( + 'Make sure to run the following ' + 'command from your shell once:') print('\texport SSL_CERT_DIR=/etc/ssl/certs/') else: print('Export not successful!') @@ -291,4 +326,5 @@ def run(self, args): return eid def note(self, eid): - return quoted_format('Exported python dependencies of execution {0} \n', eid) + return quoted_format( + 'Exported python dependencies of execution {0} \n', eid) diff --git a/sciunit2/command/given.py b/sciunit2/command/given.py index 0330cc9..3fbf4ed 100644 --- a/sciunit2/command/given.py +++ b/sciunit2/command/given.py @@ -48,7 +48,8 @@ def run(self, args): copytree(os.path.relpath(f, '/'), dst) join_fn = str.__add__ else: - dst = de.cwd_on_host() # project dir inside ./cde-root/root/home + # project dir inside ./cde-root/root/home + dst = de.cwd_on_host() for f in files: copytree(f, dst) join_fn = os.path.join @@ -73,9 +74,10 @@ def run(self, args): self.do_commit(pkgdir, rev, emgr, repo) return sys.exit(repeat_out) + def copytree(src, dst): if not os.path.isdir(src): path = src.rsplit("/", 1)[0] else: path = src - os.makedirs(dst+"/"+path, exist_ok=True) + os.makedirs(dst + "/" + path, exist_ok=True) diff --git a/sciunit2/command/remove.py b/sciunit2/command/remove.py index 52dd638..c93382e 100644 --- a/sciunit2/command/remove.py +++ b/sciunit2/command/remove.py @@ -35,4 +35,6 @@ def run(self, args): return None def note(self, project_dir): - return quoted_format('Successfully removed sciunit project {0}\n', project_dir) + return quoted_format( + 'Successfully removed sciunit project {0}\n', + project_dir) diff --git a/sciunit2/command/rm.py b/sciunit2/command/rm.py index 5b6ca6b..64bb2f7 100644 --- a/sciunit2/command/rm.py +++ b/sciunit2/command/rm.py @@ -57,7 +57,7 @@ def run(self, args): # # for id_a in bounds: # repo.unlink(self.__to_rev(id_a)) - for _id in range(bounds[0], bounds[1]+1): + for _id in range(bounds[0], bounds[1] + 1): repo.unlink(self.__to_rev(_id)) except MalformedExecutionId: diff --git a/sciunit2/records.py b/sciunit2/records.py index ca2cc66..a4f2815 100644 --- a/sciunit2/records.py +++ b/sciunit2/records.py @@ -116,7 +116,7 @@ def add(self, args): try: last_row_id = self.__c.execute(script).fetchone()[0] - if last_row_id != None: + if last_row_id is not None: last_id = last_row_id + 1 except Error as e: @@ -149,7 +149,7 @@ def get_last_id(self): script = "select max(id) from revs" last_row_id = self.__c.execute(script).fetchone()[0] - if last_row_id != None: + if last_row_id is not None: return last_row_id else: return 1 @@ -169,7 +169,7 @@ def __get(self, i): row = self.__c.execute(script).fetchone() - if row != None: + if row is not None: return row[1] else: raise CommandError('execution %r not found' % self.__to_rev(i)) @@ -230,7 +230,7 @@ def deletemany(self, revrange): # for idb in range(bounds[0], bounds[1]): # self.delete_id(idb) - for _id in range(bounds[0], bounds[1]+1): + for _id in range(bounds[0], bounds[1] + 1): self.delete_id(_id) def sort(self, revls): @@ -301,7 +301,6 @@ def __for_from(self, first, f): # todo pass - @staticmethod def __to_rev(id_): return 'e%d' % id_ diff --git a/sciunit2/wget.py b/sciunit2/wget.py index d2c8530..a559ce6 100644 --- a/sciunit2/wget.py +++ b/sciunit2/wget.py @@ -22,6 +22,6 @@ def http_error_default(self, url, fp, code, msg, hdrs): def fetch(url, base): with tempfile.NamedTemporaryFile(prefix=base, dir='') as fp, \ - TqdmHook(unit='B', unit_scale=True, miniters=1) as t: + TqdmHook(unit='B', unit_scale=True, miniters=1) as t: ThrowOnErrorOpener().retrieve(url, fp.name, t.update_to) return open(fp.name, 'rb') diff --git a/sciunit2/workspace.py b/sciunit2/workspace.py index d8c54dd..7844613 100644 --- a/sciunit2/workspace.py +++ b/sciunit2/workspace.py @@ -96,8 +96,10 @@ def _create(name, by, overwrite=False): # checks if the given folder exists def _delete(name, by): if not by(location_for(name)): - raise CommandError('directory %s does not exists for delete operation' % - shlex.quote(location_for(name))) + raise CommandError( + 'directory %s does not exists for delete operation' % + shlex.quote( + location_for(name))) # opens a sciunit container already created diff --git a/tests/test_archiver.py b/tests/test_archiver.py index 15779a7..a08c879 100644 --- a/tests/test_archiver.py +++ b/tests/test_archiver.py @@ -47,9 +47,9 @@ def test_layout(): f.write('tmp/test/a.txt', 'test/2nd/a.txt') assert_true(sciunit2.archiver.extract( - p, - lambda x: True, - lambda x: os.path.join('tmp', x)) + p, + lambda x: True, + lambda x: os.path.join('tmp', x)) .startswith(os.path.join('tmp', 'test__'))) with zipfile.ZipFile(p, 'a') as f: diff --git a/tests/test_copy.py b/tests/test_copy.py index 6abfbc8..0767f3d 100644 --- a/tests/test_copy.py +++ b/tests/test_copy.py @@ -29,7 +29,8 @@ def test_all(self): testit.sciunit('open', 'nonexistent#') assert_equal(r.exception.code, 1) - # these test cases need revision because copy functionality is depdendent on file.io which + # these test cases need revision because copy functionality + # is depdendent on file.io which # has been changed to limewire. We need a new service. # out = StringIO() # with mock.patch('sys.stdout', out): diff --git a/tox.ini b/tox.ini index ddaca3f..f608f5d 100644 --- a/tox.ini +++ b/tox.ini @@ -14,5 +14,5 @@ deps = -r{toxinidir}/test-requirements.txt [testenv:pep8] basepython = python3.13 deps = pycodestyle -commands = - pycodestyle +commands = - pycodestyle ./sciunit2 ./tests usedevelop = True From ca8e4c3925fbb0ec6dc71d30544d835151d29cc5 Mon Sep 17 00:00:00 2001 From: Caleb Harris Date: Fri, 10 Oct 2025 19:30:17 -0500 Subject: [PATCH 2/2] fix: Fixed sciunit rm issue where if the user removes the latest eid then it breaks the sqlite db. This was caused by the db using autoincrement so the eid in the db and the add functionality mismatched. (https://github.com/radiant-systems-lab/sciunit/issues/21) --- .idea/workspace.xml | 60 +++++++++++++++++++++++++++++++-------------- sciunit2/records.py | 4 +-- tests/test_rm.py | 41 +++++++++++++++++++++++++++++++ 3 files changed, 84 insertions(+), 21 deletions(-) diff --git a/.idea/workspace.xml b/.idea/workspace.xml index 65f2441..7595e4c 100644 --- a/.idea/workspace.xml +++ b/.idea/workspace.xml @@ -1,18 +1,12 @@ + + - - - - - + + - - - - - - - - - - - - - + + + + + + + + + + + + + @@ -159,6 +169,15 @@ @@ -171,7 +190,10 @@ - + + + diff --git a/sciunit2/records.py b/sciunit2/records.py index dbaa0ee..88bb339 100644 --- a/sciunit2/records.py +++ b/sciunit2/records.py @@ -130,8 +130,8 @@ def commit(self, size): k, v = self.__pending # eid, Metadata v.size = size # self.__f[k] = str(v) - script = "insert into revs (data) values (?)" - self.__c.execute(script, [str(v)]) + script = "insert into revs (id,data) values (?,?)" + self.__c.execute(script, [str(k),str(v)]) self.__f.commit() return self.__to_rev(k), v diff --git a/tests/test_rm.py b/tests/test_rm.py index 6df4f45..60d2378 100644 --- a/tests/test_rm.py +++ b/tests/test_rm.py @@ -43,6 +43,25 @@ def test_single(self): assert_is_none(testit.sciunit('exec', 'true')) + for _ in range(3): + testit.sciunit('exec', 'true') + + testit.sciunit('rm', 'e4') + + with assert_raises(SystemExit) as r: + testit.sciunit('repeat', 'e4') + assert_equal(r.exception.code, 1) + + testit.sciunit('exec', 'true') + + with assert_raises(SystemExit) as r: + testit.sciunit('repeat', 'e4') + assert_equal(r.exception.code, 0) + + with assert_raises(SystemExit) as r: + testit.sciunit('repeat', 'e5') + assert_equal(r.exception.code, 1) + def test_range(self): with assert_raises(SystemExit) as r: testit.sciunit('rm', 'e1-') @@ -103,3 +122,25 @@ def test_range(self): assert_is_none(testit.sciunit('rm', 'e1-10')) assert_is_none(testit.sciunit('exec', 'true')) + + for _ in range(4): + testit.sciunit('exec', 'true') + + testit.sciunit('rm', 'e3-5') + + for eid in ['e3', 'e4', 'e5']: + with assert_raises(SystemExit) as r: + testit.sciunit('repeat', eid) + assert_equal(r.exception.code, 1) + + testit.sciunit('exec', 'true') + + with assert_raises(SystemExit) as r: + testit.sciunit('repeat', 'e3') + assert_equal(r.exception.code, 0) + + testit.sciunit('exec', 'true') + + with assert_raises(SystemExit) as r: + testit.sciunit('repeat', 'e4') + assert_equal(r.exception.code, 0)