Fix Options.Update().

4 files changed, 127 insertions, 17 deletions

diff --git a/src/CHANGES.txt b/src/CHANGES.txt
index dd976ef..b04942e 100644
--- a/src/CHANGES.txt
+++ b/src/CHANGES.txt
@@ -31,6 +31,9 @@ RELEASE 0.15 - XXX
   - Eliminate a dependency on the distutils.fancy_getopt module by
     copying and pasting its wrap_text() function directly.
 
+  - Make the Script.Options() subclass match the underlying base class
+    implementation.
+
   From Steve Leblanc:
 
   - Don't update the .sconsign files when run with -n.
diff --git a/src/engine/SCons/Options.py b/src/engine/SCons/Options.py
index 9d315b8..953f1a4 100644
--- a/src/engine/SCons/Options.py
+++ b/src/engine/SCons/Options.py
@@ -39,7 +39,7 @@ class Options:
     Holds all the options, updates the environment with the variables,
     and renders the help text.
     """
-    def __init__(self, files=None):
+    def __init__(self, files=None, args={}):
         """
         files - [optional] List of option configuration files to load
             (backward compatibility) If a single string is passed it is 
@@ -47,6 +47,7 @@ class Options:
         """
 
         self.options = []
+        self.args = args
         self.files = None
         if SCons.Util.is_String(files):
            self.files = [ files ]
@@ -83,12 +84,11 @@ class Options:
 
         self.options.append(option)
 
-    def Update(self, env, args):
+    def Update(self, env, args=None):
         """
         Update an environment with the option variables.
 
         env - the environment to update.
-        args - the dictionary to get the command line arguments from.
         """
 
         values = {}
@@ -105,12 +105,15 @@ class Options:
                  execfile(filename, values)
 
         # finally set the values specified on the command line
+        if args is None:
+            args = self.args
         values.update(args)
         
-        # Update the should save state
-        # This will mark options that have either been set on command line
-        # or in a loaded option file
-        # KeyError occurs when an option has default of None and has not been set
+        # Update should save state.
+        # This will mark options that have either been set on
+        # the command line or in a loaded option file.
+        # KeyError occurs when an option has default of None
+        # and has not been set.
         for option in self.options:
             try:
                 if values[option.key] != option.default:
@@ -177,7 +180,8 @@ class Options:
         """
         Generate the help text for the options.
 
-        env - an environment that is used to get the current values of the options.
+        env - an environment that is used to get the current values
+              of the options.
         """
 
         help_text = ""
@@ -196,4 +200,3 @@ class Options:
                 help_text = help_text + '    actual: None\n'
 
         return help_text
-
diff --git a/src/engine/SCons/OptionsTests.py b/src/engine/SCons/OptionsTests.py
index 93705c0..ce8bee7 100644
--- a/src/engine/SCons/OptionsTests.py
+++ b/src/engine/SCons/OptionsTests.py
@@ -43,8 +43,8 @@ class Environment:
         return self.dict.has_key(key)
 
 
-def check(key,value, env):
-    assert value == 6 * 9,key
+def check(key, value, env):
+    assert value == 6 * 9, "key %s = %s" % (key, value)
     
 # Check saved option file by executing and comparing against
 # the expected dictionary
@@ -56,6 +56,7 @@ def checkSave(file, expected):
 
 class OptionsTestCase(unittest.TestCase):
     def test_Add(self):
+        """Test adding to an Options object"""
         opts = SCons.Options.Options()
 
         opts.Add('VAR')
@@ -91,7 +92,9 @@ class OptionsTestCase(unittest.TestCase):
         test_it('foo.bar')
 
     def test_Update(self):
+        """Test updating an Environment"""
 
+        # Test that a default value is validated correctly.
         test = TestSCons.TestSCons()
         file = test.workpath('custom.py')
         opts = SCons.Options.Options(file)
@@ -103,9 +106,15 @@ class OptionsTestCase(unittest.TestCase):
                  lambda x: int(x) + 12)
 
         env = Environment()
+        opts.Update(env)
+        assert env['ANSWER'] == 54
+
+        env = Environment()
         opts.Update(env, {})
         assert env['ANSWER'] == 54
 
+        # Test that a bad value from the file is used and
+        # validation fails correctly.
         test = TestSCons.TestSCons()
         file = test.workpath('custom.py')
         test.write('custom.py', 'ANSWER=54')
@@ -118,11 +127,22 @@ class OptionsTestCase(unittest.TestCase):
                  lambda x: int(x) + 12)
 
         env = Environment()
+        exc_caught = None
+        try:
+            opts.Update(env)
+        except AssertionError:
+            exc_caught = 1
+        assert exc_caught, "did not catch expected assertion"
+
+        env = Environment()
+        exc_caught = None
         try:
             opts.Update(env, {})
         except AssertionError:
-            pass
+            exc_caught = 1
+        assert exc_caught, "did not catch expected assertion"
 
+        # Test that a good value from the file is used and validated.
         test = TestSCons.TestSCons()
         file = test.workpath('custom.py')
         test.write('custom.py', 'ANSWER=42')
@@ -130,22 +150,49 @@ class OptionsTestCase(unittest.TestCase):
         
         opts.Add('ANSWER',
                  'THE answer to THE question',
-                 "54",
+                 "10",
                  check,
                  lambda x: int(x) + 12)
 
         env = Environment()
+        opts.Update(env)
+        assert env['ANSWER'] == 54
+
+        env = Environment()
         opts.Update(env, {})
         assert env['ANSWER'] == 54
 
+        # Test that a bad value from an args dictionary passed to
+        # Update() is used and validation fails correctly.
         test = TestSCons.TestSCons()
         file = test.workpath('custom.py')
-        test.write('custom.py', 'ANSWER=54')
+        test.write('custom.py', 'ANSWER=10')
         opts = SCons.Options.Options(file)
         
         opts.Add('ANSWER',
                  'THE answer to THE question',
-                 "54",
+                 "12",
+                 check,
+                 lambda x: int(x) + 12)
+
+        env = Environment()
+        exc_caught = None
+        try:
+            opts.Update(env, {'ANSWER':'54'})
+        except AssertionError:
+            exc_caught = 1
+        assert exc_caught, "did not catch expected assertion"
+
+        # Test that a good value from an args dictionary
+        # passed to Update() is used and validated.
+        test = TestSCons.TestSCons()
+        file = test.workpath('custom.py')
+        test.write('custom.py', 'ANSWER=10')
+        opts = SCons.Options.Options(file)
+        
+        opts.Add('ANSWER',
+                 'THE answer to THE question',
+                 "12",
                  check,
                  lambda x: int(x) + 12)
 
@@ -153,8 +200,65 @@ class OptionsTestCase(unittest.TestCase):
         opts.Update(env, {'ANSWER':'42'})
         assert env['ANSWER'] == 54
 
+    def test_args(self):
+        """Test updating an Environment with arguments overridden"""
+
+        # Test that a bad (command-line) argument is used
+        # and the validation fails correctly.
+        test = TestSCons.TestSCons()
+        file = test.workpath('custom.py')
+        test.write('custom.py', 'ANSWER=42')
+        opts = SCons.Options.Options(file, {'ANSWER':54})
+        
+        opts.Add('ANSWER',
+                 'THE answer to THE question',
+                 "42",
+                 check,
+                 lambda x: int(x) + 12)
+
+        env = Environment()
+        exc_caught = None
+        try:
+            opts.Update(env)
+        except AssertionError:
+            exc_caught = 1
+        assert exc_caught, "did not catch expected assertion"
+
+        # Test that a good (command-line) argument is used and validated.
+        test = TestSCons.TestSCons()
+        file = test.workpath('custom.py')
+        test.write('custom.py', 'ANSWER=54')
+        opts = SCons.Options.Options(file, {'ANSWER':42})
+        
+        opts.Add('ANSWER',
+                 'THE answer to THE question',
+                 "54",
+                 check,
+                 lambda x: int(x) + 12)
+
+        env = Environment()
+        opts.Update(env)
+        assert env['ANSWER'] == 54
+
+        # Test that a (command-line) argument is overridden by a dictionary
+        # supplied to Update() and the dictionary value is validated correctly.
+        test = TestSCons.TestSCons()
+        file = test.workpath('custom.py')
+        test.write('custom.py', 'ANSWER=54')
+        opts = SCons.Options.Options(file, {'ANSWER':54})
+        
+        opts.Add('ANSWER',
+                 'THE answer to THE question',
+                 "54",
+                 check,
+                 lambda x: int(x) + 12)
+
+        env = Environment()
+        opts.Update(env, {'ANSWER':42})
+        assert env['ANSWER'] == 54
 
     def test_Save(self):
+        """Testing saving Options"""
 
         test = TestSCons.TestSCons()
         cache_file = test.workpath('cached.options')
diff --git a/src/engine/SCons/Script/SConscript.py b/src/engine/SCons/Script/SConscript.py
index 1e14658..0b3aa41 100644
--- a/src/engine/SCons/Script/SConscript.py
+++ b/src/engine/SCons/Script/SConscript.py
@@ -423,8 +423,8 @@ def SourceSignatures(type):
 
 
 class Options(SCons.Options.Options):
-    def Update(self, env):
-        return SCons.Options.Options.Update(self, env, arguments)
+    def __init__(self, files=None, args=arguments):
+        SCons.Options.Options.__init__(self, files, args)
 
 def CheckVersion(major,minor,version_string):
     """Return 0 if 'major' and 'minor' are greater than the version