Refactoring Galaxy unit tests for uniformity (#20828)

* Making tests more uniform

Removing unnecessary GalaxyCLI arguments/patching of the command line since parsing of the CLI args has been modified.

Run GalaxyCLI.parse() without saving the returned value to be uniform with the rest of the code.

Fix test_execute_remove to use the correct path

Use GalaxyCLI.run() instead of super(GalaxyCLI, gc).run() and GalaxyCLI.api = ansible.galaxy.api.GalaxyAPI(gc.galaxy).

* Refactor so one unit test checks one thing instead of multiple.

Improve readability by using a dict instead of lots of elifs

* Removing import used for debugging

* Fixing PEP 8 issues.

* Fix PEP 8 issues

test/units/cli/test_galaxy.py

@@ -33,7 +33,6 @@ from ansible.errors import AnsibleError, AnsibleOptionsError
 
 from ansible.cli.galaxy import GalaxyCLI
 
-
 class TestGalaxy(unittest.TestCase):
     @classmethod
     def setUpClass(cls):
@@ -44,7 +43,7 @@ class TestGalaxy(unittest.TestCase):
             shutil.rmtree("./delete_me")
 
         # creating framework for a role
-        gc = GalaxyCLI(args=["init", "-c", "--offline", "delete_me"])
+        gc = GalaxyCLI(args=["init", "--offline", "delete_me"])
         gc.parse()
         gc.run()
         cls.role_dir = "./delete_me"
@@ -114,9 +113,8 @@ class TestGalaxy(unittest.TestCase):
 
     def test_run(self):
         ''' verifies that the GalaxyCLI object's api is created and that execute() is called. '''
-        gc = GalaxyCLI(args=["install"])
+        gc = GalaxyCLI(args=["install", "--ignore-errors", "imaginary_role"])
-        with patch('sys.argv', ["-c", "-v", '--ignore-errors', 'imaginary_role']):
+        gc.parse()
-            galaxy_parser = gc.parse()
         with patch.object(ansible.cli.CLI, "execute", return_value=None) as mock_ex:
             with patch.object(ansible.cli.CLI, "run", return_value=None) as mock_run:
                 gc.run()
@@ -129,40 +127,36 @@ class TestGalaxy(unittest.TestCase):
     def test_execute_remove(self):
         # installing role
         gc = GalaxyCLI(args=["install", "--offline", "-p", self.role_path, "-r", self.role_req])
-        galaxy_parser = gc.parse()
+        gc.parse()
         gc.run()
 
-        # checking that installation worked
+        # location where the role was installed
         role_file = os.path.join(self.role_path, self.role_name)
-        self.assertTrue(os.path.exists(role_file))
 
         # removing role
-        gc = GalaxyCLI(args=["remove", "-c", "-p", self.role_path, self.role_name])
+        gc = GalaxyCLI(args=["remove", "-p", role_file, self.role_name])
-        galaxy_parser = gc.parse()
+        gc.parse()
-        super(GalaxyCLI, gc).run()
+        gc.run()
-        gc.api = ansible.galaxy.api.GalaxyAPI(gc.galaxy)
-        completed_task = gc.execute_remove()
 
         # testing role was removed
-        self.assertTrue(completed_task == 0)
+        removed_role = not os.path.exists(role_file)
-        self.assertTrue(not os.path.exists(role_file))
+        self.assertTrue(removed_role)
 
-    def test_exit_without_ignore(self):
+    def test_exit_without_ignore_without_flag(self):
-        ''' tests that GalaxyCLI exits with the error specified unless the --ignore-errors flag is used '''
+        ''' tests that GalaxyCLI exits with the error specified if the --ignore-errors flag is not used '''
-        gc = GalaxyCLI(args=["install", "--server=None", "-c", "fake_role_name"])
+        gc = GalaxyCLI(args=["install", "--server=None", "fake_role_name"])
-
+        gc.parse()
-        # testing without --ignore-errors flag
-        galaxy_parser = gc.parse()
         with patch.object(ansible.utils.display.Display, "display", return_value=None) as mocked_display:
             # testing that error expected is raised
             self.assertRaises(AnsibleError, gc.run)
             self.assertTrue(mocked_display.called_once_with("- downloading role 'fake_role_name', owned by "))
 
+    def test_exit_without_ignore_with_flag(self):
+        ''' tests that GalaxyCLI exits without the error specified if the --ignore-errors flag is used  '''
         # testing with --ignore-errors flag
-        gc = GalaxyCLI(args=["install", "--server=None", "-c", "fake_role_name", "--ignore-errors"])
+        gc = GalaxyCLI(args=["install", "--server=None", "fake_role_name", "--ignore-errors"])
-        galalxy_parser = gc.parse()
+        gc.parse()
         with patch.object(ansible.utils.display.Display, "display", return_value=None) as mocked_display:
-            # testing that error expected is not raised with --ignore-errors flag in use
             gc.run()
             self.assertTrue(mocked_display.called_once_with("- downloading role 'fake_role_name', owned by "))
 
@@ -173,94 +167,101 @@ class TestGalaxy(unittest.TestCase):
             # checking that the common results of parse() for all possible actions have been created/called
             self.assertIsInstance(galaxycli_obj.parser, ansible.cli.SortedOptParser)
             self.assertIsInstance(galaxycli_obj.galaxy, ansible.galaxy.Galaxy)
-            if action in ['import', 'delete']:
+            formatted_call = {
-                formatted_call = 'usage: %prog ' + action + ' [options] github_user github_repo'
+                'import'    : 'usage: %prog import [options] github_user github_repo',
-            elif action == 'info':
+                'delete'    : 'usage: %prog delete [options] github_user github_repo',
-                formatted_call = 'usage: %prog ' + action + ' [options] role_name[,version]'
+                'info'      : 'usage: %prog info [options] role_name[,version]',
-            elif action == 'init':
+                'init'      : 'usage: %prog init [options] role_name',
-                formatted_call = 'usage: %prog ' + action + ' [options] role_name'
+                'install'   : 'usage: %prog install [options] [-r FILE | role_name(s)[,version] | scm+role_repo_url[,version] | tar_file(s)]',
-            elif action == 'install':
+                'list'      : 'usage: %prog list [role_name]',
-                formatted_call = 'usage: %prog ' + action + ' [options] [-r FILE | role_name(s)[,version] | scm+role_repo_url[,version] | tar_file(s)]'
+                'login'     : 'usage: %prog login [options]',
-            elif action == 'list':
+                'remove'    : 'usage: %prog remove role1 role2 ...',
-                formatted_call = 'usage: %prog ' + action + ' [role_name]'
+                'search'    : 'usage: %prog search [searchterm1 searchterm2] [--galaxy-tags galaxy_tag1,galaxy_tag2] [--platforms platform1,platform2] [--author username]',
-            elif action == 'login':
+                'setup'     : 'usage: %prog setup [options] source github_user github_repo secret',
-                formatted_call = 'usage: %prog ' + action + ' [options]'
+            }
-            elif action == 'remove':
+
-                formatted_call = 'usage: %prog ' + action + ' role1 role2 ...'
+            first_call = 'usage: %prog [delete|import|info|init|install|list|login|remove|search|setup] [--help] [options] ...'
-            elif action == 'search':
+            second_call = formatted_call[action]
-                formatted_call = 'usage: %prog ' + action + ' [searchterm1 searchterm2] [--galaxy-tags galaxy_tag1,galaxy_tag2] [--platforms platform1,platform2] [--author username]'
+            calls = [call(first_call), call(second_call)]
-            elif action == 'setup':
-                formatted_call = 'usage: %prog ' + action + ' [options] source github_user github_repo secret'
-            calls = [call('usage: %prog [delete|import|info|init|install|list|login|remove|search|setup] [--help] [options] ...'), call(formatted_call)]
             mocked_usage.assert_has_calls(calls)
 
-    def test_parse(self):
+    def test_parse_no_action(self):
-        ''' systematically testing that the expected options parser is created '''
+        ''' testing the options parser when no action is given '''
-        # testing no action given
+        gc = GalaxyCLI(args=[""])
-        gc = GalaxyCLI(args=["-c"])
         self.assertRaises(AnsibleOptionsError, gc.parse)
 
-        # testing action that doesn't exist
+    def test_parse_invalid_action(self):
-        gc = GalaxyCLI(args=["NOT_ACTION", "-c"])
+        ''' testing the options parser when an invalid action is given '''
+        gc = GalaxyCLI(args=["NOT_ACTION"])
         self.assertRaises(AnsibleOptionsError, gc.parse)
 
-        # testing action 'delete'
+    def test_parse_delete(self):
-        gc = GalaxyCLI(args=["delete", "-c"])
+        ''' testing the options parser when the action 'delete' is given '''
+        gc = GalaxyCLI(args=["delete"])
         self.run_parse_common(gc, "delete")
         self.assertEqual(gc.options.verbosity, 0)
 
-        # testing action 'import'
+    def test_parse_import(self):
-        gc = GalaxyCLI(args=["import", "-c"])
+        ''' testing the options parser when the action 'import' is given '''
+        gc = GalaxyCLI(args=["import"])
         self.run_parse_common(gc, "import")
         self.assertEqual(gc.options.wait, True)
         self.assertEqual(gc.options.reference, None)
         self.assertEqual(gc.options.check_status, False)
         self.assertEqual(gc.options.verbosity, 0)
 
-        # testing action 'info'
+    def test_parse_info(self):
-        gc = GalaxyCLI(args=["info", "-c"])
+        ''' testing the options parser when the action 'info' is given '''
+        gc = GalaxyCLI(args=["info"])
         self.run_parse_common(gc, "info")
         self.assertEqual(gc.options.offline, False)
 
-        # testing action 'init'
+    def test_parse_init(self):
-        gc = GalaxyCLI(args=["init", "-c"])
+        ''' testing the options parser when the action 'init' is given '''
+        gc = GalaxyCLI(args=["init"])
         self.run_parse_common(gc, "init")
         self.assertEqual(gc.options.offline, False)
         self.assertEqual(gc.options.force, False)
 
-        # testing action 'install'
+    def test_parse_install(self):
-        gc = GalaxyCLI(args=["install", "-c"])
+        ''' testing the options parser when the action 'install' is given '''
+        gc = GalaxyCLI(args=["install"])
         self.run_parse_common(gc, "install")
         self.assertEqual(gc.options.ignore_errors, False)
         self.assertEqual(gc.options.no_deps, False)
         self.assertEqual(gc.options.role_file, None)
         self.assertEqual(gc.options.force, False)
 
-        # testing action 'list'
+    def test_parse_list(self):
-        gc = GalaxyCLI(args=["list", "-c"])
+        ''' testing the options parser when the action 'list' is given '''
+        gc = GalaxyCLI(args=["list"])
         self.run_parse_common(gc, "list")
         self.assertEqual(gc.options.verbosity, 0)
 
-        # testing action 'login'
+    def test_parse_login(self):
-        gc = GalaxyCLI(args=["login", "-c"])
+        ''' testing the options parser when the action 'login' is given '''
+        gc = GalaxyCLI(args=["login"])
         self.run_parse_common(gc, "login")
         self.assertEqual(gc.options.verbosity, 0)
         self.assertEqual(gc.options.token, None)
 
-        # testing action 'remove'
+    def test_parse_remove(self):
-        gc = GalaxyCLI(args=["remove", "-c"])
+        ''' testing the options parser when the action 'remove' is given '''
+        gc = GalaxyCLI(args=["remove"])
         self.run_parse_common(gc, "remove")
         self.assertEqual(gc.options.verbosity, 0)
 
-        # testing action 'search'
+    def test_parse_search(self):
-        gc = GalaxyCLI(args=["search", "-c"])
+        ''' testing the options parswer when the action 'search' is given '''
+        gc = GalaxyCLI(args=["search"])
         self.run_parse_common(gc, "search")
         self.assertEqual(gc.options.platforms, None)
         self.assertEqual(gc.options.galaxy_tags, None)
         self.assertEqual(gc.options.author, None)
 
-        # testing action 'setup'
+    def test_parse_setup(self):
-        gc = GalaxyCLI(args=["setup", "-c"])
+        ''' testing the options parser when the action 'setup' is given '''
+        gc = GalaxyCLI(args=["setup"])
         self.run_parse_common(gc, "setup")
         self.assertEqual(gc.options.verbosity, 0)
         self.assertEqual(gc.options.remove_id, None)