Brett Cannon added the comment: On Jan 28, 2008 6:20 PM, Giampaolo Rodola' <[EMAIL PROTECTED]> wrote: > > Giampaolo Rodola' added the comment: > > > Well, if you have an opinion, feel free to leave a comment in this > > issue about it. I will most likely be the one who does the checkin and > > I will read this issue before I commit. > > * One of the things I dislike is the fact that the student used "self.g > = gdbm.open(self.filename, 'c')" in setUp method since if that fails > there will be a NameError exception raised in tearDown method and the > errors reported by unittest would be 2 where in fact it would be only 1. > Probably a thing like this one would have been better: > > def setUp(self): > self.g = None > > def tearDown(self): > if self.g is not None: > self.g.close() > ... > > def test_it(self): > self.g = gdbm.open(self.filename, 'c') > ... >
Could also still open perform the open() call in setUp() and do what you need in the tearDown() to not be an error. There should probably also be a way to not cause an error if the file path is just not writable as that is not a sign of a failing test but an unavailable resource. > - Another thing I don't like is that os.unlink(self.filename) executed > in tearDown would be better if protected by a try/except statement. > Even better is test.test_support.unlink() which handles those details for you. > - +1 for the self.g.close() used by the student in the tearDown method. > That was a thing I haven't considered and it's not included in my patch. > > - +0.5 for the "test_modes" method added by the student. Maybe it's > useful, maybe it's not, I don't know. > > > Aside from that I don't see other relevant differences since the code is > really minimalistic. Feel free to commit the patch you consider to be > the best. > If you want me to do so I can provide a merged version of my patch > including the differences I described above. Fine by me. Or you can wait until I have time to look at your code. It's up to you. __________________________________ Tracker <[EMAIL PROTECTED]> <http://bugs.python.org/issue1960> __________________________________ _______________________________________________ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
