Ticket #3208 defect closed fixed

Opened 5 years ago

Last modified 5 years ago

test_logfile uselessly calls shutil.rmtree on tearDown

Reported by: therve Owned by:
Priority: normal Milestone:
Component: core Keywords:
Cc: Branch:
Author: Launchpad Bug:

Description

That causes some intermittent windows failures ( http://buildbot.twistedmatrix.com/builders/winxp32-py2.5-select/builds/310/steps/select/logs/problems for example).

Attachments

3208.diff Download (1.0 KB) - added by therve 5 years ago.
3208.2.diff Download (2.9 KB) - added by therve 5 years ago.
3208.3.diff Download (3.0 KB) - added by therve 5 years ago.

Change History

1

Changed 5 years ago by therve

  • summary changed from test_logfile uselessly call shutil.rmtree on tearDown to test_logfile uselessly calls shutil.rmtree on tearDown

Changed 5 years ago by therve

2

Changed 5 years ago by therve

  • keywords review added
  • owner therve deleted

Please review!

3

Changed 5 years ago by exarkun

  • keywords review removed
  • owner set to therve

The try/finally in LogFileTestCase.testNoPermission seems to be unnecessary given LogFileTestCase.tearDown. What do you think? Can we get rid of that too?

Maybe the comment in LogFileTestCase.tearDown could be a docstring instead (and say for what it is necessary).

Basically great, though.

Changed 5 years ago by therve

Changed 5 years ago by therve

4

Changed 5 years ago by therve

  • owner therve deleted
  • keywords review added

I got rid of it, but added the chmod on self.dir conditionarly (I don't know if that could create a problem). Docstring done, too.

5

Changed 5 years ago by exarkun

  • keywords review removed
  • owner set to therve

Great. Thanks. I only ran the tests on Linux. I don't see anything that should break on Windows, though. One other minor change that might be smart. If somehow we get to tearDown and self.path exists and self.dir is not executable, then os.path.exists will return False. Then we'll make the dir readable but the file will still be unreadable. If we fix the directory first, then this works better. So flip the order if you agree, then apply.

6

Changed 5 years ago by therve

  • status changed from new to closed
  • resolution set to fixed

(In [23430]) Remove useless shutil.rmtree in test_logfile. This removes some potential intermittent failures under Windows.

Author: therve Reviewer: exarkun Fixes #3208

7

Changed 2 years ago by <automation>

  • owner therve deleted
Note: See TracTickets for help on using tickets.