From 2a86047efeea9ba5ea8aee2bea984326a93f69df Mon Sep 17 00:00:00 2001 From: Doug Hellmann Date: Wed, 22 Jun 2016 11:37:38 -0400 Subject: [PATCH] add warnings for malformated input Log warnings if the input doesn't match the type of data expected. Eventually we may want to enforce these rules by raising exceptions, but for now it should be enough to help someone debug their problem to just print a warning. Change-Id: I9016041bf13e9047d1894d2284b57b0a94554977 Signed-off-by: Doug Hellmann --- reno/loader.py | 42 ++++++++++++++++-- reno/tests/base.py | 14 +++++- reno/tests/test_loader.py | 92 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 143 insertions(+), 5 deletions(-) create mode 100644 reno/tests/test_loader.py diff --git a/reno/loader.py b/reno/loader.py index e3b3ee7..5faa530 100644 --- a/reno/loader.py +++ b/reno/loader.py @@ -15,6 +15,7 @@ import os.path from reno import scanner +import six import yaml LOG = logging.getLogger(__name__) @@ -101,9 +102,44 @@ class Loader(object): return self._scanner_output[version] def parse_note_file(self, filename, sha): - "Return the data structure encoded in the note file." + """Return the data structure encoded in the note file. + + Emit warnings for content that does not look valid in some + way, but return it anway for backwards-compatibility. + + """ if self._cache: - return self._cache['file-contents'][filename] + content = self._cache['file-contents'][filename] else: body = scanner.get_file_at_commit(self._reporoot, filename, sha) - return yaml.safe_load(body) + content = yaml.safe_load(body) + + for section_name, section_content in content.items(): + if section_name == 'prelude': + if not isinstance(section_content, six.string_types): + LOG.warning( + ('The prelude section of %s ' + 'does not parse as a single string. ' + 'Is the YAML input escaped properly?') % + filename, + ) + else: + if not isinstance(section_content, list): + LOG.warning( + ('The %s section of %s ' + 'does not parse as a list of strings. ' + 'Is the YAML input escaped properly?') % ( + section_name, filename), + ) + else: + for item in section_content: + if not isinstance(item, six.string_types): + LOG.warning( + ('The item %r in the %s section of %s ' + 'parses as a %s instead of a string. ' + 'Is the YAML input escaped properly?' + ) % (item, section_name, + filename, type(item)), + ) + + return content diff --git a/reno/tests/base.py b/reno/tests/base.py index 1c30cdb..c1f9829 100644 --- a/reno/tests/base.py +++ b/reno/tests/base.py @@ -15,9 +15,19 @@ # License for the specific language governing permissions and limitations # under the License. -from oslotest import base +import fixtures +import testtools -class TestCase(base.BaseTestCase): +class TestCase(testtools.TestCase): """Test case base class for all unit tests.""" + + def setUp(self): + super(TestCase, self).setUp() + self._stdout_fixture = fixtures.StringStream('stdout') + self.stdout = self.useFixture(self._stdout_fixture).stream + self.useFixture(fixtures.MonkeyPatch('sys.stdout', self.stdout)) + self._stderr_fixture = fixtures.StringStream('stderr') + self.stderr = self.useFixture(self._stderr_fixture).stream + self.useFixture(fixtures.MonkeyPatch('sys.stderr', self.stderr)) diff --git a/reno/tests/test_loader.py b/reno/tests/test_loader.py new file mode 100644 index 0000000..50b6520 --- /dev/null +++ b/reno/tests/test_loader.py @@ -0,0 +1,92 @@ +# -*- coding: utf-8 -*- + +# 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. + +import logging +import textwrap + +from reno import loader +from reno.tests import base + +import fixtures +import mock +import six +import yaml + + +class TestValidate(base.TestCase): + + scanner_output = { + '0.0.0': [('note', 'shaA')], + } + + versions = ['0.0.0'] + + def setUp(self): + super(TestValidate, self).setUp() + self.logger = self.useFixture( + fixtures.FakeLogger( + format='%(message)s', + level=logging.WARNING, + ) + ) + + def _make_loader(self, note_bodies): + def _load(ldr): + ldr._scanner_output = self.scanner_output + ldr._cache = { + 'file-contents': {'note1': note_bodies}, + } + + with mock.patch('reno.loader.Loader._load_data', _load): + return loader.Loader( + reporoot='reporoot', + notesdir='notesdir', + branch=None, + collapse_pre_releases=None, + earliest_version=None, + ignore_cache=False, + ) + + def test_prelude_list(self): + note_bodies = yaml.safe_load(textwrap.dedent(''' + prelude: + - This is the first comment. + - This is a second. + ''')) + self.assertIsInstance(note_bodies['prelude'], list) + ldr = self._make_loader(note_bodies) + ldr.parse_note_file('note1', None) + self.assertIn('prelude', self.logger.output) + + def test_non_prelude_single_string(self): + note_bodies = yaml.safe_load(textwrap.dedent(''' + issues: | + This is a single string. + ''')) + print(type(note_bodies['issues'])) + self.assertIsInstance(note_bodies['issues'], six.string_types) + ldr = self._make_loader(note_bodies) + ldr.parse_note_file('note1', None) + self.assertIn('list of strings', self.logger.output) + + def test_note_with_colon_as_dict(self): + note_bodies = yaml.safe_load(textwrap.dedent(''' + issues: + - This is the first issue. + - dict: This is parsed as a dictionary. + ''')) + self.assertIsInstance(note_bodies['issues'][-1], dict) + ldr = self._make_loader(note_bodies) + ldr.parse_note_file('note1', None) + self.assertIn('dict', self.logger.output)