Storage and portal service tests leak file handles

[hannes-ucsc]

Only reproducible on PR #4562 for #4559.

https://github.com/DataBiosphere/azul/actions/runs/3222566657/jobs/5271770375#step:5:8200

======================================================================
ERROR: tearDownClass (service.test_storage_service.StorageServiceTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/runner/work/azul/azul/test/azul_test_case.py", line 202, in tearDownClass
    super().tearDownClass()
  File "/home/runner/work/azul/azul/test/azul_test_case.py", line 131, in tearDownClass
    assert False, list(map(str, caught_warnings))
AssertionError: ["{message : ResourceWarning('unclosed file <_io.BufferedRandom name=62>'), category : 'ResourceWarning', filename : '/home/runner/work/azul/azul/.venv/lib/python3.9/site-packages/moto/core/base_backend.py', lineno : 33, line : None}"]

Upstream issue is:

https://github.com/spulec/moto/issues/5551

Will add workaround to that PR, expecting the warning.

[melainalegaspi]

@hannes-ucsc to continue PR work in upstream.

[hannes-ucsc]

Upstream ticket was closed in Moto 4.0.8 but versioned buckets still leak keys. Opened new upstream ticket for that:

https://github.com/spulec/moto/issues/5584

[hannes-ucsc]

To repro the leak of keys in versioned buckets, run test_portal_service.TestPortalService without the workaround for this issue.

[hannes-ucsc]

After updating to moto 4.1.2 I can't repro the leak anymore. Given that the upstream issue was closed, its reasonable to assume that this issue is fixed for good.

[hannes-ucsc]

Unfortunately, there is still a leak

https://github.com/DataBiosphere/azul/actions/runs/4129707604/jobs/7135609219

I've opened https://github.com/getmoto/moto/issues/5913.

[hannes-ucsc]

The quickest repro with our own unit tests is to combine version and storage service tests and running the tests in a thusly modified test_version_service.py:

Index: test/service/test_version_service.py
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/test/service/test_version_service.py b/test/service/test_version_service.py
--- a/test/service/test_version_service.py  (revision 32a276e27aa9e2ccd37b5839d3c9b8fc1829cd17)
+++ b/test/service/test_version_service.py  (date 1675906827179)
@@ -1,7 +1,10 @@
+import tempfile
 import unittest

 from moto import (
     mock_dynamodb,
+    mock_s3,
+    mock_sts,
 )

 from azul.logging import (
@@ -11,6 +14,12 @@
     VersionConflict,
     VersionService,
 )
+from azul_test_case import (
+    AzulUnitTestCase,
+)
+from service import (
+    StorageServiceTestCase,
+)
 from version_table_test_case import (
     VersionTableTestCase,
 )
@@ -21,6 +30,24 @@
     configure_test_logging()

+class StorageServiceTest(AzulUnitTestCase, StorageServiceTestCase):
+    """
+    Functional Test for Storage Service
+    """
+
+    @mock_s3
+    @mock_sts
+    def test_upload_tags(self):
+        self.storage_service.create_bucket()
+
+        object_key = 'test_file'
+        with tempfile.NamedTemporaryFile('w') as f:
+            f.write('some contents')
+            f.flush()
+            self.storage_service.upload(file_path=f.name,
+                                        object_key=object_key)
+
+
 @mock_dynamodb
 class TestVersionService(VersionTableTestCase):

[hannes-ucsc]

Also note some weirdness with test_upload_tags(). WIthout the flush, the written data is buffered, the file is still empty and the object body is the emtpy string.

Index: test/service/test_storage_service.py
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/test/service/test_storage_service.py b/test/service/test_storage_service.py
--- a/test/service/test_storage_service.py  (revision 32a276e27aa9e2ccd37b5839d3c9b8fc1829cd17)
+++ b/test/service/test_storage_service.py  (date 1675906827182)
@@ -36,6 +36,7 @@
         object_key = 'test_file'
         with tempfile.NamedTemporaryFile('w') as f:
             f.write('some contents')
+            f.flush()
             for tags in (None, {}, {'Name': 'foo', 'game': 'bar'}):
                 with self.subTest(tags=tags):
                     self.storage_service.upload(file_path=f.name,