Adam Litke has uploaded a new change for review.
Change subject: tests: Introduce schema parsing tests ......................................................................
tests: Introduce schema parsing tests
Change-Id: I48c2d2cb260e175ad119ece4c5e36552bb303ada Signed-off-by: Adam Litke alitke@redhat.com --- M tests/schemaTests.py 1 file changed, 88 insertions(+), 0 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/64/45464/1
diff --git a/tests/schemaTests.py b/tests/schemaTests.py index e00edba..c6fc89b 100644 --- a/tests/schemaTests.py +++ b/tests/schemaTests.py @@ -17,6 +17,10 @@ # Refer to the README and COPYING files for full details of the license #
+import textwrap +from contextlib import contextmanager + +from testlib import temporaryPath from testlib import VdsmTestCase as TestCaseBase from rpc import vdsmapi
@@ -92,3 +96,87 @@ def testTokenizeRaiseOnInvalidData(self): generator = vdsmapi.tokenize("{'a': invalid, 'b': 'c'}") self.assertRaises(ValueError, list, generator) + + +class ParseTest(TestCaseBase): + blank_schema = {'types': {}, 'enums': {}, 'aliases': {}, + 'maps': {}, 'commands': {}, 'unions': {}} + + @contextmanager + def load_api(self, data): + vdsmapi._api_info = None + data = textwrap.dedent(data) + with temporaryPath(data=data) as filename: + yield vdsmapi.get_api(filename) + + def assertTypeRelation(self, api, type_a, type_b): + self.assertIn(type_b, api['unions'][type_a]) + self.assertIn(type_a, api['unions'][type_b]) + + def test_empty_schema(self): + with self.load_api('') as api: + self.assertEqual(self.blank_schema, api) + + def test_unknown_symbol_type(self): + with self.load_api(''' + {'foo': 'bar', 'data': ['a', 'b', 'c']} + ''') as api: + self.assertEqual(self.blank_schema, api) + + def test_single_type(self): + with self.load_api(''' + {'type': 'foo', 'data': {'a': 'str'}} + ''') as api: + self.assertIn('foo', api['types']) + self.assertEqual('str', api['types']['foo']['data']['a']) + + def test_single_enum(self): + with self.load_api(''' + {'enum': 'foo', 'data': ['a', 'b', 'c']} + ''') as api: + self.assertEqual(['a', 'b', 'c'], api['enums']['foo']['data']) + + def test_single_alias(self): + with self.load_api(''' + {'alias': 'UUID', 'data': 'str'} + ''') as api: + self.assertEqual('str', api['aliases']['UUID']['data']) + + def test_single_map(self): + map = {'map': 'foo', 'key': 'str', 'value': 'str'} + with self.load_api(str(map)) as api: + self.assertEqual(map, api['maps']['foo']) + + def test_single_command(self): + cmd = {'command': {'class': 'foo', 'name': 'bar'}, + 'data': {'a': 'uint'}, 'returns': 'str'} + with self.load_api(str(cmd)) as api: + self.assertEqual(cmd, api['commands']['foo']['bar']) + + def test_command_parameter_order(self): + # The parser will preserve the order of dict keys when loading + with self.load_api(''' + {'command': {'class': 'foo', 'name': 'bar'}, + 'data': {'a': 'uint', 'b': 'str', 'c': 'int'}} + ''') as api: + self.assertEqual(['a', 'b', 'c'], + api['commands']['foo']['bar']['data'].keys()) + + def test_type_union(self): + with self.load_api(''' + {'enum': 'ThingTypes', 'data': ['person', 'place']} + + {'type': 'Thing', + 'data': {'specificType': 'ThingTypes', 'name': 'str'}, + 'union': ['Person', 'Place']} + + {'type': 'Person', + 'data': {'specificType': 'ThingTypes', 'name': 'str', + 'address': 'str'}} + + {'type': 'Place', + 'data': {'specificType': 'ThingTypes', 'name': 'str', + 'longitude': 'float', 'latitude': 'float'}} + ''') as api: + self.assertTypeRelation(api, 'Thing', 'Person') + self.assertTypeRelation(api, 'Thing', 'Place')
automation@ovirt.org has posted comments on this change.
Change subject: tests: Introduce schema parsing tests ......................................................................
Patch Set 1:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
automation@ovirt.org has posted comments on this change.
Change subject: tests: Introduce schema parsing tests ......................................................................
Patch Set 2:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Nir Soffer has posted comments on this change.
Change subject: tests: Introduce schema parsing tests ......................................................................
Patch Set 2: Code-Review-1
(1 comment)
Partial review, -1 for visibility.
https://gerrit.ovirt.org/#/c/45464/2/tests/schemaTests.py File tests/schemaTests.py:
Line 106: def load_api(self, data): Line 107: vdsmapi._api_info = None Line 108: data = textwrap.dedent(data) Line 109: with temporaryPath(data=data) as filename: Line 110: yield vdsmapi.get_api(filename) Can we avoid creating a temporary file for each test?
I think we need only one test to check that vdsmapi.get_api can read from a file. The rest of the tests should use memory based schema.
Not visiting the file system is much faster, and we don't want all parsing tests to fail if the file reading code is broken. We like to have one failing test for one error in the code. Line 111: Line 112: def assertTypeRelation(self, api, type_a, type_b): Line 113: self.assertIn(type_b, api['unions'][type_a]) Line 114: self.assertIn(type_a, api['unions'][type_b])
Piotr Kliczewski has posted comments on this change.
Change subject: tests: Introduce schema parsing tests ......................................................................
Patch Set 2: Code-Review-1
Agree with Nir
Nir Soffer has posted comments on this change.
Change subject: tests: Introduce schema parsing tests ......................................................................
Patch Set 2: -Code-Review
(1 comment)
I don't have time to dive into the schema parser now.
https://gerrit.ovirt.org/#/c/45464/2/tests/schemaTests.py File tests/schemaTests.py:
Line 106: def load_api(self, data): Line 107: vdsmapi._api_info = None Line 108: data = textwrap.dedent(data) Line 109: with temporaryPath(data=data) as filename: Line 110: yield vdsmapi.get_api(filename)
Can we avoid creating a temporary file for each test?
After talking with Adam, we decided to improve this later. Line 111: Line 112: def assertTypeRelation(self, api, type_a, type_b): Line 113: self.assertIn(type_b, api['unions'][type_a]) Line 114: self.assertIn(type_a, api['unions'][type_b])
Piotr Kliczewski has posted comments on this change.
Change subject: tests: Introduce schema parsing tests ......................................................................
Patch Set 2: Code-Review+1
After talking to Adam we can have them as they are and refactor later if needed.
Adam Litke has posted comments on this change.
Change subject: tests: Introduce schema parsing tests ......................................................................
Patch Set 2: Verified+1
automation@ovirt.org has posted comments on this change.
Change subject: tests: Introduce schema parsing tests ......................................................................
Patch Set 3:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Dan Kenigsberg has posted comments on this change.
Change subject: tests: Introduce schema parsing tests ......................................................................
Patch Set 3: Code-Review+2
raising score
Dan Kenigsberg has submitted this change and it was merged.
Change subject: tests: Introduce schema parsing tests ......................................................................
tests: Introduce schema parsing tests
We currently have some tests that test the schema tokenizer but we don't have any tests to verify that the tokens are parsed correctly into a schema. Introduce a new set of tests to do some really basic verification of the API builder.
Change-Id: I48c2d2cb260e175ad119ece4c5e36552bb303ada Signed-off-by: Adam Litke alitke@redhat.com Reviewed-on: https://gerrit.ovirt.org/45464 Reviewed-by: Piotr Kliczewski piotr.kliczewski@gmail.com Continuous-Integration: Jenkins CI Reviewed-by: Dan Kenigsberg danken@redhat.com --- M tests/schemaTests.py 1 file changed, 88 insertions(+), 0 deletions(-)
Approvals: Piotr Kliczewski: Looks good to me, but someone else must approve Adam Litke: Verified Jenkins CI: Passed CI tests Dan Kenigsberg: Looks good to me, approved
automation@ovirt.org has posted comments on this change.
Change subject: tests: Introduce schema parsing tests ......................................................................
Patch Set 4:
* Update tracker::IGNORE, no Bug-Url found * Set MODIFIED::IGNORE, no Bug-Url found.
vdsm-patches@lists.fedorahosted.org