Add test to make sure files are not being left open after sync tasks #1232
Conversation
dralley
force-pushed
the
file-close-test
branch
4 times, most recently
from
427fc91
to
0061b12
Compare
| """ | ||
| pkg_mgr = cli.PackageManager(self.cfg) | ||
| pkg_mgr.raise_if_unsupported(unittest.SkipTest) | ||
| pkg_mgr.install('lsof') |
There was a problem hiding this comment.
@kersommoura I left this just as it was committed to pulp_file, but I kind of think it would be better if the test didn't try to do install or uninstall anything - just check if lsof is available and skip if it isn't installed. The reason being, if the user already had lsof installed, either individually or as a dependency for another application, wouldn't the next line would remove it from their system? I don't think we should be doing anything that would make permanent changes to the system configuration of the host machine.
I think it would make more sense to document dependencies like this and have the user (or the CI box configuration scripts) make sure they're installed if they want that test to be run.
Also, lsof is installed on Travis (or maybe it's included by default with Ubuntu), so doing it this way is needlessly restrictive. If not for the code trying to interact with the system package manager this test would actually work fine in Travis and not need to be skipped.
There was a problem hiding this comment.
If you agree, I don't mind changing it both here and in pulp_file.
There was a problem hiding this comment.
@dralley, thanks for your working on this. I agree. What about we state in the test case something like...this test requires that lsof is available. Otherwise test will be skipped.
dralley
force-pushed
the
file-close-test
branch
4 times, most recently
from
99d3fd7
to
90f0413
Compare
dralley
force-pushed
the
file-close-test
branch
from
90f0413
to
a313b66
Compare
|
@pulp/qe |
re #4073
https://pulp.plan.io/issues/4073