From 3d6f78023f99ec7d2ba993c486b5ffaa36eb52d9 Mon Sep 17 00:00:00 2001 From: Rafael Zalamena Date: Wed, 28 Jun 2017 15:04:00 -0300 Subject: [PATCH 1/6] topotest: implement json_cmp function Implemented a JSON compare function that tells you when a specific subset of items exist or not inside a JSON dataset. More details can be found in the function docstring or in the test file lib/test_json.py. --- lib/test/__init__.py | 0 lib/test/test_json.py | 253 ++++++++++++++++++++++++++++++++++++++++++ lib/topotest.py | 39 +++++++ 3 files changed, 292 insertions(+) create mode 100644 lib/test/__init__.py create mode 100755 lib/test/test_json.py diff --git a/lib/test/__init__.py b/lib/test/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/lib/test/test_json.py b/lib/test/test_json.py new file mode 100755 index 000000000000..87bf48dda242 --- /dev/null +++ b/lib/test/test_json.py @@ -0,0 +1,253 @@ +#!/usr/bin/env python + +# +# test_json.py +# Tests for library function: json_cmp(). +# +# Copyright (c) 2017 by +# Network Device Education Foundation, Inc. ("NetDEF") +# +# Permission to use, copy, modify, and/or distribute this software +# for any purpose with or without fee is hereby granted, provided +# that the above copyright notice and this permission notice appear +# in all copies. +# +# THE SOFTWARE IS PROVIDED "AS IS" AND NETDEF DISCLAIMS ALL WARRANTIES +# WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF +# MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL NETDEF BE LIABLE FOR +# ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY +# DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, +# WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS +# ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE +# OF THIS SOFTWARE. +# + +""" +Tests for the json_cmp() function. +""" + +import os +import sys +import pytest + +# Save the Current Working Directory to find lib files. +CWD = os.path.dirname(os.path.realpath(__file__)) +sys.path.append(os.path.join(CWD, '../../')) + +# pylint: disable=C0413 +from lib.topotest import json_cmp + +def test_json_intersect_true(): + "Test simple correct JSON intersections" + + dcomplete = { + 'i1': 'item1', + 'i2': 'item2', + 'i3': 'item3', + 'i100': 'item4', + } + + dsub1 = { + 'i1': 'item1', + 'i3': 'item3', + } + dsub2 = { + 'i1': 'item1', + 'i2': 'item2', + } + dsub3 = { + 'i100': 'item4', + 'i2': 'item2', + } + dsub4 = { + 'i50': None, + 'i100': 'item4', + } + + assert json_cmp(dcomplete, dsub1) is None + assert json_cmp(dcomplete, dsub2) is None + assert json_cmp(dcomplete, dsub3) is None + assert json_cmp(dcomplete, dsub4) is None + +def test_json_intersect_false(): + "Test simple incorrect JSON intersections" + + dcomplete = { + 'i1': 'item1', + 'i2': 'item2', + 'i3': 'item3', + 'i100': 'item4', + } + + # Incorrect value for 'i1' + dsub1 = { + 'i1': 'item3', + 'i3': 'item3', + } + # Non-existing key 'i5' + dsub2 = { + 'i1': 'item1', + 'i5': 'item2', + } + # Key should not exist + dsub3 = { + 'i100': None, + } + + assert json_cmp(dcomplete, dsub1) is not None + assert json_cmp(dcomplete, dsub2) is not None + assert json_cmp(dcomplete, dsub3) is not None + +def test_json_intersect_multilevel_true(): + "Test multi level correct JSON intersections" + + dcomplete = { + 'i1': 'item1', + 'i2': 'item2', + 'i3': { + 'i100': 'item100', + }, + 'i4': { + 'i41': { + 'i411': 'item411', + }, + 'i42': { + 'i421': 'item421', + 'i422': 'item422', + } + } + } + + dsub1 = { + 'i1': 'item1', + 'i3': { + 'i100': 'item100', + }, + 'i10': None, + } + dsub2 = { + 'i1': 'item1', + 'i2': 'item2', + 'i3': {}, + } + dsub3 = { + 'i2': 'item2', + 'i4': { + 'i41': { + 'i411': 'item411', + }, + 'i42': { + 'i422': 'item422', + 'i450': None, + } + } + } + dsub4 = { + 'i2': 'item2', + 'i4': { + 'i41': {}, + 'i42': { + 'i450': None, + } + } + } + dsub5 = { + 'i2': 'item2', + 'i3': { + 'i100': 'item100', + }, + 'i4': { + 'i42': { + 'i450': None, + } + } + } + + assert json_cmp(dcomplete, dsub1) is None + assert json_cmp(dcomplete, dsub2) is None + assert json_cmp(dcomplete, dsub3) is None + assert json_cmp(dcomplete, dsub4) is None + assert json_cmp(dcomplete, dsub5) is None + +def test_json_intersect_multilevel_false(): + "Test multi level incorrect JSON intersections" + + dcomplete = { + 'i1': 'item1', + 'i2': 'item2', + 'i3': { + 'i100': 'item100', + }, + 'i4': { + 'i41': { + 'i411': 'item411', + }, + 'i42': { + 'i421': 'item421', + 'i422': 'item422', + } + } + } + + # Incorrect sub-level value + dsub1 = { + 'i1': 'item1', + 'i3': { + 'i100': 'item00', + }, + 'i10': None, + } + # Inexistent sub-level + dsub2 = { + 'i1': 'item1', + 'i2': 'item2', + 'i3': None, + } + # Inexistent sub-level value + dsub3 = { + 'i1': 'item1', + 'i3': { + 'i100': None, + }, + } + # Inexistent sub-sub-level value + dsub4 = { + 'i4': { + 'i41': { + 'i412': 'item412', + }, + 'i42': { + 'i421': 'item421', + } + } + } + # Invalid sub-sub-level value + dsub5 = { + 'i4': { + 'i41': { + 'i411': 'item411', + }, + 'i42': { + 'i421': 'item420000', + } + } + } + # sub-sub-level should be value + dsub6 = { + 'i4': { + 'i41': { + 'i411': 'item411', + }, + 'i42': 'foobar', + } + } + + assert json_cmp(dcomplete, dsub1) is not None + assert json_cmp(dcomplete, dsub2) is not None + assert json_cmp(dcomplete, dsub3) is not None + assert json_cmp(dcomplete, dsub4) is not None + assert json_cmp(dcomplete, dsub5) is not None + assert json_cmp(dcomplete, dsub6) is not None + +if __name__ == '__main__': + sys.exit(pytest.main()) diff --git a/lib/topotest.py b/lib/topotest.py index d0b377b33b11..f484366750fb 100644 --- a/lib/topotest.py +++ b/lib/topotest.py @@ -42,6 +42,45 @@ from time import sleep + +def json_cmp(d1, d2, reason=False): + """ + JSON compare function. Receives two parameters: + * `d1`: json value + * `d2`: json subset which we expect + + Returns `None` when all keys that `d1` has matches `d2`, + otherwise a string containing what failed. + + Note: key absence can be tested by adding a key with value `None`. + """ + squeue = [(d1, d2)] + for s in squeue: + nd1, nd2 = s + s1, s2 = set(nd1), set(nd2) + + # Expect all required fields to exist. + s2_req = set([key for key in nd2 if nd2[key] is not None]) + diff = s2_req - s1 + if diff != set({}): + return 'expected keys "{}" in "{}"'.format(diff, str(nd1)) + + for key in s2.intersection(s1): + # Test for non existence of key in d2 + if nd2[key] is None: + return '"{}" should not exist in "{}"'.format(key, str(nd1)) + # If nd1 key is a dict, we have to recurse in it later. + if isinstance(nd2[key], type({})): + squeue.append((nd1[key], nd2[key])) + continue + # Compare JSON values + if nd1[key] != nd2[key]: + return '"{}" value is different ("{}" != "{}")'.format( + key, str(nd1[key]), str(nd2[key]) + ) + + return None + def run_and_expect(func, what, count=20, wait=3): """ Run `func` and compare the result with `what`. Do it for `count` times From af3a965bb72f93f8b7b070613f7ad234c25723f8 Mon Sep 17 00:00:00 2001 From: Rafael Zalamena Date: Wed, 28 Jun 2017 15:30:44 -0300 Subject: [PATCH 2/6] topogen: added JSON output support for vtysh_cmd Allow vtysh_cmd() to convert JSON output to Python data structures. This feature will be used to get vtysh JSON outputs for tests comparsions. Usage example: ```py router = get_topogen().gears['r1'] json_output = router.vtysh_cmd('show ip ospf json', isjson=True) json_cmp(json_output, {'important_key': 'important_value'}) ``` --- lib/topogen.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/topogen.py b/lib/topogen.py index f4374015173c..97f006c59cda 100644 --- a/lib/topogen.py +++ b/lib/topogen.py @@ -40,6 +40,7 @@ import os import sys +import json from mininet.net import Mininet from mininet.log import setLogLevel @@ -388,7 +389,7 @@ def start(self): """ return self.tgen.net[self.name].startRouter() - def vtysh_cmd(self, command): + def vtysh_cmd(self, command, isjson=False): """ Runs the provided command string in the vty shell and returns a string with the response. @@ -401,7 +402,11 @@ def vtysh_cmd(self, command): return self.vtysh_multicmd(command) vtysh_command = 'vtysh -c "{}" 2>/dev/null'.format(command) - return self.run(vtysh_command) + output = self.run(vtysh_command) + if isjson is False: + return output + + return json.loads(output) def vtysh_multicmd(self, commands, pretty_output=True): """ From 89a61ce00a003252f48757628e4aed1f4abf0f56 Mon Sep 17 00:00:00 2001 From: Rafael Zalamena Date: Thu, 29 Jun 2017 10:46:19 -0300 Subject: [PATCH 3/6] template: allow test to be run without pytest Update the PYTHONPATH for standalone runs and pass all command line arguments to pytest main. Also set the executable bit to the python scripts file. --- example-test/__init__.py | 0 example-test/test_template.py | 11 +++++++---- 2 files changed, 7 insertions(+), 4 deletions(-) mode change 100644 => 100755 example-test/__init__.py mode change 100644 => 100755 example-test/test_template.py diff --git a/example-test/__init__.py b/example-test/__init__.py old mode 100644 new mode 100755 diff --git a/example-test/test_template.py b/example-test/test_template.py old mode 100644 new mode 100755 index ac6faeaa6388..b7556458a21f --- a/example-test/test_template.py +++ b/example-test/test_template.py @@ -30,6 +30,11 @@ import sys import pytest +# Save the Current Working Directory to find configuration files. +CWD = os.path.dirname(os.path.realpath(__file__)) +sys.path.append(os.path.join(CWD, '../')) + +# pylint: disable=C0413 # Import topogen and topotest helpers from lib import topotest from lib.topogen import Topogen, TopoRouter, get_topogen @@ -37,9 +42,6 @@ # Required to instantiate the topology builder class. from mininet.topo import Topo -# Save the Current Working Directory to find configuration files. -CWD = os.path.dirname(os.path.realpath(__file__)) - class TemplateTopo(Topo): "Test topology builder" def build(self, *_args, **_opts): @@ -99,4 +101,5 @@ def test_call_mininet_cli(): tgen.mininet_cli() if __name__ == '__main__': - sys.exit(pytest.main(["-s"])) + args = ["-s"] + sys.argv[1:] + sys.exit(pytest.main(args)) From c8bbb352efc0d5422343c3825a864fcaa3047df7 Mon Sep 17 00:00:00 2001 From: Rafael Zalamena Date: Thu, 29 Jun 2017 10:49:11 -0300 Subject: [PATCH 4/6] topogen: don't backtrace when topogen is not used This allows old tests to be run with '--topology-only' without generating tons of error messages, instead it will just stop the test without trying anything else. --- conftest.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) mode change 100644 => 100755 conftest.py diff --git a/conftest.py b/conftest.py old mode 100644 new mode 100755 index e042a3d13ad4..a8bc53994342 --- a/conftest.py +++ b/conftest.py @@ -21,6 +21,9 @@ def pytest_runtest_call(): # pylint: disable=E1101 # Trust me, 'config' exists. if pytest.config.getoption('--topology-only'): - # Allow user to play with the setup. - get_topogen().mininet_cli() + tgen = get_topogen() + if tgen is not None: + # Allow user to play with the setup. + tgen.mininet_cli() + pytest.exit('the topology executed successfully') From 3e808c32880e8929f64f6544b781d0b630cdcf63 Mon Sep 17 00:00:00 2001 From: Rafael Zalamena Date: Thu, 29 Jun 2017 12:18:46 -0300 Subject: [PATCH 5/6] topotest: improve json_cmp assert output Create a specialized assert and json_cmp() result to improve the comparison output. With this we also got a way to display all comparison failures instead of just the first one. --- conftest.py | 13 +++++++++++++ lib/topotest.py | 39 +++++++++++++++++++++++++++++++-------- 2 files changed, 44 insertions(+), 8 deletions(-) diff --git a/conftest.py b/conftest.py index a8bc53994342..cea7ad448abc 100755 --- a/conftest.py +++ b/conftest.py @@ -3,6 +3,7 @@ """ from lib.topogen import get_topogen +from lib.topotest import json_cmp_result import pytest def pytest_addoption(parser): @@ -27,3 +28,15 @@ def pytest_runtest_call(): tgen.mininet_cli() pytest.exit('the topology executed successfully') + +def pytest_assertrepr_compare(op, left, right): + """ + Show proper assertion error message for json_cmp results. + """ + json_result = left + if not isinstance(json_result, json_cmp_result): + json_result = right + if not isinstance(json_result, json_cmp_result): + return None + + return json_result.errors diff --git a/lib/topotest.py b/lib/topotest.py index f484366750fb..9f540fc86f3b 100644 --- a/lib/topotest.py +++ b/lib/topotest.py @@ -42,6 +42,20 @@ from time import sleep +class json_cmp_result(object): + "json_cmp result class for better assertion messages" + + def __init__(self): + self.errors = [] + + def add_error(self, error): + "Append error message to the result" + self.errors.append(error) + + def has_errors(self): + "Returns True if there were errors, otherwise False." + return len(self.errors) > 0 + def json_cmp(d1, d2, reason=False): """ @@ -54,30 +68,39 @@ def json_cmp(d1, d2, reason=False): Note: key absence can be tested by adding a key with value `None`. """ - squeue = [(d1, d2)] + squeue = [(d1, d2, 'json')] + result = json_cmp_result() for s in squeue: - nd1, nd2 = s + nd1, nd2, parent = s s1, s2 = set(nd1), set(nd2) # Expect all required fields to exist. s2_req = set([key for key in nd2 if nd2[key] is not None]) diff = s2_req - s1 if diff != set({}): - return 'expected keys "{}" in "{}"'.format(diff, str(nd1)) + result.add_error('expected key(s) {} in {} (have {})'.format( + str(list(diff)), parent, str(list(s1)))) for key in s2.intersection(s1): # Test for non existence of key in d2 if nd2[key] is None: - return '"{}" should not exist in "{}"'.format(key, str(nd1)) + result.add_error('"{}" should not exist in {} (have {})'.format( + key, parent, str(s1))) + continue # If nd1 key is a dict, we have to recurse in it later. if isinstance(nd2[key], type({})): - squeue.append((nd1[key], nd2[key])) + nparent = '{}["{}"]'.format(parent, key) + squeue.append((nd1[key], nd2[key], nparent)) continue # Compare JSON values if nd1[key] != nd2[key]: - return '"{}" value is different ("{}" != "{}")'.format( - key, str(nd1[key]), str(nd2[key]) - ) + result.add_error( + '{}["{}"] value is different (have "{}", expected "{}")'.format( + parent, key, str(nd1[key]), str(nd2[key]))) + continue + + if result.has_errors(): + return result return None From f0003c38f46516f59a2a5d741a66a101def7cf18 Mon Sep 17 00:00:00 2001 From: Rafael Zalamena Date: Thu, 29 Jun 2017 18:23:34 -0300 Subject: [PATCH 6/6] topotest: add writing tests tips Add two tips to help improve test code quality: 1) Store function returns for later inspection 2) Identify what failed using the assert message --- GUIDELINES.md | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/GUIDELINES.md b/GUIDELINES.md index 5f745ae7cebc..1566705bc1b8 100644 --- a/GUIDELINES.md +++ b/GUIDELINES.md @@ -419,6 +419,34 @@ Requirements: * Tests must be able to run without any interaction. To make sure your test conforms with this, run it without the `-s` parameter. +Tips: + +* Keep results in stack variables, so people inspecting code with `pdb` can + easily print their values. + + Don't do this: + + ```py + assert foobar(router1, router2) + ``` + + Do this instead: + + ```py + result = foobar(router1, router2) + assert result + ``` + +* Use `assert` messages to indicate where the test failed. + + Example: + + ```py + for router in router_list: + # ... + assert condition, 'Router "{}" condition failed'.format(router.name) + ``` + ### Debugging Execution