AC-161: stability improvements

anaconda · Jul 6, 2023 · ac43028 · ac43028
1 parent a1cd2f0
commit ac43028
Show file tree

Hide file tree

Showing 11 changed files with 170 additions and 200 deletions.
diff --git a/binstar_client/commands/upload.py b/binstar_client/commands/upload.py
@@ -59,9 +59,8 @@ def main(arguments: argparse.Namespace) -> None:
         filename: str
         for filename in sorted(set(itertools.chain.from_iterable(arguments.files))):
             uploader.upload(filename)
-
-        uploader.print_uploads()
     finally:
+        uploader.print_uploads()
         uploader.cleanup()
 
 
@@ -373,12 +372,20 @@ def cleanup(self) -> None:
         Package or release considered to be empty if it was created to upload files to, but all file uploads failed.
         """
         def remove_empty_release(_key: ReleaseKey, record: ReleaseCacheRecord) -> None:
-            logger.info('Removing empty "%s/%s" release after failed upload', record.name, record.version)
-            self.api.remove_release(self.username, record.name, record.version)
+            try:
+                if not self.api.release(self.username, record.name, record.version).get('distributions', []):
+                    logger.info('Removing empty "%s/%s" release after failed upload', record.name, record.version)
+                    self.api.remove_release(self.username, record.name, record.version)
+            except (AttributeError, TypeError, errors.NotFound):
+                pass
 
         def remove_empty_package(_key: PackageKey, record: PackageCacheRecord) -> None:
-            logger.info('Removing empty "%s" package after failed upload', record.name)
-            self.api.remove_package(self.username, record.name)
+            try:
+                if not self.api.package(self.username, record.name).get('files', []):
+                    logger.info('Removing empty "%s" package after failed upload', record.name)
+                    self.api.remove_package(self.username, record.name)
+            except (AttributeError, TypeError, errors.NotFound):
+                pass
 
         CacheRecord.cleanup(self.__release_cache, remove_empty_release)
         CacheRecord.cleanup(self.__package_cache, remove_empty_package)

diff --git a/tests/fixture.py b/tests/fixture.py
@@ -1,12 +1,22 @@
-"""
-Created on Feb 22, 2014
+# -*- coding: utf8 -*-
+
+"""Base components to use in tests."""
+
+from __future__ import annotations
+
+__all__ = ['main', 'AnyIO', 'CLITestCase']
 
-@author: sean
-"""
 import io
 import logging
-import unittest
-from unittest import mock
+import typing
+import unittest.mock
+
+from binstar_client.scripts import cli
+
+
+def main(args: typing.Sequence[str]) -> None:
+    """Alternative entrypoint to use in tests."""
+    cli.main(args, allow_plugin_main=False, exit_=False)
 
 
 class AnyIO(io.StringIO):  # pylint: disable=missing-class-docstring
@@ -18,19 +28,20 @@ def write(self, msg):
 
 
 class CLITestCase(unittest.TestCase):  # pylint: disable=too-many-instance-attributes,missing-class-docstring
+
     def setUp(self):
-        self.get_config_patch = mock.patch('binstar_client.utils.get_config')
+        self.get_config_patch = unittest.mock.patch('binstar_client.utils.get_config')
         self.get_config = self.get_config_patch.start()
         self.get_config.return_value = {}
 
-        self.load_token_patch = mock.patch('binstar_client.utils.config.load_token')
+        self.load_token_patch = unittest.mock.patch('binstar_client.utils.config.load_token')
         self.load_token = self.load_token_patch.start()
         self.load_token.return_value = '123'
 
-        self.store_token_patch = mock.patch('binstar_client.utils.config.store_token')
+        self.store_token_patch = unittest.mock.patch('binstar_client.utils.config.store_token')
         self.store_token = self.store_token_patch.start()
 
-        self.setup_logging_patch = mock.patch('binstar_client.utils.logging_utils.setup_logging')
+        self.setup_logging_patch = unittest.mock.patch('binstar_client.utils.logging_utils.setup_logging')
         self.setup_logging_patch.start()
 
         self.logger = logger = logging.getLogger('binstar')

diff --git a/tests/test_authorizations.py b/tests/test_authorizations.py
@@ -1,16 +1,16 @@
-# pylint: disable=missing-module-docstring,missing-class-docstring,missing-function-docstring
+# -*- coding: utf8 -*-
+# pylint: disable=missing-function-docstring
 
-from __future__ import unicode_literals
-
-import unittest
+"""Tests for anaconda-client authorizations and token management."""
 
 from binstar_client.errors import BinstarError
-from binstar_client.scripts.cli import main
-from tests.fixture import CLITestCase
+from tests.fixture import CLITestCase, main
 from tests.urlmock import urlpatch
 
 
 class Test(CLITestCase):
+    """Tests for anaconda-client authorizations and token management."""
+
     @urlpatch
     def test_remove_token_from_org(self, urls):
         remove_token = urls.register(
@@ -19,7 +19,7 @@ def test_remove_token_from_org(self, urls):
             content='{"token": "a-token"}',
             status=201
         )
-        main(['--show-traceback', 'auth', '--remove', 'tokenname', '-o', 'orgname'], exit_=False)
+        main(['--show-traceback', 'auth', '--remove', 'tokenname', '-o', 'orgname'])
         self.assertIn('Removed token tokenname', self.stream.getvalue())
 
         remove_token.assertCalled()
@@ -32,7 +32,7 @@ def test_remove_token(self, urls):
             content='{"token": "a-token"}',
             status=201
         )
-        main(['--show-traceback', 'auth', '--remove', 'tokenname'], exit_=False)
+        main(['--show-traceback', 'auth', '--remove', 'tokenname'])
         self.assertIn('Removed token tokenname', self.stream.getvalue())
 
         remove_token.assertCalled()
@@ -46,10 +46,6 @@ def test_remove_token_forbidden(self, urls):
             status=403
         )
         with self.assertRaises(BinstarError):
-            main(['--show-traceback', 'auth', '--remove', 'tokenname', '-o', 'wrong_org'], exit_=False)
+            main(['--show-traceback', 'auth', '--remove', 'tokenname', '-o', 'wrong_org'])
 
         remove_token.assertCalled()
-
-
-if __name__ == '__main__':
-    unittest.main()
diff --git a/tests/test_config.py b/tests/test_config.py
@@ -1,35 +1,37 @@
-# pylint: disable=missing-module-docstring,missing-class-docstring,missing-function-docstring
+# -*- coding: utf8 -*-
+# pylint: disable=missing-function-docstring
 
-from __future__ import unicode_literals
+"""Tests for configuration management."""
 
 import shutil
 import tempfile
-from os.path import join, exists
+import os
+import unittest.mock
 
-from unittest import mock
-
-from binstar_client.scripts.cli import main
-from tests.fixture import CLITestCase
+from tests.fixture import CLITestCase, main
 
 
 class Test(CLITestCase):
+    """Tests for configuration management."""
+
     def test_write_env(self):
         tmpdir = tempfile.mkdtemp()
         self.addCleanup(shutil.rmtree, tmpdir)
 
-        with mock.patch('binstar_client.commands.config.USER_CONFIG', join(tmpdir, 'config.yaml')), \
-                mock.patch('binstar_client.commands.config.SEARCH_PATH', [tmpdir]):
-            main(['config', '--set', 'url', 'http://localhost:5000'], exit_=False)
+        with unittest.mock.patch('binstar_client.commands.config.USER_CONFIG', os.path.join(tmpdir, 'config.yaml')), \
+                unittest.mock.patch('binstar_client.commands.config.SEARCH_PATH', [tmpdir]):
+            main(['config', '--set', 'url', 'http://localhost:5000'])
 
-            self.assertTrue(exists(join(tmpdir, 'config.yaml')))
+            self.assertTrue(os.path.exists(os.path.join(tmpdir, 'config.yaml')))
 
-            with open(join(tmpdir, 'config.yaml'), encoding='utf-8') as conf_file:
+            with open(os.path.join(tmpdir, 'config.yaml'), encoding='utf-8') as conf_file:
                 config_output = conf_file.read()
             expected_config_output = 'url: http://localhost:5000\n'
             self.assertEqual(config_output, expected_config_output)
 
-            main(['config', '--show-sources'], exit_=False)
+            main(['config', '--show-sources'])
             expected_show_sources_output = '==> {config} <==\nurl: http://localhost:5000\n\n'.format(
-                config=join(tmpdir, 'config.yaml'))
+                config=os.path.join(tmpdir, 'config.yaml'),
+            )
 
             self.assertIn(expected_show_sources_output, self.stream.getvalue())
diff --git a/tests/test_copy.py b/tests/test_copy.py
@@ -1,26 +1,25 @@
-# pylint: disable=missing-module-docstring,missing-class-docstring,missing-function-docstring
+# -*- coding: utf8 -*-
+# pylint: disable=missing-function-docstring
 
-from __future__ import unicode_literals
+"""Tests for package copy operations."""
 
-# Standard library imports
 import json
-import unittest
 
-# Local imports
 from binstar_client.errors import Conflict
-from binstar_client.scripts.cli import main
 from tests.urlmock import urlpatch
-from tests.fixture import CLITestCase
+from tests.fixture import CLITestCase, main
 
 
 class Test(CLITestCase):
+    """Tests for package copy operations."""
+
     @urlpatch
     def test_copy_label(self, urls):
         urls.register(method='GET', path='/channels/u1', content='["dev"]')
         copy = urls.register(
             method='POST', path='/copy/package/u1/p1/1.0/', content='[{"basename": "copied-file_1.0.tgz"}]')
 
-        main(['--show-traceback', 'copy', '--from-label', 'dev', '--to-label', 'release/xyz', 'u1/p1/1.0'], exit_=False)
+        main(['--show-traceback', 'copy', '--from-label', 'dev', '--to-label', 'release/xyz', 'u1/p1/1.0'])
 
         urls.assertAllCalled()
         req = json.loads(copy.req.body)
@@ -33,9 +32,7 @@ def test_copy_replace(self, urls):
         copy = urls.register(
             method='PUT', path='/copy/package/u1/p1/1.0/', content='[{"basename": "copied-file_1.0.tgz"}]')
 
-        main([
-            '--show-traceback', 'copy', '--from-label', 'dev', '--to-label', 'release/xyz', 'u1/p1/1.0', '--replace',
-        ], exit_=False)
+        main(['--show-traceback', 'copy', '--from-label', 'dev', '--to-label', 'release/xyz', 'u1/p1/1.0', '--replace'])
 
         urls.assertAllCalled()
         req = json.loads(copy.req.body)
@@ -48,9 +45,7 @@ def test_copy_update(self, urls):
         copy = urls.register(
             method='PATCH', path='/copy/package/u1/p1/1.0/', content='[{"basename": "copied-file_1.0.tgz"}]')
 
-        main([
-            '--show-traceback', 'copy', '--from-label', 'dev', '--to-label', 'release/xyz', 'u1/p1/1.0', '--update',
-        ], exit_=False)
+        main(['--show-traceback', 'copy', '--from-label', 'dev', '--to-label', 'release/xyz', 'u1/p1/1.0', '--update'])
 
         urls.assertAllCalled()
         req = json.loads(copy.req.body)
@@ -64,9 +59,7 @@ def test_copy_file_conflict(self, urls):
             method='POST', path='/copy/package/u1/p1/1.0/', status=409
         )
         with self.assertRaises(Conflict):
-            main([
-                '--show-traceback', 'copy', '--from-label', 'dev', '--to-label', 'release/xyz', 'u1/p1/1.0',
-            ], exit_=False)
+            main(['--show-traceback', 'copy', '--from-label', 'dev', '--to-label', 'release/xyz', 'u1/p1/1.0'])
 
         urls.assertAllCalled()
         req = json.loads(copy.req.body)
@@ -80,8 +73,4 @@ def test_copy_argument_error(self, urls):
             main([
                 '--show-traceback', 'copy', '--from-label', 'dev',
                 '--to-label', 'release/xyz', 'u1/p1/1.0', '--update', '--replace',
-            ], exit_=False)
-
-
-if __name__ == '__main__':
-    unittest.main()
+            ])
diff --git a/tests/test_groups.py b/tests/test_groups.py
@@ -1,13 +1,16 @@
-# pylint: disable=unused-argument,missing-module-docstring,missing-class-docstring,missing-function-docstring
+# -*- coding: utf8 -*-
+# pylint: disable=missing-function-docstring
+
+"""Tests for group management commands."""
 
-import unittest
-from tests.fixture import CLITestCase
-from tests.urlmock import urlpatch
-from binstar_client.scripts.cli import main
 from binstar_client import errors
+from tests.fixture import CLITestCase, main
+from tests.urlmock import urlpatch
 
 
 class Test(CLITestCase):
+    """Tests for group management commands."""
+
     @urlpatch
     def test_show(self, urls):
         urls.register(
@@ -16,7 +19,7 @@ def test_show(self, urls):
             content='{"groups": [{"name":"grp", "permission": "read"}]}',
         )
 
-        main(['--show-traceback', 'groups', 'show', 'org'], exit_=False)
+        main(['--show-traceback', 'groups', 'show', 'org'])
 
         urls.assertAllCalled()
 
@@ -28,7 +31,7 @@ def test_show_group(self, urls):
             content='{"name": "owners", "permission": "read", "members_count": 1, "repos_count": 1}',
         )
 
-        main(['--show-traceback', 'groups', 'show', 'org/owners'], exit_=False)
+        main(['--show-traceback', 'groups', 'show', 'org/owners'])
 
         urls.assertAllCalled()
 
@@ -40,14 +43,13 @@ def test_create(self, urls):
             status=204,
         )
 
-        main(['--show-traceback', 'groups', 'add', 'org/new_grp'], exit_=False)
+        main(['--show-traceback', 'groups', 'add', 'org/new_grp'])
 
         urls.assertAllCalled()
 
-    @urlpatch
-    def test_create_missing_group(self, urls):
+    def test_create_missing_group(self):
         with self.assertRaisesRegex(errors.UserError, 'Group name not given'):
-            main(['--show-traceback', 'groups', 'add', 'org'], exit_=False)
+            main(['--show-traceback', 'groups', 'add', 'org'])
 
     @urlpatch
     def test_add_member(self, urls):
@@ -57,14 +59,13 @@ def test_add_member(self, urls):
             status=204,
         )
 
-        main(['--show-traceback', 'groups', 'add_member', 'org/grp/new_member'], exit_=False)
+        main(['--show-traceback', 'groups', 'add_member', 'org/grp/new_member'])
 
         urls.assertAllCalled()
 
-    @urlpatch
-    def test_add_member_missing_member(self, urls):
+    def test_add_member_missing_member(self):
         with self.assertRaisesRegex(errors.UserError, 'Member name not given'):
-            main(['--show-traceback', 'groups', 'add_member', 'org/grp'], exit_=False)
+            main(['--show-traceback', 'groups', 'add_member', 'org/grp'])
 
     @urlpatch
     def test_remove_member(self, urls):
@@ -74,7 +75,7 @@ def test_remove_member(self, urls):
             status=204,
         )
 
-        main(['--show-traceback', 'groups', 'remove_member', 'org/grp/new_member'], exit_=False)
+        main(['--show-traceback', 'groups', 'remove_member', 'org/grp/new_member'])
 
         urls.assertAllCalled()
 
@@ -86,7 +87,7 @@ def test_packages(self, urls):
             content='[{"name": "pkg", "full_name": "org/pkg", "summary": "An org pkg"}]'
         )
 
-        main(['--show-traceback', 'groups', 'packages', 'org/grp'], exit_=False)
+        main(['--show-traceback', 'groups', 'packages', 'org/grp'])
 
         urls.assertAllCalled()
 
@@ -98,7 +99,7 @@ def test_add_package(self, urls):
             status=204,
         )
 
-        main(['--show-traceback', 'groups', 'add_package', 'org/grp/pkg'], exit_=False)
+        main(['--show-traceback', 'groups', 'add_package', 'org/grp/pkg'])
 
         urls.assertAllCalled()
 
@@ -110,10 +111,6 @@ def test_remove_package(self, urls):
             status=204,
         )
 
-        main(['--show-traceback', 'groups', 'remove_package', 'org/grp/pkg'], exit_=False)
+        main(['--show-traceback', 'groups', 'remove_package', 'org/grp/pkg'])
 
         urls.assertAllCalled()
-
-
-if __name__ == '__main__':
-    unittest.main()