From 928e907f908b693796cd19b728a75db0d66cdd23 Mon Sep 17 00:00:00 2001 From: Nolan Brubaker Date: Tue, 19 Apr 2016 17:25:39 -0400 Subject: [PATCH] Refactor main inventory function for testability This change alters the dynamic_inventory.py file's main function with two main changes: * main now accepts arguments, which could be the ArgParse parsed arguments or a plain dictionary for testing * return output to then be printed, instead of just printing. With these changes, testing inventory can now be done via Python imports rather than a subprocess call. This make existing tests more clear, and allows for using pdb when creating future patches. With the subprocess method, a pdb in the dynamic_inventory.py would cause the test suite to hang, since the subprocess was waiting for input. Existing functionality is *not* changed - arguments and output are simply being moved to make the code easier to test. Change-Id: I3629c9d0704a7412832d7e898463d376b4e13317 --- playbooks/inventory/dynamic_inventory.py | 10 +++++----- tests/test_inventory.py | 17 ++++++----------- 2 files changed, 11 insertions(+), 16 deletions(-) diff --git a/playbooks/inventory/dynamic_inventory.py b/playbooks/inventory/dynamic_inventory.py index 21d3e009c8..cc12771285 100755 --- a/playbooks/inventory/dynamic_inventory.py +++ b/playbooks/inventory/dynamic_inventory.py @@ -926,9 +926,8 @@ def load_user_configuration(config_path): return user_defined_config -def main(): +def main(all_args): """Run the main application.""" - all_args = args() # Get the path to the user configuration files config_path = find_config_path( user_config_path=all_args.get('config') @@ -1045,8 +1044,9 @@ def main(): with open(dynamic_inventory_file, 'wb') as f: f.write(dynamic_inventory_json) - # Print out our inventory - print(dynamic_inventory_json) + return dynamic_inventory_json if __name__ == '__main__': - main() + all_args = args() + output = main(all_args) + print(output) diff --git a/tests/test_inventory.py b/tests/test_inventory.py index feedb0dc27..0861d07bcb 100644 --- a/tests/test_inventory.py +++ b/tests/test_inventory.py @@ -4,7 +4,6 @@ import collections import json import os from os import path -import subprocess import sys import unittest import yaml @@ -39,11 +38,7 @@ def cleanup(): def get_inventory(): "Return the inventory mapping in a dict." try: - cmd = [INV_SCRIPT, '--config', TARGET_DIR] - inventory_string = subprocess.check_output( - cmd, - stderr=subprocess.STDOUT - ) + inventory_string = di.main({'config': TARGET_DIR}) inventory = json.loads(inventory_string) return inventory finally: @@ -324,19 +319,19 @@ class TestConfigChecks(unittest.TestCase): # create config file without provider networks self.setup_config_file(self.user_defined_config, 'provider_networks') # check if provider networks absence is Caught - with self.assertRaises(subprocess.CalledProcessError) as context: + with self.assertRaises(SystemExit) as context: get_inventory() expectedLog = "provider networks can't be found under global_overrides" - self.assertTrue(expectedLog in context.exception.output) + self.assertTrue(expectedLog in context.exception.message) def test_global_overrides_check(self): # create config file without global_overrides self.setup_config_file(self.user_defined_config, 'global_overrides') # check if global_overrides absence is Caught - with self.assertRaises(subprocess.CalledProcessError) as context: + with self.assertRaises(SystemExit) as context: get_inventory() - expectedLog = "global_overrides can't be found in user config\n" - self.assertEqual(context.exception.output, expectedLog) + expectedLog = "global_overrides can't be found in user config" + self.assertEqual(context.exception.message, expectedLog) def tearDown(self): # get back our initial user config file