From c5c4f236fec4c7165c41300f9b0fcc9cb15e36c6 Mon Sep 17 00:00:00 2001 From: linuxdaemon Date: Wed, 13 Dec 2017 16:46:39 -0600 Subject: [PATCH 1/7] Add pylint-leaks to test for resource leaks --- .travis.yml | 2 +- tests/core_tests/test_plugin_hooks.py | 4 +++- travis/requirements.txt | 1 + 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 8e53260f3..0aa3ab4db 100644 --- a/.travis.yml +++ b/.travis.yml @@ -13,7 +13,7 @@ install: script: - "python ./travis/test_json.py" - "git diff --diff-filter=d --name-only ${TRAVIS_COMMIT_RANGE} | grep -i '\\.py$' | xargs -r pylint --rcfile=travis/pylintrc" - - "py.test . -v --cov . --cov-report term-missing" + - "py.test . -R : -v --cov . --cov-report term-missing" after_success: - "coveralls" diff --git a/tests/core_tests/test_plugin_hooks.py b/tests/core_tests/test_plugin_hooks.py index 9538778f0..e8967470c 100644 --- a/tests/core_tests/test_plugin_hooks.py +++ b/tests/core_tests/test_plugin_hooks.py @@ -26,7 +26,9 @@ def gather_plugins(): def load_plugin(plugin_path, monkeypatch): - monkeypatch.setattr('cloudbot.plugin.Hook.original_init', Hook.__init__, raising=False) + if not hasattr(Hook, "original_init"): + monkeypatch.setattr('cloudbot.plugin.Hook.original_init', Hook.__init__, raising=False) + monkeypatch.setattr('cloudbot.plugin.Hook.__init__', patch_hook_init) monkeypatch.setattr('cloudbot.util.database.metadata', MetaData()) diff --git a/travis/requirements.txt b/travis/requirements.txt index f4d918e6b..be2139313 100644 --- a/travis/requirements.txt +++ b/travis/requirements.txt @@ -2,6 +2,7 @@ pytest responses pytest-cov pytest-pep8 +pytest-leaks flake8 python-coveralls pylint From d251fd715064a7806b294699167ab267ef8fa1c8 Mon Sep 17 00:00:00 2001 From: linuxdaemon Date: Wed, 13 Dec 2017 17:01:58 -0600 Subject: [PATCH 2/7] Add travis-ci log folding plugin to pytest --- travis/requirements.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/travis/requirements.txt b/travis/requirements.txt index be2139313..8fea0d9eb 100644 --- a/travis/requirements.txt +++ b/travis/requirements.txt @@ -3,6 +3,7 @@ responses pytest-cov pytest-pep8 pytest-leaks +pytest-travis-fold flake8 python-coveralls pylint From 7759d35521a8afc7d0061b12ca99f99abe84dcd7 Mon Sep 17 00:00:00 2001 From: linuxdaemon Date: Wed, 13 Dec 2017 17:20:55 -0600 Subject: [PATCH 3/7] Move pylint tests to pytest with the pytest-pylint plugin --- .travis.yml | 4 +--- travis/requirements.txt | 1 + 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index 0aa3ab4db..2e53b0460 100644 --- a/.travis.yml +++ b/.travis.yml @@ -11,9 +11,7 @@ install: - "pip install -r ./travis/requirements.txt" script: - - "python ./travis/test_json.py" - - "git diff --diff-filter=d --name-only ${TRAVIS_COMMIT_RANGE} | grep -i '\\.py$' | xargs -r pylint --rcfile=travis/pylintrc" - - "py.test . -R : -v --cov . --cov-report term-missing" + - "py.test . -R : -v --cov . --cov-report term-missing --pylint --pylint-rcfile=travis/pylintrc" after_success: - "coveralls" diff --git a/travis/requirements.txt b/travis/requirements.txt index 8fea0d9eb..f10f4d49d 100644 --- a/travis/requirements.txt +++ b/travis/requirements.txt @@ -4,6 +4,7 @@ pytest-cov pytest-pep8 pytest-leaks pytest-travis-fold +pytest-pylint flake8 python-coveralls pylint From 15e2be00d6feedf3f0d53cfd524da3c1a30acd6b Mon Sep 17 00:00:00 2001 From: linuxdaemon Date: Wed, 13 Dec 2017 22:37:41 -0600 Subject: [PATCH 4/7] Refactor hook tests --- tests/core_tests/test_plugin_hooks.py | 85 ++++++++++++++++++++------- 1 file changed, 64 insertions(+), 21 deletions(-) diff --git a/tests/core_tests/test_plugin_hooks.py b/tests/core_tests/test_plugin_hooks.py index e8967470c..33dbf3318 100644 --- a/tests/core_tests/test_plugin_hooks.py +++ b/tests/core_tests/test_plugin_hooks.py @@ -8,15 +8,28 @@ from sqlalchemy import MetaData +from cloudbot.event import Event, CommandEvent, RegexEvent, CapEvent, PostHookEvent, IrcOutEvent from cloudbot.hook import Action from cloudbot.plugin import Plugin, Hook +from cloudbot.util import database + +database.metadata = MetaData() +Hook.original_init = Hook.__init__ + +PLUGINS = [] + + +class MockBot: + def __init__(self): + self.loop = None def patch_hook_init(self, _type, plugin, func_hook): self.original_init(_type, plugin, func_hook) + self.func_hook = func_hook + - assert not func_hook.kwargs, \ - "Unknown arguments '{}' passed during registration of hook '{}'".format(func_hook.kwargs, self.function_name) +Hook.__init__ = patch_hook_init def gather_plugins(): @@ -25,14 +38,7 @@ def gather_plugins(): return path_list -def load_plugin(plugin_path, monkeypatch): - if not hasattr(Hook, "original_init"): - monkeypatch.setattr('cloudbot.plugin.Hook.original_init', Hook.__init__, raising=False) - - monkeypatch.setattr('cloudbot.plugin.Hook.__init__', patch_hook_init) - - monkeypatch.setattr('cloudbot.util.database.metadata', MetaData()) - +def load_plugin(plugin_path): path = Path(plugin_path) file_path = path.resolve() file_name = file_path.name @@ -47,10 +53,23 @@ def load_plugin(plugin_path, monkeypatch): return Plugin(str(file_path), file_name, title, plugin_module) +def get_plugins(): + if not PLUGINS: + PLUGINS.extend(map(load_plugin, gather_plugins())) + + return PLUGINS + + def pytest_generate_tests(metafunc): - if 'plugin_path' in metafunc.fixturenames: - paths = list(gather_plugins()) - metafunc.parametrize('plugin_path', paths, ids=list(map(str, paths))) + if 'plugin' in metafunc.fixturenames: + plugins = get_plugins() + metafunc.parametrize('plugin', plugins, ids=[plugin.title for plugin in plugins]) + elif 'hook' in metafunc.fixturenames: + plugins = get_plugins() + hooks = [hook for plugin in plugins for hook_list in plugin.hooks.values() for hook in hook_list] + metafunc.parametrize( + 'hook', hooks, ids=["{}.{}".format(hook.plugin.title, hook.function_name) for hook in hooks] + ) HOOK_ATTR_TYPES = { @@ -69,15 +88,12 @@ def pytest_generate_tests(metafunc): } -def test_plugin(plugin_path, monkeypatch): - plugin = load_plugin(plugin_path, monkeypatch) - for hooks in plugin.hooks.values(): - for hook in hooks: - _test_hook(hook) - +def test_hook_kwargs(hook): + assert not hook.func_hook.kwargs, \ + "Unknown arguments '{}' passed during registration of hook '{}'".format( + hook.func_hook.kwargs, hook.function_name + ) -def _test_hook(hook): - assert 'async' not in hook.required_args, "Use of deprecated function Event.async" for name, types in HOOK_ATTR_TYPES.items(): try: attr = getattr(hook, name) @@ -87,6 +103,33 @@ def _test_hook(hook): assert isinstance(attr, types), \ "Unexpected type '{}' for hook attribute '{}'".format(type(attr).__name__, name) + +def test_hook_doc(hook): if hook.type == "command" and hook.doc: assert hook.doc[:1] not in "." + string.ascii_letters, \ "Invalid docstring '{}' format for command hook".format(hook.doc) + + +def test_hook_args(hook): + assert 'async' not in hook.required_args, "Use of deprecated function Event.async" + + bot = MockBot() + if hook.type in ("irc_raw", "perm_check", "periodic", "on_start", "on_stop", "event", "on_connect"): + event = Event(bot=bot) + elif hook.type == "command": + event = CommandEvent(bot=bot, hook=hook, text="", triggered_command="") + elif hook.type == "regex": + event = RegexEvent(bot=bot, hook=hook, match=None) + elif hook.type.startswith("on_cap"): + event = CapEvent(bot=bot, cap="") + elif hook.type == "post_hook": + event = PostHookEvent(bot=bot) + elif hook.type == "irc_out": + event = IrcOutEvent(bot=bot) + elif hook.type == "sieve": + return + else: + assert False, "Unhandled hook type '{}' in tests".format(hook.type) + + for arg in hook.required_args: + assert hasattr(event, arg), "Undefined parameter '{}' for hook function".format(arg) From 55b1803ebd866dc21b2b8110f0fe534e28827bc7 Mon Sep 17 00:00:00 2001 From: linuxdaemon Date: Wed, 13 Dec 2017 23:18:32 -0600 Subject: [PATCH 5/7] Switch docstring test to use a RegExp --- tests/core_tests/test_plugin_hooks.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/core_tests/test_plugin_hooks.py b/tests/core_tests/test_plugin_hooks.py index 33dbf3318..84baf8350 100644 --- a/tests/core_tests/test_plugin_hooks.py +++ b/tests/core_tests/test_plugin_hooks.py @@ -2,7 +2,7 @@ Validates all hook registrations in all plugins """ import importlib -import string +import re from numbers import Number from pathlib import Path @@ -16,6 +16,7 @@ database.metadata = MetaData() Hook.original_init = Hook.__init__ +DOC_RE = re.compile(r"^(?:(?:<.+?>|{.+?}|\[.+?\]).+?)*?-\s.+$") PLUGINS = [] @@ -106,7 +107,7 @@ def test_hook_kwargs(hook): def test_hook_doc(hook): if hook.type == "command" and hook.doc: - assert hook.doc[:1] not in "." + string.ascii_letters, \ + assert DOC_RE.match(hook.doc), \ "Invalid docstring '{}' format for command hook".format(hook.doc) From 237b784f35ebc734255f86a575cc9eda8cf6efd7 Mon Sep 17 00:00:00 2001 From: linuxdaemon Date: Wed, 13 Dec 2017 23:26:30 -0600 Subject: [PATCH 6/7] Fix docstrings to satisfy the new tests --- plugins/admin_bot.py | 2 +- plugins/cryptocurrency.py | 2 +- plugins/drinks.py | 2 +- plugins/geoip.py | 2 +- plugins/piglatin.py | 4 ++-- plugins/profile.py | 6 +++--- plugins/time_plugin.py | 4 ++-- plugins/wyr.py | 2 +- 8 files changed, 12 insertions(+), 12 deletions(-) diff --git a/plugins/admin_bot.py b/plugins/admin_bot.py index c692c687c..11b316d5a 100644 --- a/plugins/admin_bot.py +++ b/plugins/admin_bot.py @@ -369,7 +369,7 @@ def me(text, conn, chan, message, nick, admin_log): @asyncio.coroutine @hook.command(autohelp=False, permissions=["botcontrol"]) def listchans(conn, chan, message, notice): - """-- Lists the current channels the bot is in""" + """- Lists the current channels the bot is in""" chans = ', '.join(sorted(conn.channels, key=lambda x: x.strip('#').lower())) lines = formatting.chunk_str("I am currently in: {}".format(chans)) for line in lines: diff --git a/plugins/cryptocurrency.py b/plugins/cryptocurrency.py index dfd309170..8219601be 100644 --- a/plugins/cryptocurrency.py +++ b/plugins/cryptocurrency.py @@ -67,7 +67,7 @@ def init_aliases(): # main command @hook.command("crypto", "cryptocurrency") def crypto_command(text, reply): - """ [currency] -- Returns current value of a cryptocurrency """ + """ [currency] - Returns current value of a cryptocurrency""" args = text.split() ticker = args.pop(0) diff --git a/plugins/drinks.py b/plugins/drinks.py index 931810f9f..99d8db1b6 100644 --- a/plugins/drinks.py +++ b/plugins/drinks.py @@ -15,7 +15,7 @@ def load_drinks(bot): @hook.command() def drink(text, chan, action): - """, makes the user a random cocktail.""" + """ - makes the user a random cocktail.""" index = random.randint(0,len(drinks)-1) drink = drinks[index]['title'] url = web.try_shorten(drinks[index]['url']) diff --git a/plugins/geoip.py b/plugins/geoip.py index 2d8047ab0..a2334ba8e 100644 --- a/plugins/geoip.py +++ b/plugins/geoip.py @@ -76,7 +76,7 @@ def load_geoip(loop): @asyncio.coroutine @hook.command def geoip(text, reply, loop): - """ geoip -- Looks up the physical location of using Maxmind GeoLite """ + """ - Looks up the physical location of using Maxmind GeoLite """ global geoip_reader if not geoip_reader: diff --git a/plugins/piglatin.py b/plugins/piglatin.py index 43fee109a..9db9a98dd 100644 --- a/plugins/piglatin.py +++ b/plugins/piglatin.py @@ -67,7 +67,7 @@ def load_nltk(): @hook.command("pig", "piglatin") def piglatin(text): - """ pig -- Converts to pig latin. """ + """ - Converts to pig latin.""" global pronunciations if not pronunciations: return "Please wait, getting NLTK ready!" @@ -94,4 +94,4 @@ def piglatin(text): if text.isupper(): return " ".join(words).upper() else: - return " ".join(words) \ No newline at end of file + return " ".join(words) diff --git a/plugins/profile.py b/plugins/profile.py index 2c3da4815..2373f5f01 100644 --- a/plugins/profile.py +++ b/plugins/profile.py @@ -87,7 +87,7 @@ def moreprofile(text, chan, nick, notice): @hook.command() def profile(text, chan, notice, nick): - """ [category] Returns a user's saved profile data from \"\", or lists all available profile categories for the user if no category specified""" + """ [category] - Returns a user's saved profile data from \"\", or lists all available profile categories for the user if no category specified""" chan_cf = chan.casefold() nick_cf = nick.casefold() @@ -133,7 +133,7 @@ def profile(text, chan, notice, nick): @hook.command() def profileadd(text, chan, nick, notice, db): - """ Adds data to your profile in the current channel under \"\"""" + """ - Adds data to your profile in the current channel under \"\"""" if nick.casefold() == chan.casefold(): return "Profile data can not be set outside of channels" @@ -163,7 +163,7 @@ def profileadd(text, chan, nick, notice, db): @hook.command() def profiledel(nick, chan, text, notice, db): - """ Deletes \"\" from a user's profile""" + """ - Deletes \"\" from a user's profile""" if nick.casefold() == chan.casefold(): return "Profile data can not be set outside of channels" diff --git a/plugins/time_plugin.py b/plugins/time_plugin.py index 76366d898..8f2a990f9 100644 --- a/plugins/time_plugin.py +++ b/plugins/time_plugin.py @@ -44,7 +44,7 @@ def load_key(bot): @hook.command("time") def time_command(text, reply): - """ -- Gets the current time in .""" + """ - Gets the current time in .""" if not dev_key: return "This command requires a Google Developers Console API key." @@ -110,7 +110,7 @@ def time_command(text, reply): @hook.command(autohelp=False) def beats(text): - """ -- Gets the current time in .beats (Swatch Internet Time). """ + """- Gets the current time in .beats (Swatch Internet Time).""" if text.lower() == "wut": return "Instead of hours and minutes, the mean solar day is divided " \ diff --git a/plugins/wyr.py b/plugins/wyr.py index 9f1ebf8e7..58148e148 100644 --- a/plugins/wyr.py +++ b/plugins/wyr.py @@ -46,7 +46,7 @@ def get_wyr(headers): @hook.command("wyr", "wouldyourather", autohelp=False) def wyr(bot): - """ -- What would you rather do? """ + """- What would you rather do?""" headers = {"User-Agent": bot.user_agent} # keep trying to get entries until we find one that is not filtered From c2d8470fb3da01392c9d74f34c13df1b017d9116 Mon Sep 17 00:00:00 2001 From: linuxdaemon Date: Thu, 14 Dec 2017 11:15:21 -0600 Subject: [PATCH 7/7] Refactor json tests --- travis/test_json.py | 34 +++++++++++----------------------- 1 file changed, 11 insertions(+), 23 deletions(-) diff --git a/travis/test_json.py b/travis/test_json.py index 496181eb3..68b8dd6ad 100644 --- a/travis/test_json.py +++ b/travis/test_json.py @@ -3,33 +3,21 @@ Test JSON files for errors. """ -import codecs -import fnmatch import json -import os -import sys from collections import OrderedDict +from pathlib import Path -exit_code = 0 -print("Travis: Testing all JSON files in source") -for root, dirs, files in os.walk('.'): - for filename in fnmatch.filter(files, '*.json'): - file = os.path.join(root, filename) - with codecs.open(file, encoding="utf-8") as f: - text = f.read() +def pytest_generate_tests(metafunc): + if 'json_file' in metafunc.fixturenames: + paths = list(Path().rglob("*.json")) + metafunc.parametrize('json_file', paths, ids=list(map(str, paths))) - try: - data = json.loads(text, object_pairs_hook=OrderedDict) - except Exception as e: - exit_code |= 1 - print("Travis: {} is not a valid JSON file, json.load threw exception:\n{}".format(file, e)) - else: - formatted_text = json.dumps(data, indent=4) + '\n' - if text != formatted_text: - exit_code |= 2 - print("Travis: {} is not a properly formatted JSON file".format(file)) +def test_json(json_file): + with json_file.open(encoding="utf-8") as f: + text = f.read() -if exit_code != 0: - sys.exit(exit_code) + data = json.loads(text, object_pairs_hook=OrderedDict) + formatted_text = json.dumps(data, indent=4) + '\n' + assert formatted_text == text, "Improperly formatted JSON file"