andreafrancia / trash-cli

Command line interface to the freedesktop.org trashcan.
GNU General Public License v2.0
3.63k stars 177 forks source link

Running tests leave behind temp directories #218

Closed zpuskas closed 2 years ago

zpuskas commented 2 years ago

Describe the bug Running tests leaves behind temp directories, some of them are empty, some contain some test directories and files.

trash-cli version trash 0.21.10.24

Operating system:

# emerge --info trash-cli
Portage 3.0.28 (python 3.9.7-final-0, default/linux/amd64/17.1/desktop/plasma/systemd, gcc-10.3.0, glibc-2.33-r7, 5.15.0-gentoo x86_64)
=================================================================
                         System Settings
=================================================================
System uname: Linux-5.15.0-gentoo-x86_64-Intel-R-_Core-TM-_i7-8550U_CPU_@_1.80GHz-with-glibc2.33
KiB Mem:    32774044 total,  23331244 free
KiB Swap:    2097128 total,   2097128 free
Timestamp of repository gentoo: Sat, 04 Dec 2021 16:00:01 +0000
Head commit of repository gentoo: 6b80f3ebc54556c4118d54434fd457969b1115a0
Head commit of repository sinustrom: b00db0bb93abf83d5d80f3c36f03d0d31475c8f1

sh bash 5.1_p8
ld GNU ld (Gentoo 2.37_p1 p0) 2.37
distcc 3.4 x86_64-pc-linux-gnu [enabled]
app-shells/bash:          5.1_p8::gentoo
dev-java/java-config:     2.3.1::gentoo
dev-lang/perl:            5.34.0-r5::gentoo
dev-lang/python:          2.7.18_p13::gentoo, 3.8.12_p1::gentoo, 3.9.7_p1::gentoo, 3.10.0_p1::gentoo
dev-lang/rust:            1.56.1::gentoo
dev-util/cmake:           3.21.4::gentoo
sys-apps/baselayout:      2.8::gentoo
sys-apps/sandbox:         2.29::gentoo
sys-devel/autoconf:       2.13-r1::gentoo, 2.71-r1::gentoo
sys-devel/automake:       1.16.5::gentoo
sys-devel/binutils:       2.37_p1::gentoo
sys-devel/gcc:            10.3.0-r2::gentoo
sys-devel/gcc-config:     2.4::gentoo
sys-devel/libtool:        2.4.6-r6::gentoo
sys-devel/make:           4.3::gentoo
sys-kernel/linux-headers: 5.14::gentoo (virtual/os-headers)
sys-libs/glibc:           2.33-r7::gentoo

To Reproduce

$ cd /tmp
$ tar xvf /var/cache/distfiles/trash-cli-0.21.10.24.tar.gz
$ cd trash-cli-0.21.10.24
$ python -m unittest discover -v
$ ls -1 /tmp
...
tmp00wf3wdg
tmp071fqovd
tmp0b6h7515
tmp0c1b7n5s
tmp0e9ppdl0
tmp0r4a_bj7
tmp_1bq6w_q
tmp1ppt0ph0
tmp20la5k0o
tmp24ghpku7
...

Expected behavior After running tests full clean up should include removing the top level temporary directory.

zpuskas commented 2 years ago

Offending test entries seem to be:

Obtained by running tests with the following modification:

>>> git diff
diff --git a/tests/support.py b/tests/support.py
index afbb402..039ce40 100644
--- a/tests/support.py
+++ b/tests/support.py
@@ -1,6 +1,7 @@
 import os
 import shutil
 import tempfile
+import inspect

 def list_trash_dir(trash_dir_path):
@@ -34,4 +35,6 @@ class MyPath(str):

     @classmethod
     def make_temp_dir(cls):
-        return cls(os.path.realpath(tempfile.mkdtemp()))
+        stack = inspect.stack()
+        the_class = stack[1][0].f_locals["self"].__class__.__name__
+        return cls(os.path.realpath(tempfile.mkdtemp(suffix=f"-{the_class}-trash_cli_test")))

and then running:

>>> yes| python -m unittest discover -v
>>> ls -1 /tmp | grep trash_cli_test | cut -d'-' -f2 | sort -u
zpuskas commented 2 years ago

PR #219 should be considered a band aid solution, since it won't prevent similar issues from happening in the future. The proper way to fix this permanently would be to create a tool specific test class:

class TrashCliTestCase(unittest.TestCase):
   def setUp(self):
       self.tmp_dir = MyPath.make_temp_dir()

   def tearDown(self):
       self.tmp_dir.clean_up()

And then update relevant tests to be a child of this class. Example:

class TestTrashEmptyCmd(TrashCliTestCase):

If a test needs additional things to happen in the setUp()/tearDown() then simply use super() to make sure the parent functionality is executed, e.g.:

class TestSomething(TrashCliTestCase):
    def setUp(self):
       super().setUp()
       self.more_stuff = foo()
andreafrancia commented 2 years ago

Your band aid solution is more than good. Merged