Message52646
+1 on the fix.
I do have a few minor suggestions, however. In the fix itself, the parens in "(src_dir)" are still there. If the fix will only be applied in >= 2.5, you might also use the "plen = len(src_dir)+1 if src_dir else 0" form.
As for the test, since you're testing a very specific error scenario (Distribution.run_commands raising DistutilsFileError), I'd suggest catching the exception and using "self.fail", so that testing the unfixed code fails instead of issues an error. Since you expect the error to occur inside run_commands, I'd put the try...except block around the "dist.run_commands()" only, as in:
# Auxiliary variable, since you need cleanup and self.fail exits before "finally":
error = False
try:
....dist.run_commands()
except DistutilsFileError:
....error = True
finally:
....os.chdir(cwd)
if error:
....self.fail("run_commands fails with no package_dir")
Gustavo Niemeyer also mentioned you should add a dosctring to the test explaining what exactly it's testing.
To sum it up, I recommend commit of this patch regardless of my comments above, but I hope they're taken into consideration :) |
|
Date |
User |
Action |
Args |
2007-08-23 15:58:29 | admin | link | issue1720897 messages |
2007-08-23 15:58:29 | admin | create | |
|