From 24049ea900c5d8b530e7fcf38f503a8170ab99c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Suszy=C5=84ski=20Krzysztof?= Date: Fri, 28 Jul 2017 18:33:41 +0200 Subject: [PATCH] Fixing issues with test, container and py27 vs py36 --- puppeter/container.py | 198 ++++++++++-------- puppeter/domain/model/installer.py | 5 +- .../persistence/service/commandscollector.py | 1 + puppeter/persistence/service/java.py | 1 + puppeter/persistence/service/os.py | 7 +- puppeter/presentation/__init__.py | 5 + puppeter/presentation/answersprocessor.py | 7 +- puppeter/presentation/app.py | 21 +- puppeter/presentation/appfactory.py | 14 -- puppeter/presentation/cmdparser.py | 35 +--- puppeter/presentation/interactiveapp.py | 2 + puppeter/presentation/unattendedapp.py | 6 +- setup.py | 3 +- .../gateway/example-answers-simple.yaml | 3 + tests/persistence/gateway/test_answers.py | 2 +- tests/presentation/test_appfactory.py | 23 -- tests/presentation/test_cmdparser.py | 95 +++++---- tests/presentation/test_unattendedapp.py | 5 +- tox.ini | 19 +- 19 files changed, 236 insertions(+), 216 deletions(-) delete mode 100644 puppeter/presentation/appfactory.py create mode 100644 tests/persistence/gateway/example-answers-simple.yaml delete mode 100644 tests/presentation/test_appfactory.py diff --git a/puppeter/container.py b/puppeter/container.py index 455c09b..af2ab38 100644 --- a/puppeter/container.py +++ b/puppeter/container.py @@ -28,13 +28,10 @@ def original_cls(): pass -NamedClass = Union[Type[T], Callable[[Any, Any], T], NamedBean] - - def Named(bean_name): def named_decorator(cls): - class __NamedBean(NamedBean, cls): + class NamedBeanImpl(NamedBean, cls): def __init__(self, *args, **kwargs): self.wrapped = cls(*args, **kwargs) @@ -51,93 +48,17 @@ def __repr__(self): def __getattr__(self, name): return getattr(self.wrapped, name) - return __NamedBean + return NamedBeanImpl return named_decorator -def bind(cls, impl_cls): - # type: (Type[T], Type[T]) -> None - beans = __get_all_beans(cls) - beans.append(__Bean(cls, impl_cls=impl_cls)) - - -def bind_to_instance(cls, impl): - # type: (Type[T], T) -> None - beans = __get_all_beans(cls) - beans.append(__Bean(cls, impl=impl)) - - -def get_all(cls, *args, **kwargs): - # type: (Type[T], Any, Any) -> Sequence[T] - beans = __get_all_beans(cls) - try: - return __instantinate_beans(beans, *args, **kwargs) - except Exception: - return tuple() - - -def get(cls, *args, **kwargs): - # type: (Type[T], Any, Any) -> T - beans = __get_all_beans(cls) - if len(beans) == 1: - return beans[0].impl(*args, **kwargs) - else: - impls = list(map(lambda bean: str(bean.impl_cls()), beans)) - raise ValueError('Zero or more then one implementation found for class %s. ' - 'Found those implementations: %s. ' - 'Use @Named beans and get_named() function!' % (cls, impls)) - - -def get_named(cls, bean_name, *args, **kwargs): - # type: (Type[T], str, Any, Any) -> T - for bean in __get_all_beans(cls): - if bean.name() == bean_name: - return bean.impl(*args, **kwargs) - raise ValueError('Bean named %s has not been found for class %s' % (bean_name, cls)) - - -def get_all_with_name_starting_with(cls, name_prefix, *args, **kwargs): - # type: (Type[T], str, Any, Any) -> Sequence[T] - beans = [] - for bean in __get_all_beans(cls): - name = bean.name() - if name is not None and name.startswith(name_prefix): - beans.append(bean) - return __instantinate_beans(beans, *args, **kwargs) - - -def get_bean(cls): - # type: (Type[T]) -> __Bean[T] - return __get_all_beans(cls)[0] - - -__ROOT_DIR = dirname(dirname(__file__)) -__beans = {} # type: Dict[Type, MutableSequence[__Bean]] - - -def __instantinate_beans(beans, *args, **kwargs): - # type: (Sequence[__Bean[T]], Any, Any) -> Sequence[T] - impls = tuple(map(lambda bean: bean.impl(*args, **kwargs), beans)) # type: Sequence[T] - return impls - - -def __get_all_beans(cls): - # type: (Type[T]) -> MutableSequence[__Bean[T]] - try: - return __beans[cls] - except KeyError: - lst = [] # type: MutableSequence[__Bean[T]] - __beans[cls] = lst - return lst - - -class __Bean(Generic[T]): +class Bean(Generic[T]): def __init__(self, cls, impl=None, impl_cls=None): if impl is None and impl_cls is None: raise Exception('20170707:164636') self.__cls = cls # type: Type[T] self.__impl = impl # type: T - self.__impl_cls = impl_cls # type: NamedClass[T] + self.__impl_cls = impl_cls # type: Union[Type[T], Callable[[Any, Any], T], NamedBean] def __repr__(self): name = self.name() @@ -147,7 +68,7 @@ def __repr__(self): return 'Bean of %s' % repr(self.impl_cls()) def impl_cls(self): - # type: () -> NamedClass[T] + # type: () -> Union[Type[T], Callable[[Any, Any], T], NamedBean] if self.__impl is None and self.__impl_cls is not None: return self.__impl_cls else: @@ -169,11 +90,118 @@ def impl(self, *args, **kwargs): return self.__impl +class Container: + + def __init__(self): + self.__beans = {} # type: Dict[Type, MutableSequence[Bean]] + + def bind(self, cls, impl_cls): + # type: (Type[T], Type[T]) -> None + beans = self.__get_all_beans(cls) + beans.append(Bean(cls, impl_cls=impl_cls)) + + def bind_to_instance(self, cls, impl): + # type: (Type[T], T) -> None + beans = self.__get_all_beans(cls) + beans.append(Bean(cls, impl=impl)) + + def get_all(self, cls, *args, **kwargs): + # type: (Type[T], Any, Any) -> Sequence[T] + beans = self.__get_all_beans(cls) + try: + return self.__instantinate_beans(beans, *args, **kwargs) + except Exception: + return tuple() + + def get(self, cls, *args, **kwargs): + # type: (Type[T], Any, Any) -> T + beans = self.__get_all_beans(cls) + if len(beans) == 1: + return beans[0].impl(*args, **kwargs) + else: + impls = list(map(lambda bean: str(bean.impl_cls()), beans)) + raise ValueError('Zero or more then one implementation found for class %s. ' + 'Found those implementations: %s. ' + 'Use @Named beans and get_named() function!' % (cls, impls)) + + def get_named(self, cls, bean_name, *args, **kwargs): + # type: (Type[T], str, Any, Any) -> T + for bean in self.__get_all_beans(cls): + if bean.name() == bean_name: + return bean.impl(*args, **kwargs) + raise ValueError('Bean named %s has not been found for class %s' % (bean_name, cls)) + + def get_all_with_name_starting_with(self, cls, name_prefix, *args, **kwargs): + # type: (Type[T], str, Any, Any) -> Sequence[T] + beans = [] + for bean in self.__get_all_beans(cls): + name = bean.name() + if name is not None and name.startswith(name_prefix): + beans.append(bean) + return self.__instantinate_beans(beans, *args, **kwargs) + + def get_bean(self, cls): + # type: (Type[T]) -> Bean[T] + return self.__get_all_beans(cls)[0] + + def __instantinate_beans(self, beans, *args, **kwargs): + # type: (Sequence[Bean[T]], Any, Any) -> Sequence[T] + impls = tuple(map(lambda bean: bean.impl(*args, **kwargs), beans)) # type: Sequence[T] + return impls + + def __get_all_beans(self, cls): + # type: (Type[T]) -> MutableSequence[Bean[T]] + try: + return self.__beans[cls] + except KeyError: + lst = [] # type: MutableSequence[Bean[T]] + self.__beans[cls] = lst + return lst + + +app_container = Container() + + +def bind(cls, impl_cls): + # type: (Type[T], Type[T]) -> None + app_container.bind(cls, impl_cls) + + +def bind_to_instance(cls, impl): + # type: (Type[T], T) -> None + app_container.bind_to_instance(cls, impl) + + +def get_all(cls, *args, **kwargs): + # type: (Type[T], Any, Any) -> Sequence[T] + return app_container.get_all(cls, *args, **kwargs) + + +def get(cls, *args, **kwargs): + # type: (Type[T], Any, Any) -> T + return app_container.get(cls, *args, **kwargs) + + +def get_named(cls, bean_name, *args, **kwargs): + # type: (Type[T], str, Any, Any) -> T + return app_container.get_named(cls, bean_name, *args, **kwargs) + + +def get_all_with_name_starting_with(cls, name_prefix, *args, **kwargs): + # type: (Type[T], str, Any, Any) -> Sequence[T] + return app_container.get_all_with_name_starting_with(cls, name_prefix, *args, **kwargs) + + +def get_bean(cls): + return app_container.get_bean(cls) + + def __load_modules(module_name): import os from os.path import join, abspath, isdir, exists + rootdir = dirname(dirname(__file__)) - search = join(abspath(__ROOT_DIR), module_name.replace('.', os.sep)) + search = join(abspath(rootdir), module_name.replace('.', os.sep)) lst = os.listdir(search) modules = [] for d in lst: diff --git a/puppeter/domain/model/installer.py b/puppeter/domain/model/installer.py index b9f60a8..6f0f8f5 100644 --- a/puppeter/domain/model/installer.py +++ b/puppeter/domain/model/installer.py @@ -55,6 +55,9 @@ def is_after_4x(self): @Named('gem') class RubygemsInstaller(Installer): + def is_after_4x(self): + return True + def __init__(self): Installer.__init__(self) self.__version = None @@ -202,7 +205,7 @@ def raw_options(self): def read_raw_options(self, options): try: new_args = options['jvm_args'] - self.__args.clear() + del self.__args[:] self.__args.extend(new_args) except KeyError: pass diff --git a/puppeter/persistence/service/commandscollector.py b/puppeter/persistence/service/commandscollector.py index 62c932b..23889dc 100644 --- a/puppeter/persistence/service/commandscollector.py +++ b/puppeter/persistence/service/commandscollector.py @@ -1,3 +1,4 @@ +from __future__ import absolute_import import os import re import pkg_resources diff --git a/puppeter/persistence/service/java.py b/puppeter/persistence/service/java.py index 0ffb79d..a44a5d7 100644 --- a/puppeter/persistence/service/java.py +++ b/puppeter/persistence/service/java.py @@ -1,3 +1,4 @@ +from __future__ import absolute_import import re import subprocess diff --git a/puppeter/persistence/service/os.py b/puppeter/persistence/service/os.py index 42ac9ed..1c0b914 100644 --- a/puppeter/persistence/service/os.py +++ b/puppeter/persistence/service/os.py @@ -1,10 +1,9 @@ +from __future__ import absolute_import import os +import distro import platform from typing import Sequence, Callable -import distro -from os.path import isfile - import puppeter.settings from puppeter.domain.facter import Facter from puppeter.domain.model.osfacts import OsFamily, OperatingSystem, \ @@ -71,7 +70,7 @@ def __calculate_docker_bool(): def __is_readable(file): - return isfile(file) and os.access(file, os.R_OK) + return os.path.isfile(file) and os.access(file, os.R_OK) def __any(seq, predicate): diff --git a/puppeter/presentation/__init__.py b/puppeter/presentation/__init__.py index a7417a5..5fe9916 100644 --- a/puppeter/presentation/__init__.py +++ b/puppeter/presentation/__init__.py @@ -1,5 +1,10 @@ from puppeter import container from puppeter.domain.gateway.answers import AnswersProcessor from puppeter.presentation.answersprocessor import AnswersProcessorImpl +from puppeter.presentation.app import App +from puppeter.presentation.interactiveapp import InteractiveApp +from puppeter.presentation.unattendedapp import UnattendedApp container.bind(AnswersProcessor, AnswersProcessorImpl) +container.bind(App, InteractiveApp) +container.bind(App, UnattendedApp) diff --git a/puppeter/presentation/answersprocessor.py b/puppeter/presentation/answersprocessor.py index 929fcfe..0ad94e7 100644 --- a/puppeter/presentation/answersprocessor.py +++ b/puppeter/presentation/answersprocessor.py @@ -1,6 +1,8 @@ from argparse import Namespace from typing import Sequence, List +from pip._vendor.packaging.markers import Op + import puppeter from puppeter import container from puppeter.domain.facter import Facter @@ -9,12 +11,13 @@ from puppeter.domain.model import osfacts from puppeter.domain.model.answers import Answers from puppeter.domain.model.configurer import Configurer +from puppeter.presentation.app import Options class AnswersProcessorImpl(AnswersProcessor): def __init__(self, options): - self.options = options # type: Namespace + self.options = options # type: Options self.__log = puppeter.get_logger(AnswersProcessorImpl) def process(self, answers): @@ -22,7 +25,7 @@ def process(self, answers): configurers.extend(self.__perform_installation(answers)) commands = self.__collect_commands(configurers) for line in commands: - if self.options.execute: + if self.options.execute(): self.__log.warning('EXECUTING: %s', line) else: print(line) diff --git a/puppeter/presentation/app.py b/puppeter/presentation/app.py index 0050882..ba44045 100644 --- a/puppeter/presentation/app.py +++ b/puppeter/presentation/app.py @@ -4,6 +4,7 @@ from abc import abstractmethod, ABCMeta from logging import StreamHandler from logging.handlers import SysLogHandler +from typing import IO, Any from six import with_metaclass @@ -13,7 +14,25 @@ from puppeter.domain.gateway.answers import AnswersProcessor from puppeter.domain.model.answers import Answers from puppeter.domain.model.osfacts import OsFamily -from puppeter.presentation.cmdparser import Options + + +class Options: + def __init__(self, namespace): + self.__answers = namespace.answers # type: IO[Any] + self.__verbose = namespace.verbose # type: int + self.__execute = namespace.execute # type: bool + + def answers(self): + # type: () -> IO[Any] + return self.__answers + + def verbose(self): + # type: () -> int + return self.__verbose + + def execute(self): + # type: () -> bool + return self.__execute class App(with_metaclass(ABCMeta, object)): diff --git a/puppeter/presentation/appfactory.py b/puppeter/presentation/appfactory.py deleted file mode 100644 index 78ce843..0000000 --- a/puppeter/presentation/appfactory.py +++ /dev/null @@ -1,14 +0,0 @@ -from puppeter.presentation.app import App -from puppeter.presentation.cmdparser import Options -from puppeter.presentation.interactiveapp import InteractiveApp -from puppeter.presentation.unattendedapp import UnattendedApp - - -class AppFactory: - def interactive(self, options): - # type: (Options) -> App - return InteractiveApp(options) - - def unattended(self, options): - # type: (Options) -> App - return UnattendedApp(options) diff --git a/puppeter/presentation/cmdparser.py b/puppeter/presentation/cmdparser.py index 982c36c..19479a1 100644 --- a/puppeter/presentation/cmdparser.py +++ b/puppeter/presentation/cmdparser.py @@ -1,9 +1,10 @@ import argparse import sys -from typing import IO, Any import puppeter -from puppeter.presentation.appfactory import AppFactory +from puppeter import container +from puppeter.presentation import App +from puppeter.presentation.app import Options class _VersionAction(argparse.Action): @@ -39,33 +40,14 @@ def print_help(self, file=None): self._print_message(self.format_help(), file) -class Options: - def __init__(self, namespace): - self.__answers = namespace.answers # type: IO[Any] - self.__verbose = namespace.verbose # type: int - self.__execute = namespace.execute # type: bool - - def answers(self): - # type: () -> IO[Any] - return self.__answers - - def verbose(self): - # type: () -> int - return self.__verbose - - def execute(self): - # type: () -> bool - return self.__execute - - class CommandLineParser(object): """CommandLineParser for Puppeter""" - def __init__(self, argv, appfactory=AppFactory()): + def __init__(self, argv): super(CommandLineParser, self).__init__() self.__argv = argv[1:] - self.__appfactory = appfactory def parse(self): + # type: () -> App parser = StdErrArgumentParser(prog='puppeter', description='Puppeter - an automatic puppet installer', epilog='By default interactive setup is performed and chosen values can be saved' ' to answer file.') @@ -80,8 +62,5 @@ def parse(self): parsed = parser.parse_args(self.__argv) options = Options(parsed) - if options.answers() is None: - app = self.__appfactory.interactive(options) - else: - app = self.__appfactory.unattended(options) - return app + apptype = 'interactive' if options.answers() is None else 'unattended' + return container.get_named(App, apptype, options=options) diff --git a/puppeter/presentation/interactiveapp.py b/puppeter/presentation/interactiveapp.py index b606f56..9beb9d2 100644 --- a/puppeter/presentation/interactiveapp.py +++ b/puppeter/presentation/interactiveapp.py @@ -1,6 +1,8 @@ +from puppeter.container import Named from puppeter.presentation.app import App +@Named('interactive') class InteractiveApp(App): def __init__(self, options): diff --git a/puppeter/presentation/unattendedapp.py b/puppeter/presentation/unattendedapp.py index e4b81a8..f21916d 100644 --- a/puppeter/presentation/unattendedapp.py +++ b/puppeter/presentation/unattendedapp.py @@ -1,14 +1,16 @@ import puppeter from puppeter import container +from puppeter.container import Named from puppeter.domain.gateway.answers import AnswersGateway from puppeter.domain.model.answers import Answers from puppeter.presentation.app import App +@Named('unattended') class UnattendedApp(App): - def __init__(self, parsed): - App.__init__(self, parsed) + def __init__(self, options): + App.__init__(self, options) self.__log = puppeter.get_logger(UnattendedApp) def _collect_answers(self): diff --git a/setup.py b/setup.py index fffbf5c..954f101 100644 --- a/setup.py +++ b/setup.py @@ -97,7 +97,8 @@ def find_version(*file_paths): 'six', 'ruamel.yaml<0.15', 'distro', - 'enum34' + 'enum34', + 'typing' ], # List additional groups of dependencies here (e.g. development diff --git a/tests/persistence/gateway/example-answers-simple.yaml b/tests/persistence/gateway/example-answers-simple.yaml new file mode 100644 index 0000000..b3ab628 --- /dev/null +++ b/tests/persistence/gateway/example-answers-simple.yaml @@ -0,0 +1,3 @@ +installer: + mode: Server + type: pc4x diff --git a/tests/persistence/gateway/test_answers.py b/tests/persistence/gateway/test_answers.py index 09d2039..de6b75c 100644 --- a/tests/persistence/gateway/test_answers.py +++ b/tests/persistence/gateway/test_answers.py @@ -5,7 +5,7 @@ from puppeter.domain.model.installer import Collection4xInstaller, Mode from puppeter.persistence.gateway.answers import YamlAnswersGateway -EXAMPLE_ANSWERS = open(join(dirname(__file__), 'example-answers.yaml'), mode='r').read() +EXAMPLE_ANSWERS = open(join(dirname(__file__), 'example-answers-simple.yaml'), mode='r').read() def __load(yaml_content): diff --git a/tests/presentation/test_appfactory.py b/tests/presentation/test_appfactory.py deleted file mode 100644 index e325ba3..0000000 --- a/tests/presentation/test_appfactory.py +++ /dev/null @@ -1,23 +0,0 @@ -from puppeter.presentation.appfactory import AppFactory - - -def test_app_factory_interactive(): - # given - factory = AppFactory() - # when - app = factory.interactive({'answers': None}) - runmethod = app.run - # then - assert app is not None - assert callable(runmethod) - - -def test_app_factory_unattended(): - # given - factory = AppFactory() - # when - app = factory.unattended({'answers': __file__}) - runmethod = app.run - # then - assert app is not None - assert callable(runmethod) diff --git a/tests/presentation/test_cmdparser.py b/tests/presentation/test_cmdparser.py index e9cb586..14cc149 100644 --- a/tests/presentation/test_cmdparser.py +++ b/tests/presentation/test_cmdparser.py @@ -1,45 +1,62 @@ import os + +from puppeter import container +from puppeter.container import Named, Container +from puppeter.domain.model.answers import Answers +from puppeter.presentation import App from puppeter.presentation.cmdparser import CommandLineParser HERE = os.path.abspath(__file__) -def __get_test_appfactory(): - class TestApp: - def __init__(self, arg): - self.arg = arg - - def run(self): - return 'runned %s' % self.arg - - class TestFactory: - def interactive(self, parsed): - return TestApp('interactivly') - - def unattended(self, parsed): - return TestApp('unattended') - return TestFactory() - - -def test_proper_app_creation(): - # given - factory = __get_test_appfactory() - parser = CommandLineParser(['puppeter'], appfactory=factory) - # when - app = parser.parse() - result = app.run() - # then - assert app is not None - assert result == 'runned interactivly' - - -def test_proper_app_creation_unattended(): - # given - factory = __get_test_appfactory() - parser = CommandLineParser(['puppeter', '--answers', HERE], appfactory=factory) - # when - app = parser.parse() - result = app.run() - # then - assert app is not None - assert result == 'runned unattended' +@Named('interactive') +class InteractiveTestApp(App): + def _collect_answers(self): + return Answers() + + def run(self): + return 'runned interactivly' + + +@Named('unattended') +class UnattendedTestApp(App): + def _collect_answers(self): + return Answers() + + def run(self): + return 'runned unattended' + + +class TestAppCreation: + old_container = None + + @classmethod + def setup_class(cls): + cls.old_container = container.app_container + container.app_container = Container() + container.bind(App, InteractiveTestApp) + container.bind(App, UnattendedTestApp) + + @classmethod + def teardown_class(cls): + container.app_container = cls.old_container + + def test_proper_app_creation(self): + # given + parser = CommandLineParser(['puppeter']) + # when + app = parser.parse() + result = app.run() + # then + assert app is not None + assert result == 'runned interactivly' + + def test_proper_app_creation_unattended(self): + # given + parser = CommandLineParser(['puppeter', '--answers', HERE]) + # when + app = parser.parse() + result = app.run() + # then + assert app is not None + assert result == 'runned unattended' diff --git a/tests/presentation/test_unattendedapp.py b/tests/presentation/test_unattendedapp.py index 45c58de..a0e4b67 100644 --- a/tests/presentation/test_unattendedapp.py +++ b/tests/presentation/test_unattendedapp.py @@ -3,6 +3,7 @@ from puppeter import container from puppeter.domain.model.osfacts import OperatingSystemCodename,\ OperatingSystem, OsFamily, OperatingSystemRelease +from puppeter.presentation.app import Options from puppeter.presentation.unattendedapp import UnattendedApp from tests.domain.mock_facter import MockFacter @@ -14,8 +15,8 @@ def test_unattendedapp_onlyinstaller_with_verbosity(tmpdir, capsys): ' type: pc4x\n' ' mode: Server\n') answers = open(str(tmp)) - parsed = dotdict(dict(answers=answers, verbose=2, execute=False)) - app = UnattendedApp(parsed) + options = Options(dotdict(dict(answers=answers, verbose=2, execute=False))) + app = UnattendedApp(options) # when app.run() diff --git a/tox.ini b/tox.ini index 2776066..b6a81aa 100644 --- a/tox.ini +++ b/tox.ini @@ -11,14 +11,13 @@ # and also to help confirm pull requests to this project. [tox] -envlist = py{27,34,36},flake8 +envlist = py{36,34,27} [testenv] basepython = - py27: python2.7 - py34: python3.4 py36: python3.6 - flake8: python3.6 + py34: python3.4 + py27: python2.7 deps = setuptools>=18.5 check-manifest @@ -29,20 +28,14 @@ deps = pytest-cov ruamel.yaml<0.15 distro + py36: flake8 + py36: flake8-mypy commands = check-manifest --ignore tox.ini,.python-version,.editorconfig,tests* python setup.py check -m -r -s + py36: flake8 . py.test tests --cov=puppeter --cov-report=term --cov-report=html -[testenv:flake8] -deps = - flake8 - flake8-mypy - ruamel.yaml<0.15 - distro -commands = - flake8 . - [flake8] exclude = # No need to traverse our git directory