Cleanup warnings

Zuul has taken to including warnings in the locations that they're
raised in. We have a few of these in recent jobs so go ahead and clean
them up.

Change-Id: Ifcce20159d872ffd1447ca10f126ae2f2162f956
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>

5 files changed, 46 insertions, 17 deletions

diff --git a/oslo_policy/generator.py b/oslo_policy/generator.py
index fd9f159..48a9972 100644
--- a/oslo_policy/generator.py
+++ b/oslo_policy/generator.py
@@ -277,6 +277,9 @@ def _generate_sample(namespaces, output_file=None, output_format='yaml',
             ',\n    '.join(sections_text),
             '\n}\n'))
 
+    if output_file != sys.stdout:
+        output_file.close()
+
 
 def _generate_policy(namespace, output_file=None):
     """Generate a policy file showing what will be used.
@@ -304,6 +307,9 @@ def _generate_policy(namespace, output_file=None):
     for section in _sort_and_format_by_section(policies, include_help=False):
         output_file.write(section)
 
+    if output_file != sys.stdout:
+        output_file.close()
+
 
 def _list_redundant(namespace):
     """Generate a list of configured policies which match defaults.
@@ -396,12 +402,11 @@ def upgrade_policy(args=None, conf=None):
     _upgrade_policies(policies, default_policies)
 
     if conf.output_file:
-        if conf.format == 'yaml':
-            yaml.safe_dump(policies, open(conf.output_file, 'w'),
-                           default_flow_style=False)
-        elif conf.format == 'json':
-            jsonutils.dump(policies, open(conf.output_file, 'w'),
-                           indent=4)
+        with open(conf.output_file, 'w') as fh:
+            if conf.format == 'yaml':
+                yaml.safe_dump(policies, fh, default_flow_style=False)
+            elif conf.format == 'json':
+                jsonutils.dump(policies, fh, indent=4)
     else:
         if conf.format == 'yaml':
             sys.stdout.write(yaml.safe_dump(policies,
diff --git a/oslo_policy/tests/test_generator.py b/oslo_policy/tests/test_generator.py
index 33d420e..86003fe 100644
--- a/oslo_policy/tests/test_generator.py
+++ b/oslo_policy/tests/test_generator.py
@@ -674,7 +674,8 @@ class UpgradePolicyTestCase(base.PolicyBaseTestCase):
             with mock.patch('sys.argv', testargs):
                 generator.upgrade_policy(conf=self.local_conf)
                 new_file = self.get_config_file_fullname('new_policy.json')
-                new_policy = jsonutils.loads(open(new_file, 'r').read())
+                with open(new_file, 'r') as fh:
+                    new_policy = jsonutils.loads(fh.read())
                 self.assertIsNotNone(new_policy.get('new_policy_name'))
                 self.assertIsNone(new_policy.get('deprecated_name'))
 
@@ -693,7 +694,8 @@ class UpgradePolicyTestCase(base.PolicyBaseTestCase):
             with mock.patch('sys.argv', testargs):
                 generator.upgrade_policy(conf=self.local_conf)
                 new_file = self.get_config_file_fullname('new_policy.yaml')
-                new_policy = yaml.safe_load(open(new_file, 'r'))
+                with open(new_file, 'r') as fh:
+                    new_policy = yaml.safe_load(fh)
                 self.assertIsNotNone(new_policy.get('new_policy_name'))
                 self.assertIsNone(new_policy.get('deprecated_name'))
 
diff --git a/oslo_policy/tests/test_parser.py b/oslo_policy/tests/test_parser.py
index edfd475..f1ac36b 100644
--- a/oslo_policy/tests/test_parser.py
+++ b/oslo_policy/tests/test_parser.py
@@ -34,16 +34,20 @@ class ParseCheckTestCase(test_base.BaseTestCase):
 
         self.assertIsInstance(result, _checks.TrueCheck)
 
-    def test_bad_rule(self):
+    @mock.patch.object(_parser, 'LOG')
+    def test_bad_rule(self, mock_log):
         result = _parser._parse_check('foobar')
 
         self.assertIsInstance(result, _checks.FalseCheck)
+        mock_log.exception.assert_called_once()
 
     @mock.patch.object(_checks, 'registered_checks', {})
-    def test_no_handler(self):
+    @mock.patch.object(_parser, 'LOG')
+    def test_no_handler(self, mock_log):
         result = _parser._parse_check('no:handler')
 
         self.assertIsInstance(result, _checks.FalseCheck)
+        mock_log.error.assert_called()
 
     @mock.patch.object(_checks, 'registered_checks', {
         'spam': mock.Mock(return_value='spam_check'),
@@ -375,6 +379,7 @@ class ParseTextRuleTestCase(test_base.BaseTestCase):
         mock_shift.assert_has_calls(
             [mock.call('tok1', 'val1'), mock.call('tok2', 'val2')])
 
+    @mock.patch.object(_parser, 'LOG', new=mock.Mock())
     @mock.patch.object(_parser, '_parse_tokenize', return_value=[])
     def test_fail(self, mock_parse_tokenize):
         result = _parser._parse_text_rule('test rule')
diff --git a/oslo_policy/tests/test_policy.py b/oslo_policy/tests/test_policy.py
index f3f75b0..0b0164e 100644
--- a/oslo_policy/tests/test_policy.py
+++ b/oslo_policy/tests/test_policy.py
@@ -797,6 +797,7 @@ class EnforcerTest(base.PolicyBaseTestCase):
         for k, v in expected_creds.items():
             self.assertEqual(expected_creds[k], creds[k])
 
+    @mock.patch('warnings.warn', new=mock.Mock())
     def test_map_context_attributes_populated_system(self):
         request_context = context.RequestContext(system_scope='all')
         expected_creds = request_context.to_policy_values()
@@ -1161,6 +1162,7 @@ class RuleDefaultTestCase(base.PolicyBaseTestCase):
 
 class DocumentedRuleDefaultDeprecationTestCase(base.PolicyBaseTestCase):
 
+    @mock.patch('warnings.warn', new=mock.Mock())
     def test_deprecate_a_policy_check_string(self):
         deprecated_rule = policy.DeprecatedRule(
             name='foo:create_bar',
@@ -1200,6 +1202,7 @@ class DocumentedRuleDefaultDeprecationTestCase(base.PolicyBaseTestCase):
             enforcer.enforce('foo:create_bar', {}, {'roles': ['baz']})
         )
 
+    @mock.patch('warnings.warn', new=mock.Mock())
     def test_deprecate_an_empty_policy_check_string(self):
         deprecated_rule = policy.DeprecatedRule(
             name='foo:create_bar',
@@ -1227,6 +1230,7 @@ class DocumentedRuleDefaultDeprecationTestCase(base.PolicyBaseTestCase):
         enforcer.enforce('foo:create_bar', {}, {'roles': ['fizz']},
                          do_raise=True)
 
+    @mock.patch('warnings.warn', new=mock.Mock())
     def test_deprecate_replace_with_empty_policy_check_string(self):
         deprecated_rule = policy.DeprecatedRule(
             name='foo:create_bar',
@@ -1468,6 +1472,7 @@ class DocumentedRuleDefaultDeprecationTestCase(base.PolicyBaseTestCase):
             deprecated_since='N'
         )
 
+    @mock.patch('warnings.warn', new=mock.Mock())
     def test_override_deprecated_policy_with_old_name(self):
         # Simulate an operator overriding a policy
         rules = jsonutils.dumps({'foo:bar': 'role:bazz'})
@@ -1536,6 +1541,7 @@ class DocumentedRuleDefaultDeprecationTestCase(base.PolicyBaseTestCase):
             self.enforcer.enforce('foo:create_bar', {}, {'roles': ['bazz']})
         )
 
+    @mock.patch('warnings.warn', new=mock.Mock())
     def test_override_both_new_and_old_policy(self):
         # Simulate an operator overriding a policy using both the the new and
         # old policy names. The following doesn't make a whole lot of sense
@@ -1586,6 +1592,7 @@ class DocumentedRuleDefaultDeprecationTestCase(base.PolicyBaseTestCase):
             self.enforcer.enforce('foo:create_bar', {}, {'roles': ['bazz']})
         )
 
+    @mock.patch('warnings.warn', new=mock.Mock())
     def test_override_deprecated_policy_with_new_rule(self):
         # Simulate an operator overriding a deprecated policy with a reference
         # to the new policy, as done by the sample policy generator.
@@ -1712,37 +1719,46 @@ class EnforcerCheckRulesTest(base.PolicyBaseTestCase):
         self.enforcer.load_rules(True)
         self.assertTrue(self.enforcer.check_rules(raise_on_violation=True))
 
-    def test_undefined_rule(self):
+    @mock.patch.object(policy, 'LOG')
+    def test_undefined_rule(self, mock_log):
         rules = jsonutils.dumps({'foo': 'rule:bar'})
         self.create_config_file('policy.json', rules)
         self.enforcer.load_rules(True)
 
         self.assertFalse(self.enforcer.check_rules())
+        mock_log.warning.assert_called()
 
-    def test_undefined_rule_raises(self):
+    @mock.patch.object(policy, 'LOG')
+    def test_undefined_rule_raises(self, mock_log):
         rules = jsonutils.dumps({'foo': 'rule:bar'})
         self.create_config_file('policy.json', rules)
         self.enforcer.load_rules(True)
 
         self.assertRaises(policy.InvalidDefinitionError,
                           self.enforcer.check_rules, raise_on_violation=True)
+        mock_log.warning.assert_called()
 
-    def test_cyclical_rules(self):
+    @mock.patch.object(policy, 'LOG')
+    def test_cyclical_rules(self, mock_log):
         rules = jsonutils.dumps({'foo': 'rule:bar', 'bar': 'rule:foo'})
         self.create_config_file('policy.json', rules)
         self.enforcer.load_rules(True)
 
         self.assertFalse(self.enforcer.check_rules())
+        mock_log.warning.assert_called()
 
-    def test_cyclical_rules_raises(self):
+    @mock.patch.object(policy, 'LOG')
+    def test_cyclical_rules_raises(self, mock_log):
         rules = jsonutils.dumps({'foo': 'rule:bar', 'bar': 'rule:foo'})
         self.create_config_file('policy.json', rules)
         self.enforcer.load_rules(True)
 
         self.assertRaises(policy.InvalidDefinitionError,
                           self.enforcer.check_rules, raise_on_violation=True)
+        mock_log.warning.assert_called()
 
-    def test_complex_cyclical_rules_false(self):
+    @mock.patch.object(policy, 'LOG')
+    def test_complex_cyclical_rules_false(self, mock_log):
         rules = jsonutils.dumps({'foo': 'rule:bar',
                                  'bar': 'rule:baz and role:admin',
                                  'baz': 'rule:foo or role:user'})
@@ -1750,6 +1766,7 @@ class EnforcerCheckRulesTest(base.PolicyBaseTestCase):
         self.enforcer.load_rules(True)
 
         self.assertFalse(self.enforcer.check_rules())
+        mock_log.warning.assert_called()
 
     def test_complex_cyclical_rules_true(self):
         rules = jsonutils.dumps({'foo': 'rule:bar or rule:baz',
diff --git a/oslo_policy/tests/test_shell.py b/oslo_policy/tests/test_shell.py
index 0eb6aa6..b27347c 100644
--- a/oslo_policy/tests/test_shell.py
+++ b/oslo_policy/tests/test_shell.py
@@ -229,7 +229,7 @@ passed: sampleservice:sample_rule2
         self.create_config_file(
             "target.json",
             jsonutils.dumps(target))
-        target_file = open(self.get_config_file_fullname('target.json'), 'r')
-        target_from_file = target_file.read()
+        with open(self.get_config_file_fullname('target.json'), 'r') as fh:
+            target_from_file = fh.read()
         result = shell.flatten(jsonutils.loads(target_from_file))
         self.assertEqual(result, {"target.secret.project_id": "1234"})