Bug 1604360 - [manifestparser] Properly merge [DEFAULT] section of manifest with parent defaults r=gbrown

Previously the [DEFAULT] section of a manifest would simply overwrite whatever
values were passed down from the parent. This patch ensures we use
'combine_fields' so things like 'skip-if' and 'support-files' are properly
merged.

Differential Revision: https://phabricator.services.mozilla.com/D57410

--HG--
extra : moz-landing-system : lando

testing/mozbase/manifestparser/manifestparser/ini.py

@@ -24,12 +24,12 @@ class IniParseError(Exception):
         super(IniParseError, self).__init__(msg)
 
 
-def read_ini(fp, variables=None, default='DEFAULT', comments=None,
+def read_ini(fp, defaults=None, default='DEFAULT', comments=None,
              separators=None, strict=True, handle_defaults=True):
     """
     read an .ini file and return a list of [(section, values)]
     - fp : file pointer or path to read
-    - variables : default set of variables
+    - defaults : default set of variables
     - default : name of the section for the default section
     - comments : characters that if they start a line denote a comment
     - separators : strings that denote key, value separation in order
@@ -38,7 +38,8 @@ def read_ini(fp, variables=None, default='DEFAULT', comments=None,
     """
 
     # variables
-    variables = variables or {}
+    defaults = defaults or {}
+    default_section = {}
     comments = comments or ('#',)
     separators = separators or ('=', ':')
     sections = []
@@ -89,7 +90,7 @@ def read_ini(fp, variables=None, default='DEFAULT', comments=None,
                 if strict:
                     assert default not in section_names
                 section_names.add(default)
-                current_section = variables
+                current_section = default_section
                 continue
 
             if strict:
@@ -123,7 +124,7 @@ def read_ini(fp, variables=None, default='DEFAULT', comments=None,
                 key_indent = line_indent
 
                 # make sure this key isn't already in the section
-                if key and current_section is not variables:
+                if key:
                     assert key not in current_section
 
                 if strict:
@@ -136,9 +137,12 @@ def read_ini(fp, variables=None, default='DEFAULT', comments=None,
             # something bad happened!
             raise IniParseError(fp, linenum, "Unexpected line '{}'".format(stripped))
 
-    global_vars = variables if handle_defaults else {}
-    sections = [(i, combine_fields(global_vars, j)) for i, j in sections]
-    return sections
+    # merge global defaults with the DEFAULT section
+    defaults = combine_fields(defaults, default_section)
+    if handle_defaults:
+        # merge combined defaults into each section
+        sections = [(i, combine_fields(defaults, j)) for i, j in sections]
+    return sections, defaults
 
 
 def combine_fields(global_vars, local_vars):
@@ -149,7 +153,7 @@ def combine_fields(global_vars, local_vars):
     if not global_vars:
         return local_vars
     if not local_vars:
-        return global_vars
+        return global_vars.copy()
     field_patterns = {
         'skip-if': '(%s) || (%s)',
         'support-files': '%s %s',

testing/mozbase/manifestparser/manifestparser/manifestparser.py

@@ -164,8 +164,8 @@ class ManifestParser(object):
         defaults['here'] = here
 
         # read the configuration
-        sections = read_ini(fp=fp, variables=defaults, strict=self.strict,
-                            handle_defaults=self._handle_defaults)
+        sections, defaults = read_ini(fp=fp, defaults=defaults, strict=self.strict,
+                                      handle_defaults=self._handle_defaults)
         if parentmanifest and filename:
             # A manifest can be read multiple times, via "include:", optionally
             # with section-specific variables. These variables only apply to

testing/mozbase/manifestparser/tests/test_read_ini.py

@@ -28,7 +28,7 @@ def parse_manifest():
         buf = StringIO()
         buf.write(dedent(string))
         buf.seek(0)
-        return read_ini(buf, **kwargs)
+        return read_ini(buf, **kwargs)[0]
 
     return inner
 
@@ -80,5 +80,35 @@ def test_dupes_error(parse_manifest):
         parse_manifest(dupes, strict=False)
 
 
+def test_defaults_handling(parse_manifest):
+    manifest = """
+    [DEFAULT]
+    flower = rose
+    skip-if = true
+
+    [test_defaults]
+    """
+
+    result = parse_manifest(manifest)[0][1]
+    assert result['flower'] == 'rose'
+    assert result['skip-if'] == 'true'
+
+    result = parse_manifest(
+        manifest,
+        defaults={
+            'flower': 'tulip',
+            'colour': 'pink',
+            'skip-if': 'false',
+        },
+    )[0][1]
+    assert result['flower'] == 'rose'
+    assert result['colour'] == 'pink'
+    assert result['skip-if'] == '(false) || (true)'
+
+    result = parse_manifest(manifest.replace('DEFAULT', 'default'))[0][1]
+    assert result['flower'] == 'rose'
+    assert result['skip-if'] == 'true'
+
+
 if __name__ == '__main__':
     mozunit.main()