Edit Config.get_required so as to raise error for any blank options.

As it stood, get_required would not raise a ConfigError for strings
(the default) if the option was left blank, unlike other data types.
A str option that was left blank was parsed as '' and any error
catching was left to the tooling. This adds an explicit check for
times where get_required() could return an empty string instead of the
expected None.

If a blank option is a viable choice, I think get(...default="") is
a better choice for those situations. I added a set of get_required
tests as well.

Testing Done:
PANTS_DEV=1 ./pants test tests/python/pants_test:all : Success
Travis Passed: https://travis-ci.org/pantsbuild/pants/builds/47998589

Reviewed at https://rbcommons.com/s/twitter/r/1638/

src/python/pants/base/config.py

@@ -181,7 +181,8 @@ def get_required(self, section, option, type=str):
     :raises: :class:`pants.base.config.Config.ConfigError` if option is not found.
     """
     val = self.get(section, option, type=type)
-    if val is None:
+    # Empty str catches blank options. If blank entries are ok, use get(..., default='') instead.
+    if val is None or val == '':
       raise Config.ConfigError('Required option %s.%s is not defined.' % (section, option))
     return val
 

tests/python/pants_test/tasks/test_config.py

@@ -28,6 +28,7 @@ def setUp(self):
         disclaimer:
           Let it be known
           that.
+        blank_section:
 
         [a]
         list: [1, 2, 3, %(answer)s]
@@ -90,6 +91,16 @@ def test_getmap(self):
     self._check_defaults(self.config.get, {})
     self._check_defaults(self.config.get, dict(a=42))
 
+  def test_get_required(self):
+    self.assertEquals('foo', self.config.get_required('a', 'name'))
+    self.assertEquals(42, self.config.get_required('a', 'answer', type=int))
+    with self.assertRaises(Config.ConfigError):
+      self.config.get_required('a', 'answer', type=dict)
+    with self.assertRaises(Config.ConfigError):
+      self.config.get_required('a', 'no_section')
+    with self.assertRaises(Config.ConfigError):
+      self.config.get_required('a', 'blank_section')
+
   def _check_defaults(self, accessor, default):
     self.assertEquals(None, accessor('c', 'fast'))
     self.assertEquals(None, accessor('c', 'preempt', None))