Éric Araujo <mer...@netwok.org> added the comment: Looks acceptable to me. A few details in the code could be improved:
+ @unittest.skipUnless(sys.platform == 'darwin', 'MacOSX test') Skip messages generally use another form, like “test relevant only on Mac OS X”. + finally: + os.environ = orig_environ I’ve grown fond of using self.addCleanup(setattr, os, 'environ', os.environ.copy()) instead of try/finally. The cleanup action can be written right before the monkey-patching line, there’s no need to indent (especially nice when you patch many things, like later in the patch with sys.stdout), and it’s less lines. + def _try_compile_deployment_target(self): + import textwrap I’d prefer avoiding function-level imports. + fp.close() I suggest a with statement. + tgt = '%02d%01d0'%(tgt) I think that using real words (“target”) and following PEP 8 (“ % target”) would make this slightly more readable. + except CompileError: + self.fail("Wrong deployment target during compilation") Why not just let the CompileError propagate and cause a unittest failure? + self.assertEquals(get_platform(), 'macosx-10.4-fat') assertEquals raises a DeprecationWarning; assertEqual should be used. + + + + # Test without MACOSX_DEPLOYMENT_TARGET in the environment + Three blank lines, a comment line and another blank line is a lot of whitespace. + stderr=open('/dev/null'), Won’t this cause a ResourceWarning? ---------- versions: +Python 3.1 _______________________________________ Python tracker <rep...@bugs.python.org> <http://bugs.python.org/issue9516> _______________________________________ _______________________________________________ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com