Some fitzing with imports and docstrings to make the linters happy.

Copybara generated commit for Python Fire.


PiperOrigin-RevId: 150942368
Change-Id: I7ba73d5a9845d642f99ffeba6c953d8116830a33
Reviewed-on: https://team-review.git.corp.google.com/65058
Reviewed-by: David Bieber <dbieber@google.com>

fire/__init__.py

@@ -12,7 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-"""Python Fire module for third_party."""
+"""The Python Fire module."""
 
 from __future__ import absolute_import
 from __future__ import division

fire/core.py

@@ -70,7 +70,7 @@ import six
 
 
 def Fire(component=None, command=None, name=None):
-  """This function, Fire, is the main entrypoint for Fire.
+  """This function, Fire, is the main entrypoint for Python Fire.
 
   Executes a command either from the `command` argument or from sys.argv by
   recursively traversing the target object `component`'s members consuming
@@ -92,9 +92,10 @@ def Fire(component=None, command=None, name=None):
     it's a class). When all arguments are consumed and there's no function left
     to call or class left to instantiate, the resulting current component is
     the final result.
-    If a Fire error is encountered, the Fire Trace is displayed to stdout and
-    None is returned.
-    If the trace command line argument is supplied, the FireTrace is returned.
+  Raises:
+    FireExit: When Fire encounters a FireError, Fire will raise a FireExit with
+        code 2. When used with the help or trace flags, Fire will raise a
+        FireExit with code 0 if successful.
   """
   # Get args as a list.
   if command is None:
@@ -127,21 +128,21 @@ def Fire(component=None, command=None, name=None):
     result = component_trace.GetResult()
     print(
         helputils.HelpString(result, component_trace, component_trace.verbose))
-    return None
+    raise FireExit(2, component_trace)
   elif component_trace.show_trace and component_trace.show_help:
     print('Fire trace:\n{trace}\n'.format(trace=component_trace))
     result = component_trace.GetResult()
     print(
         helputils.HelpString(result, component_trace, component_trace.verbose))
-    return component_trace
+    raise FireExit(0, component_trace)
   elif component_trace.show_trace:
     print('Fire trace:\n{trace}'.format(trace=component_trace))
-    return component_trace
+    raise FireExit(0, component_trace)
   elif component_trace.show_help:
     result = component_trace.GetResult()
     print(
         helputils.HelpString(result, component_trace, component_trace.verbose))
-    return None
+    raise FireExit(0, component_trace)
   else:
     _PrintResult(component_trace, verbose=component_trace.verbose)
     result = component_trace.GetResult()
@@ -161,6 +162,27 @@ class FireError(Exception):
   """
 
 
+class FireExit(SystemExit):
+  """An exception raised by Fire to the client in the case of a FireError.
+
+  The trace of the Fire program is available on the `trace` property.
+
+  This exception inherits from SystemExit, so clients may explicitly catch it
+  with `except SystemExit` or `except FireExit`. If not caught, this exception
+  will cause the client program to exit without a stacktrace.
+  """
+
+  def __init__(self, code, component_trace):
+    """Constructs a FireExit exception.
+
+    Args:
+      code: (int) Exit code for the Fire CLI.
+      component_trace: (FireTrace) The trace for the Fire command.
+    """
+    super(FireExit, self).__init__(code)
+    self.trace = component_trace
+
+
 def _PrintResult(component_trace, verbose=False):
   """Prints the result of the Fire call to stdout in a human readable way."""
   # TODO: Design human readable deserializable serialization method
@@ -558,8 +580,8 @@ def _MakeParseFn(fn):
 
     # Note: _ParseArgs modifies kwargs.
     parsed_args, kwargs, remaining_args, capacity = _ParseArgs(
-        fn_spec.args, fn_spec.defaults, num_required_args, kwargs, remaining_args,
-        metadata)
+        fn_spec.args, fn_spec.defaults, num_required_args, kwargs,
+        remaining_args, metadata)
 
     if fn_spec.varargs or fn_spec.varkw:
       # If we're allowed *varargs or **kwargs, there's always capacity.

fire/core_test.py

@@ -18,13 +18,14 @@ from __future__ import print_function
 
 from fire import core
 from fire import test_components as tc
+from fire import testutils
 from fire import trace
 import mock
 
 import unittest
 
 
-class CoreTest(unittest.TestCase):
+class CoreTest(testutils.BaseTestCase):
 
   def testOneLineResult(self):
     self.assertEqual(core._OneLineResult(1), '1')
@@ -66,11 +67,21 @@ class CoreTest(unittest.TestCase):
     self.assertIsInstance(variables['trace'], trace.FireTrace)
 
   def testImproperUseOfHelp(self):
-    # This should produce a warning and return None.
-    self.assertIsNone(core.Fire(tc.TypedProperties, 'alpha --help'))
+    # This should produce a warning explaining the proper use of help.
+    with self.assertRaisesFireExit(2, 'The proper way to show help.*Usage:'):
+      core.Fire(tc.TypedProperties, 'alpha --help')
+
+  def testProperUseOfHelp(self):
+    with self.assertRaisesFireExit(0, 'Usage:.*upper'):
+      core.Fire(tc.TypedProperties, 'gamma -- --help')
+
+  def testInvalidParameterRaisesFireExit(self):
+    with self.assertRaisesFireExit(2, 'runmisspelled'):
+      core.Fire(tc.Kwargs, 'props --a=1 --b=2 runmisspelled')
 
   def testErrorRaising(self):
     # Errors in user code should not be caught; they should surface as normal.
+    # This will lead to exit status code 1 for the client program.
     with self.assertRaises(ValueError):
       core.Fire(tc.ErrorRaiser, 'fail')
 

fire/fire_import_test.py

@@ -12,16 +12,19 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import fire
-
+import mock
+import sys
 import unittest
 
+import fire
+
 
 class FireImportTest(unittest.TestCase):
   """Tests importing Fire."""
 
   def testFire(self):
-    fire.Fire()
+    with mock.patch.object(sys, 'argv', ['commandname']):
+      fire.Fire()
 
   def testFireMethods(self):
     self.assertIsNotNone(fire.Fire)

fire/fire_test.py

@@ -16,20 +16,24 @@ from __future__ import absolute_import
 from __future__ import division
 from __future__ import print_function
 
-import fire
-from fire import test_components as tc
-from fire import trace
-
-import six
+import sys
 import unittest
 
+import fire
+from fire import test_components as tc
+from fire import testutils
 
-class FireTest(unittest.TestCase):
+import mock
+import six
+
+
+class FireTest(testutils.BaseTestCase):
 
   def testFire(self):
-    fire.Fire(tc.Empty)
-    fire.Fire(tc.OldStyleEmpty)
-    fire.Fire(tc.WithInit)
+    with mock.patch.object(sys, 'argv', ['progname']):
+      fire.Fire(tc.Empty)
+      fire.Fire(tc.OldStyleEmpty)
+      fire.Fire(tc.WithInit)
     self.assertEqual(fire.Fire(tc.NoDefaults, 'double 2'), 4)
     self.assertEqual(fire.Fire(tc.NoDefaults, 'triple 4'), 12)
     self.assertEqual(fire.Fire(tc.WithDefaults, 'double 2'), 4)
@@ -41,10 +45,13 @@ class FireTest(unittest.TestCase):
     self.assertEqual(fire.Fire(tc.MixedDefaults, 'ten'), 10)
 
   def testFireExceptions(self):
-    # Exceptions of Fire are printed to stderr and None is returned.
-    self.assertIsNone(fire.Fire(tc.Empty, 'nomethod'))  # Member doesn't exist.
-    self.assertIsNone(fire.Fire(tc.NoDefaults, 'double'))  # Missing argument.
-    self.assertIsNone(fire.Fire(tc.TypedProperties, 'delta x'))  # Missing key.
+    # Exceptions of Fire are printed to stderr and a FireExit is raised.
+    with self.assertRaisesFireExit(2):
+      fire.Fire(tc.Empty, 'nomethod')  # Member doesn't exist.
+    with self.assertRaisesFireExit(2):
+      fire.Fire(tc.NoDefaults, 'double')  # Missing argument.
+    with self.assertRaisesFireExit(2):
+      fire.Fire(tc.TypedProperties, 'delta x')  # Missing key.
 
     # Exceptions of the target components are still raised.
     with self.assertRaises(ZeroDivisionError):
@@ -89,11 +96,13 @@ class FireTest(unittest.TestCase):
         fire.Fire(tc.MixedDefaults, 'identity --beta 1 --alpha 2'), (2, 1))
 
   def testFirePartialNamedArgsOneMissing(self):
-    # By default, errors are written to standard out and None is returned.
-    self.assertIsNone(  # Identity needs an arg.
-        fire.Fire(tc.MixedDefaults, 'identity'))
-    self.assertIsNone(  # Identity needs a value for alpha.
-        fire.Fire(tc.MixedDefaults, 'identity --beta 2'))
+    # Errors are written to standard out and a FireExit is raised.
+    with self.assertRaisesFireExit(2):
+      fire.Fire(tc.MixedDefaults, 'identity')  # Identity needs an arg.
+
+    with self.assertRaisesFireExit(2):
+      # Identity needs a value for alpha.
+      fire.Fire(tc.MixedDefaults, 'identity --beta 2')
 
     self.assertEqual(fire.Fire(tc.MixedDefaults, 'identity 1'), (1, '0'))
     self.assertEqual(
@@ -103,9 +112,11 @@ class FireTest(unittest.TestCase):
     self.assertEqual(fire.Fire(tc.Annotations, 'double 5'), 10)
     self.assertEqual(fire.Fire(tc.Annotations, 'triple 5'), 15)
 
-  @unittest.skipIf(six.PY2, 'Keyword-only arguments not supported in Python 2')
+  @unittest.skipIf(six.PY2, 'Keyword-only arguments not in Python 2.')
   def testFireKeywordOnlyArgs(self):
-    self.assertIsNone(fire.Fire(tc.py3.KeywordOnly, 'double 5'))
+    with self.assertRaisesFireExit(2):
+      # Keyword arguments must be passed with flag syntax.
+      fire.Fire(tc.py3.KeywordOnly, 'double 5')
 
     self.assertEqual(fire.Fire(tc.py3.KeywordOnly, 'double --count 5'), 10)
     self.assertEqual(fire.Fire(tc.py3.KeywordOnly, 'triple --count 5'), 15)
@@ -252,17 +263,19 @@ class FireTest(unittest.TestCase):
     self.assertEqual(fire.Fire(fn1, '--thing --nothing'), (True, True))
     self.assertEqual(fire.Fire(fn1, '--thing --nonothing'), (True, False))
 
-    # In the next example nothing=False (since rightmost setting of a flag gets
-    # precedence), but it errors because thing has no value.
-    self.assertEqual(fire.Fire(fn1, '--nothing --nonothing'), None)
+    with self.assertRaisesFireExit(2):
+      # In this case nothing=False (since rightmost setting of a flag gets
+      # precedence), but it errors because thing has no value.
+      fire.Fire(fn1, '--nothing --nonothing')
 
     # In these examples, --nothing sets thing=False:
     def fn2(thing, **kwargs):
       return thing, kwargs
     self.assertEqual(fire.Fire(fn2, '--thing'), (True, {}))
     self.assertEqual(fire.Fire(fn2, '--nothing'), (False, {}))
-    # In the next one, nothing=True, but it errors because thing has no value.
-    self.assertEqual(fire.Fire(fn2, '--nothing=True'), None)
+    with self.assertRaisesFireExit(2):
+      # In this case, nothing=True, but it errors because thing has no value.
+      fire.Fire(fn2, '--nothing=True')
     self.assertEqual(fire.Fire(fn2, '--nothing --nothing=True'),
                      (False, {'nothing': True}))
 
@@ -276,26 +289,28 @@ class FireTest(unittest.TestCase):
                      ('value', {'nothing': False}))
 
   def testTraceFlag(self):
-    self.assertIsInstance(
-        fire.Fire(tc.BoolConverter, 'as-bool True -- --trace'), trace.FireTrace)
-    self.assertIsInstance(
-        fire.Fire(tc.BoolConverter, 'as-bool True -- -t'), trace.FireTrace)
-    self.assertIsInstance(
-        fire.Fire(tc.BoolConverter, '-- --trace'), trace.FireTrace)
+    with self.assertRaisesFireExit(0, 'Fire trace:\n'):
+      fire.Fire(tc.BoolConverter, 'as-bool True -- --trace')
+    with self.assertRaisesFireExit(0, 'Fire trace:\n'):
+      fire.Fire(tc.BoolConverter, 'as-bool True -- -t')
+    with self.assertRaisesFireExit(0, 'Fire trace:\n'):
+      fire.Fire(tc.BoolConverter, '-- --trace')
 
   def testHelpFlag(self):
-    self.assertIsNone(fire.Fire(tc.BoolConverter, 'as-bool True -- --help'))
-    self.assertIsNone(fire.Fire(tc.BoolConverter, 'as-bool True -- -h'))
-    self.assertIsNone(fire.Fire(tc.BoolConverter, '-- --help'))
+    with self.assertRaisesFireExit(0):
+      fire.Fire(tc.BoolConverter, 'as-bool True -- --help')
+    with self.assertRaisesFireExit(0):
+      fire.Fire(tc.BoolConverter, 'as-bool True -- -h')
+    with self.assertRaisesFireExit(0):
+      fire.Fire(tc.BoolConverter, '-- --help')
 
   def testHelpFlagAndTraceFlag(self):
-    self.assertIsInstance(
-        fire.Fire(tc.BoolConverter, 'as-bool True -- --help --trace'),
-        trace.FireTrace)
-    self.assertIsInstance(
-        fire.Fire(tc.BoolConverter, 'as-bool True -- -h -t'), trace.FireTrace)
-    self.assertIsInstance(
-        fire.Fire(tc.BoolConverter, '-- -h --trace'), trace.FireTrace)
+    with self.assertRaisesFireExit(0, 'Fire trace:\n.*Usage:'):
+      fire.Fire(tc.BoolConverter, 'as-bool True -- --help --trace')
+    with self.assertRaisesFireExit(0, 'Fire trace:\n.*Usage:'):
+      fire.Fire(tc.BoolConverter, 'as-bool True -- -h -t')
+    with self.assertRaisesFireExit(0, 'Fire trace:\n.*Usage:'):
+      fire.Fire(tc.BoolConverter, '-- -h --trace')
 
   def testTabCompletionNoName(self):
     with self.assertRaises(ValueError):
@@ -323,7 +338,8 @@ class FireTest(unittest.TestCase):
         ('-', '_'))
 
     # The separator triggers a function call, but there aren't enough arguments.
-    self.assertEqual(fire.Fire(tc.MixedDefaults, 'identity - _ +'), None)
+    with self.assertRaisesFireExit(2):
+      fire.Fire(tc.MixedDefaults, 'identity - _ +')
 
   def testNonComparable(self):
     """Fire should work with classes that disallow comparisons."""
@@ -367,24 +383,34 @@ class FireTest(unittest.TestCase):
   def testClassInstantiation(self):
     self.assertIsInstance(fire.Fire(tc.InstanceVars, '--arg1=a1 --arg2=a2'),
                           tc.InstanceVars)
-    # Cannot instantiate a class with positional args by default.
-    self.assertIsNone(fire.Fire(tc.InstanceVars, 'a1 a2'))
+    with self.assertRaisesFireExit(2):
+      # Cannot instantiate a class with positional args.
+      fire.Fire(tc.InstanceVars, 'a1 a2')
 
   def testTraceErrors(self):
     # Class needs additional value but runs out of args.
-    self.assertIsNone(fire.Fire(tc.InstanceVars, 'a1'))
-    self.assertIsNone(fire.Fire(tc.InstanceVars, '--arg1=a1'))
+    with self.assertRaisesFireExit(2):
+      fire.Fire(tc.InstanceVars, 'a1')
+    with self.assertRaisesFireExit(2):
+      fire.Fire(tc.InstanceVars, '--arg1=a1')
+
     # Routine needs additional value but runs out of args.
-    self.assertIsNone(fire.Fire(tc.InstanceVars, 'a1 a2 - run b1'))
-    self.assertIsNone(
-        fire.Fire(tc.InstanceVars, '--arg1=a1 --arg2=a2 - run b1'))
+    with self.assertRaisesFireExit(2):
+      fire.Fire(tc.InstanceVars, 'a1 a2 - run b1')
+    with self.assertRaisesFireExit(2):
+      fire.Fire(tc.InstanceVars, '--arg1=a1 --arg2=a2 - run b1')
+
     # Extra args cannot be consumed.
-    self.assertIsNone(fire.Fire(tc.InstanceVars, 'a1 a2 - run b1 b2 b3'))
-    self.assertIsNone(
-        fire.Fire(tc.InstanceVars, '--arg1=a1 --arg2=a2 - run b1 b2 b3'))
+    with self.assertRaisesFireExit(2):
+      fire.Fire(tc.InstanceVars, 'a1 a2 - run b1 b2 b3')
+    with self.assertRaisesFireExit(2):
+      fire.Fire(tc.InstanceVars, '--arg1=a1 --arg2=a2 - run b1 b2 b3')
+
     # Cannot find member to access.
-    self.assertIsNone(fire.Fire(tc.InstanceVars, 'a1 a2 - jog'))
-    self.assertIsNone(fire.Fire(tc.InstanceVars, '--arg1=a1 --arg2=a2 - jog'))
+    with self.assertRaisesFireExit(2):
+      fire.Fire(tc.InstanceVars, 'a1 a2 - jog')
+    with self.assertRaisesFireExit(2):
+      fire.Fire(tc.InstanceVars, '--arg1=a1 --arg2=a2 - jog')
 
 
 if __name__ == '__main__':

fire/inspectutils_test.py

@@ -17,11 +17,12 @@ from __future__ import division
 from __future__ import print_function
 
 import unittest
-import six
 
 from fire import inspectutils
 from fire import test_components as tc
 
+import six
+
 
 class InspectUtilsTest(unittest.TestCase):
 

fire/test_components.py

@@ -110,6 +110,7 @@ class TypedProperties(object):
     }
     self.echo = ['alex', 'bethany']
     self.fox = ('carry', 'divide')
+    self.gamma = 'myexcitingstring'
 
 
 class VarArgs(object):

fire/testutils.py

@@ -0,0 +1,64 @@
+# Copyright (C) 2017 Google Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+from __future__ import absolute_import
+from __future__ import division
+from __future__ import print_function
+
+import contextlib
+import re
+import sys
+import unittest
+
+from fire import core
+from fire import trace
+
+import mock
+import six
+
+
+class BaseTestCase(unittest.TestCase):
+  """Shared test case for Python Fire tests."""
+
+  @contextlib.contextmanager
+  def assertRaisesFireExit(self, code, regexp=None):
+    """Asserts that a FireExit error is raised in the context.
+
+    Allows tests to check that Fire's wrapper around SystemExit is raised
+    and that a regexp is matched in the output.
+
+    Args:
+      code: The status code that the FireExit should contain.
+      regexp: stdout must match this regex.
+    Yields:
+      Yields to the wrapped context.
+    """
+    if regexp is None:
+      regexp = '.*'
+    with self.assertRaises(core.FireExit):
+      stdout = six.StringIO()
+      with mock.patch.object(sys, 'stdout', stdout):
+        try:
+          yield
+        except core.FireExit as exc:
+          if exc.code != code:
+            raise AssertionError('Incorrect exit code: %r != %r' % (exc.code,
+                                                                    code))
+          self.assertIsInstance(exc.trace, trace.FireTrace)
+          stdout.flush()
+          stdout.seek(0)
+          value = stdout.getvalue()
+          if not re.search(regexp, value, re.DOTALL | re.MULTILINE):
+            raise AssertionError('Expected %r to match %r' % (value, regexp))
+          raise