Sat, 11 Nov 2006 18:42:19 -0500
[svn] Add a test for recursive extraction which also makes sure that we fix
permissions after we extract the archive, and DTRT when an archive contains
one file. Add code to handle the latter two cases.
ExtractorApplication is a total mess at this point. I already am having a
hard time following how the pieces fit together. Cleaning it up is my next
task; that'll be easier now that I test most of the functionality again.
TODO | file | annotate | diff | comparison | revisions | |
scripts/x | file | annotate | diff | comparison | revisions | |
tests/compare.py | file | annotate | diff | comparison | revisions | |
tests/test-recursive-badperms.tar.bz2 | file | annotate | diff | comparison | revisions |
--- a/TODO Mon Nov 06 22:36:47 2006 -0500 +++ b/TODO Sat Nov 11 18:42:19 2006 -0500 @@ -1,10 +1,9 @@ Things which I have a use case/anti-use case for: +* Clean up ExtractorApplication, ugh. +* Reconsider the current extraction policy for the ONE_DIRECTORY case. * Decompress non-archive files. -* DTRT in the "archive contains one file" case. -* Fix permissions as you extract. Things that are generally good: -* Usage information. * Better tests. * Better error messages.
--- a/scripts/x Mon Nov 06 22:36:47 2006 -0500 +++ b/scripts/x Sat Nov 11 18:42:19 2006 -0500 @@ -312,10 +312,18 @@ self.show_error("all good names for an extraction directory taken") return directory + def move_to_directory(self, filename, target): + if not os.path.isdir(filename): + filename = os.path.split(filename)[0] + target = os.path.join(target, filename) + os.rename(filename, target) + def prepare_extraction(self): self.current_path = '.' contents = self.current_extractor.check_contents() - if contents not in (MATCHING_DIRECTORY, EMPTY): + if contents == MATCHING_DIRECTORY: + self.target_directory = self.current_filename + elif contents != EMPTY: self.target_directory = self.prepare_target_directory() if self.target_directory is None: return False @@ -323,10 +331,10 @@ os.chdir(self.target_directory) self.current_path = '..' else: - self.cleanup_actions.append((os.rename, contents, + self.cleanup_actions.append((self.move_to_directory, contents, self.target_directory)) else: - self.target_directory = os.curdir + self.target_directory = None return True def extract(self): @@ -340,15 +348,27 @@ def recurse(self): if not self.options.recursive: return True - print "wow", self.current_extractor.included_archives for filename in self.current_extractor.included_archives: tail_path, basename = os.path.split(filename) directory = os.path.join(self.current_directory, self.target_directory, tail_path) self.archives.setdefault(directory, []).append(basename) - print self.archives return True + def fix_perms(self): + if self.target_directory is None: + return True + status = subprocess.call(['chmod', '-R', 'u+rw', + os.path.join(self.current_directory, + self.target_directory)]) + if status == 0: + status = subprocess.call(['find', + os.path.join(self.current_directory, + self.target_directory), + '-type', 'd', + '-exec', 'chmod', 'u+x', '{}', ';']) + return status == 0 + def run(self): while self.archives: self.current_directory, filenames = self.archives.popitem() @@ -362,6 +382,7 @@ running = actions.pop(0)() for action in self.cleanup_actions: action[0](*action[1:]) + running = self.fix_perms() if running: self.successes.append(self.current_filename) else:
--- a/tests/compare.py Mon Nov 06 22:36:47 2006 -0500 +++ b/tests/compare.py Sat Nov 11 18:42:19 2006 -0500 @@ -28,23 +28,34 @@ set -e """ -tests = {'test-1.23.tar': ['tar -xf test-1.23.tar'], - 'test-1.23.tar.gz': ['tar -xzf test-1.23.tar.gz'], - 'test-1.23.tar.bz2': ['mkdir test-1.23', - 'cd test-1.23', - 'tar -jxf ../test-1.23.tar.bz2'], - 'test-1.23.zip': ['mkdir test-1.23', - 'cd test-1.23', - 'unzip -q ../test-1.23.zip'], - 'test-1.23.cpio': ['cpio -i --make-directories \ - <test-1.23.cpio 2>/dev/null'], - 'test-1.23_all.deb': ['TD=$PWD', - 'mkdir test-1.23', - 'cd /tmp', - 'ar x $TD/test-1.23_all.deb data.tar.gz', - 'cd $TD/test-1.23', - 'tar -zxf /tmp/data.tar.gz', - 'rm /tmp/data.tar.gz']} +tests = {'test-1.23.tar': ([], ['tar -xf test-1.23.tar'], []), + 'test-1.23.tar.gz': ([], ['tar -xzf test-1.23.tar.gz'], []), + 'test-1.23.tar.bz2': ([], ['mkdir test-1.23', + 'cd test-1.23', + 'tar -jxf ../test-1.23.tar.bz2'], []), + 'test-1.23.zip': ([], ['mkdir test-1.23', + 'cd test-1.23', + 'unzip -q ../test-1.23.zip'], []), + 'test-1.23.cpio': ([], ['cpio -i --make-directories \ + <test-1.23.cpio 2>/dev/null'], []), + 'test-1.23_all.deb': ([], ['TD=$PWD', + 'mkdir test-1.23', + 'cd /tmp', + 'ar x $TD/test-1.23_all.deb data.tar.gz', + 'cd $TD/test-1.23', + 'tar -zxf /tmp/data.tar.gz', + 'rm /tmp/data.tar.gz'], []), + 'test-recursive-badperms.tar.bz2': ( + ['-r'], + ['mkdir test-recursive-badperms', + 'cd test-recursive-badperms', + 'tar -jxf ../test-recursive-badperms.tar.bz2', + 'tar -xf test-badperms.tar', + 'mv testdir test-badperms', + 'chmod 755 test-badperms'], + ['if [ "x`cat test-recursive-badperms/test-badperms/testfile`" = "xhey" ]', + 'then exit 0; else exit 1; fi'] + )} if os.path.exists('scripts/x') and os.path.exists('tests'): os.chdir('tests') @@ -59,9 +70,9 @@ class ExtractorTest(object): - def __init__(self, archive_filename, commands): + def __init__(self, archive_filename, info): self.archive_filename = archive_filename - self.shell_commands = commands + self.arguments, self.shell_commands, self.shell_test = info def get_results(self, commands): status = subprocess.call(commands) @@ -73,17 +84,26 @@ process.stdout.close() return set(output.split('\n')) - def get_shell_results(self): + def write_script(self, commands): script = open(TESTSCRIPT_NAME, 'w') - script.write("%s%s\n" % (SCRIPT_PROLOGUE, - '\n'.join(self.shell_commands))) + script.write("%s%s\n" % (SCRIPT_PROLOGUE, '\n'.join(commands))) script.close() subprocess.call(['chmod', 'u+w', TESTSCRIPT_NAME]) + + def get_shell_results(self): + self.write_script(self.shell_commands) return self.get_results(['sh', TESTSCRIPT_NAME]) def get_extractor_results(self): - return self.get_results(['../scripts/x', self.archive_filename]) + return self.get_results(['../scripts/x'] + self.arguments + + [self.archive_filename]) + def get_posttest_result(self): + if not self.shell_test: + return 0 + self.write_script(self.shell_test) + return subprocess.call(['sh', TESTSCRIPT_NAME]) + def clean(self): status = subprocess.call(['find', '-mindepth', '1', '-maxdepth', '1', '-type', 'd', @@ -98,6 +118,7 @@ expected = self.get_shell_results() self.clean() actual = self.get_extractor_results() + posttest_result = self.get_posttest_result() self.clean() if expected is None: raise ExtractorTestError("could not get baseline results") @@ -110,6 +131,11 @@ print "Only in actual results:" print '\n'.join(actual.difference(expected)) return False + elif posttest_result != 0: + print "FAILED:", self.archive_filename + print "Posttest returned status code", posttest_result + print actual + return False else: print "Passed:", self.archive_filename return True