_yaml.py: Prevent ruamel from removing underscores

The ruamel parser interprets values such as 1_2_3 as numbers, ignoring
the underscores. However, for `track` we need the value as a verbatim
string. This change configures ruamel to parse all values as strings,
leaving potential conversions to node_get() with the `expected_type`
parameter.

This fixes #166 and adds tests

7 files changed, 52 insertions, 20 deletions

diff --git a/buildstream/_project.py b/buildstream/_project.py
index 7a5c1f9de..6ff44b0a2 100644
--- a/buildstream/_project.py
+++ b/buildstream/_project.py
@@ -246,7 +246,8 @@ class Project():
         self._variables['project-name'] = self.name
 
         # Extend variables with automatic variables and option exports
-        self._variables['max-jobs'] = multiprocessing.cpu_count()
+        # Initialize it as a string as all variables are processed as strings.
+        self._variables['max-jobs'] = str(multiprocessing.cpu_count())
 
         # Export options into variables, if that was requested
         for _, option in self._options.options.items():
diff --git a/buildstream/_variables.py b/buildstream/_variables.py
index fa73006e6..f36458dc0 100644
--- a/buildstream/_variables.py
+++ b/buildstream/_variables.py
@@ -115,9 +115,10 @@ class Variables():
 
         # Special case, if notparallel is specified in the variables for this
         # element, then override max-jobs to be 1.
+        # Initialize it as a string as all variables are processed as strings.
         #
         if _yaml.node_get(variables, bool, 'notparallel', default_value=False):
-            variables['max-jobs'] = 1
+            variables['max-jobs'] = str(1)
 
         # Resolve the dictionary once, reporting the new dictionary with things
         # substituted in it, and reporting unmatched tokens.
diff --git a/buildstream/_yaml.py b/buildstream/_yaml.py
index 0e85f0bfa..0af53c5f7 100644
--- a/buildstream/_yaml.py
+++ b/buildstream/_yaml.py
@@ -25,8 +25,12 @@ from contextlib import ExitStack
 
 from ruamel import yaml
 from ruamel.yaml.representer import SafeRepresenter, RoundTripRepresenter
+from ruamel.yaml.constructor import RoundTripConstructor
 from ._exceptions import LoadError, LoadErrorReason
 
+# This overrides the ruamel constructor to treat everything as a string
+RoundTripConstructor.add_constructor(u'tag:yaml.org,2002:int', RoundTripConstructor.construct_yaml_str)
+RoundTripConstructor.add_constructor(u'tag:yaml.org,2002:float', RoundTripConstructor.construct_yaml_str)
 
 # We store information in the loaded yaml on a DictProvenance
 # stored in all dictionaries under this key
diff --git a/tests/context/context.py b/tests/context/context.py
index a153d7f6d..0dc6c588b 100644
--- a/tests/context/context.py
+++ b/tests/context/context.py
@@ -113,18 +113,3 @@ def test_context_load_notdict_config(context_fixture, datafiles):
 
     # XXX Should this be a different LoadErrorReason ?
     assert (exc.value.reason == LoadErrorReason.INVALID_YAML)
-
-
-@pytest.mark.datafiles(os.path.join(DATA_DIR))
-def test_context_load_invalid_type(context_fixture, datafiles):
-    context = context_fixture['context']
-    assert(isinstance(context, Context))
-
-    conf_file = os.path.join(datafiles.dirname,
-                             datafiles.basename,
-                             'invalidtype.yaml')
-
-    with pytest.raises(LoadError) as exc:
-        context.load(conf_file)
-
-    assert (exc.value.reason == LoadErrorReason.ILLEGAL_COMPOSITE)
diff --git a/tests/context/data/invalidtype.yaml b/tests/context/data/invalidtype.yaml
deleted file mode 100644
index 3eb3200a8..000000000
--- a/tests/context/data/invalidtype.yaml
+++ /dev/null
@@ -1,3 +0,0 @@
-# Specify wrong value type for something
-
-sourcedir: 5
diff --git a/tests/yaml/data/convert_value_to_str.yaml b/tests/yaml/data/convert_value_to_str.yaml
new file mode 100644
index 000000000..4a59e3287
--- /dev/null
+++ b/tests/yaml/data/convert_value_to_str.yaml
@@ -0,0 +1,4 @@
+Test1: 1_23_4
+Test2: 1.23.4
+Test3: 1.20
+Test4: OneTwoThree
diff --git a/tests/yaml/yaml.py b/tests/yaml/yaml.py
index a462e18df..3b9f385ed 100644
--- a/tests/yaml/yaml.py
+++ b/tests/yaml/yaml.py
@@ -350,3 +350,43 @@ def test_list_composition_twice(datafiles, filename1, filename2,
 
     assert child['mood'] == mood
     assert_provenance(prov_file, prov_line, prov_col, child, 'mood')
+
+
+@pytest.mark.datafiles(os.path.join(DATA_DIR))
+def test_convert_value_to_string(datafiles):
+    conf_file = os.path.join(datafiles.dirname,
+                             datafiles.basename,
+                             'convert_value_to_str.yaml')
+
+    # Run file through yaml to convert it
+    test_dict = _yaml.load(conf_file)
+
+    user_config = _yaml.node_get(test_dict, str, "Test1")
+    assert isinstance(user_config, str)
+    assert user_config == "1_23_4"
+
+    user_config = _yaml.node_get(test_dict, str, "Test2")
+    assert isinstance(user_config, str)
+    assert user_config == "1.23.4"
+
+    user_config = _yaml.node_get(test_dict, str, "Test3")
+    assert isinstance(user_config, str)
+    assert user_config == "1.20"
+
+    user_config = _yaml.node_get(test_dict, str, "Test4")
+    assert isinstance(user_config, str)
+    assert user_config == "OneTwoThree"
+
+
+@pytest.mark.datafiles(os.path.join(DATA_DIR))
+def test_value_doesnt_match_expected(datafiles):
+    conf_file = os.path.join(datafiles.dirname,
+                             datafiles.basename,
+                             'convert_value_to_str.yaml')
+
+    # Run file through yaml to convert it
+    test_dict = _yaml.load(conf_file)
+
+    with pytest.raises(LoadError) as exc:
+        user_config = _yaml.node_get(test_dict, int, "Test4")
+    assert exc.value.reason == LoadErrorReason.INVALID_DATA