Author ezio.melotti
Recipients aguiar, chuck, ezio.melotti, pitrou
Date 2009-10-04.14:24:43
SpamBayes Score 0.0
Marked as misclassified No
Message-id <1254666285.16.0.225644352518.issue5395@psf.upfronthosting.co.za>
In-reply-to
Content
I tried to apply both the patches on the trunk but the tests don't pass.
With the latest patch I get an EOFError instead of IOError in the
assertRaises.

The function I was talking about was test_support.unlink(), but that
just removes the file, it doesn't close it, so the try/except is fine
when you close it (I also see that you already added
test_support.unlink() in the new patch).
I agree with Antoine that the helper function is not necessary.


A few more comments about the latest patch:
1) the try/except that imports the os module seems unnecessary to me;
2) if there are really cases where os is not available, the test should
be skipped with a message that says that the os module is not available
and that the test cannot be executed (with return the test is marked as
'passed' even if nothing is actually tested);
3) if os.close(f.fileno()) is required instead of a simpler f.close()
please write a short comment to clarify why;
4) if there are cases where EOFError is raised and they are not tested,
it would be nice to add another test that checks if/when EOFError is raised.
History
Date User Action Args
2009-10-04 14:24:45ezio.melottisetrecipients: + ezio.melotti, pitrou, aguiar, chuck
2009-10-04 14:24:45ezio.melottisetmessageid: <1254666285.16.0.225644352518.issue5395@psf.upfronthosting.co.za>
2009-10-04 14:24:43ezio.melottilinkissue5395 messages
2009-10-04 14:24:43ezio.melotticreate