From 9e31aa496828a6d9075d8b16b191c13daf10f79e Mon Sep 17 00:00:00 2001 From: Tyler Johnson Date: Fri, 3 Jan 2020 04:49:04 -0700 Subject: [PATCH 1/4] Added helpers for pkg tests - created requires_salt_state_module - corrected misuse of requires_salt_module to use the new function - eliminated pre_grains and loaded real grains before the test starts - Various fixes to get the pkg tests actually running as they were skipped before - Skipped tests when requisite packages weren't available --- tests/integration/modules/test_pkg.py | 53 ++-- tests/integration/states/test_pkg.py | 379 +++++++++++------------ tests/integration/states/test_pkgrepo.py | 5 +- tests/support/helpers.py | 107 ++++++- 4 files changed, 320 insertions(+), 224 deletions(-) diff --git a/tests/integration/modules/test_pkg.py b/tests/integration/modules/test_pkg.py index 913d1c74e1a2..1dfe55eee629 100644 --- a/tests/integration/modules/test_pkg.py +++ b/tests/integration/modules/test_pkg.py @@ -12,12 +12,12 @@ destructiveTest, requires_network, requires_salt_modules, + requires_salt_state_modules, requires_system_grains, skip_if_not_root) from tests.support.unit import skipIf # Import Salt libs -from salt.ext import six import salt.utils.path import salt.utils.pkg import salt.utils.platform @@ -181,37 +181,47 @@ def test_remove(): test_remove() @destructiveTest - @requires_salt_modules('pkg.hold', 'pkg.unhold', 'pkg.install', 'pkg.version', 'pkg.remove') + @requires_salt_modules('pkg.hold', 'pkg.unhold', 'pkg.install', 'pkg.version', 'pkg.remove', 'pkg.list_pkgs') + @requires_salt_state_modules('pkg.installed') @requires_network() @requires_system_grains def test_hold_unhold(self, grains): ''' test holding and unholding a package ''' - ret = None + versionlock_pkg = None if grains['os_family'] == 'RedHat': - # get correct plugin for dnf packages following the logic in `salt.modules.yumpkg._yum` - lock_pkg = 'yum-versionlock' if grains['osmajorrelease'] == '5' else 'yum-plugin-versionlock' - if 'fedora' in grains['os'].lower() and int(grains['osrelease']) >= 22: - if int(grains['osmajorrelease']) >= 26: - lock_pkg = 'python{py}-dnf-plugin-versionlock'.format(py=3 if six.PY3 else 2) - else: - lock_pkg = 'python{py}-dnf-plugins-extras-versionlock'.format(py=3 if six.PY3 else '') - ret = self.run_state('pkg.installed', name=lock_pkg) + pkgs = {p for p in self.run_function('pkg.list_pkgs') if '-versionlock' in p} + if not pkgs: + self.skipTest('No versionlock package found in repositories') + for versionlock_pkg in pkgs: + ret = self.run_state('pkg.installed', name=versionlock_pkg, refresh=False) + # Exit loop if a versionlock package installed correctly + try: + self.assertSaltTrueReturn(ret) + break + except AssertionError: + pass + else: + self.fail('Could not install versionlock package from {}'.format(pkgs)) self.run_function('pkg.install', [self.pkg]) - hold_ret = self.run_function('pkg.hold', [self.pkg]) - if 'versionlock is not installed' in hold_ret: + try: + hold_ret = self.run_function('pkg.hold', [self.pkg]) + if versionlock_pkg and '-versionlock is not installed' in str(hold_ret): + self.skipTest(str(hold_ret) + ' `{}` is installed'.format(versionlock_pkg)) + self.assertIn(self.pkg, hold_ret) + self.assertTrue(hold_ret[self.pkg]['result']) + + unhold_ret = self.run_function('pkg.unhold', [self.pkg]) + self.assertIn(self.pkg, unhold_ret) + self.assertTrue(unhold_ret[self.pkg]['result']) self.run_function('pkg.remove', [self.pkg]) - self.skipTest('Versionlock could not be installed on this system: {}'.format(ret)) - self.assertIn(self.pkg, hold_ret) - self.assertTrue(hold_ret[self.pkg]['result']) - - unhold_ret = self.run_function('pkg.unhold', [self.pkg]) - self.assertIn(self.pkg, unhold_ret) - self.assertTrue(unhold_ret[self.pkg]['result']) - self.run_function('pkg.remove', [self.pkg]) + finally: + if versionlock_pkg: + ret = self.run_state('pkg.removed', name=versionlock_pkg) + self.assertSaltTrueReturn(ret) @destructiveTest @requires_salt_modules('pkg.refresh_db') @@ -345,6 +355,7 @@ def test_pkg_upgrade_has_pending_upgrades(self, grains): @destructiveTest @skipIf(salt.utils.platform.is_darwin(), 'The jenkins user is equivalent to root on mac, causing the test to be unrunnable') @requires_salt_modules('pkg.remove', 'pkg.latest_version') + @requires_salt_state_modules('pkg.removed') @requires_system_grains def test_pkg_latest_version(self, grains): ''' diff --git a/tests/integration/states/test_pkg.py b/tests/integration/states/test_pkg.py index ed42e0c91c65..c5b059afe7e8 100644 --- a/tests/integration/states/test_pkg.py +++ b/tests/integration/states/test_pkg.py @@ -5,7 +5,6 @@ ''' # Import Python libs from __future__ import absolute_import, print_function, unicode_literals -import functools import logging import os import time @@ -17,7 +16,10 @@ from tests.support.helpers import ( destructiveTest, requires_salt_modules, -) + requires_salt_state_modules, + requires_system_grains, + runs_on, + not_runs_on) # Import Salt libs import salt.utils.files @@ -29,73 +31,50 @@ from salt.ext import six from salt.ext.six.moves import range # pylint: disable=import-error,redefined-builtin -try: - from distro import LinuxDistribution - pre_grains = LinuxDistribution() -except ImportError: - pre_grains = None - log = logging.getLogger(__name__) -_PKG_EPOCH_TARGETS = [] -_PKG_TARGETS = ['figlet', 'sl'] -_PKG_32_TARGETS = [] -_PKG_CAP_TARGETS = [] -_PKG_DOT_TARGETS = [] -_WILDCARDS_SUPPORTED = False -_VERSION_SPEC_SUPPORTED = True - -if salt.utils.platform.is_windows(): - _PKG_TARGETS = ['7zip', 'putty'] -elif salt.utils.platform.is_freebsd: - _VERSION_SPEC_SUPPORTED = False -elif pre_grains: - if any(arch in pre_grains.like() for arch in ('arch', 'archlinux')): - _WILDCARDS_SUPPORTED = True - elif 'debian' in pre_grains.like(): - _WILDCARDS_SUPPORTED = True - elif 'rhel' in pre_grains.like(): - _PKG_TARGETS = ['units', 'zsh-html'] - _WILDCARDS_SUPPORTED = True - if pre_grains.id() == 'centos': - if pre_grains.major_version() == 5: - _PKG_32_TARGETS = ['xz-devel.i386'] - else: - _PKG_32_TARGETS.append('xz-devel.i686') - if pre_grains.major_version() == 5: - _PKG_DOT_TARGETS = ['python-migrate0.5'] - elif pre_grains.major_version() == 6: - _PKG_DOT_TARGETS = ['tomcat6-el-2.1-api'] - elif pre_grains.major_version() == 7: - _PKG_DOT_TARGETS = ['tomcat-el-2.2-api'] - _PKG_EPOCH_TARGETS = ['comps-extras'] - elif pre_grains.id() in ('sles', 'opensuse'): - _PKG_TARGETS = ['figlet', 'htop'] - _PKG_CAP_TARGETS = [('perl(ZNC)', 'znc-perl')] - - -def runs_on(platforms=None, os_like=None, reason=''): - def decorator(caller): - @functools.wraps(caller) - def wrapper(cls): - if platforms: - if not any(getattr(salt.utils.platform, 'is_' + platform)() for platform in platforms): - cls.skipTest(reason if reason else 'OS not in [{}]'.format(', '.join(platforms))) - if pre_grains and os_like: - if not any(x in pre_grains.like() for x in os_like): - cls.skipTest(reason if reason else 'OS not similar to [{}]'.format(', '.join(os_like))) - return caller(cls) - - return wrapper - - return decorator - @destructiveTest class PkgTest(ModuleCase, SaltReturnAssertsMixin): + _PKG_EPOCH_TARGETS = [] + _PKG_TARGETS = ['figlet', 'sl'] + _PKG_32_TARGETS = [] + _PKG_CAP_TARGETS = [] + _PKG_DOT_TARGETS = [] + _WILDCARDS_SUPPORTED = False + _VERSION_SPEC_SUPPORTED = True + @classmethod - def setUpClass(cls): + @requires_system_grains + def setUpClass(cls, grains=None): # pylint:disable=W0221 cls.ctx = {} + if grains['os'] == 'Windows': + cls._PKG_TARGETS = ['7zip', 'putty'] + elif grains['os'] == 'freebsd': + cls._VERSION_SPEC_SUPPORTED = False + elif grains['os_family'] in ('Arch', 'Debian'): + cls._WILDCARDS_SUPPORTED = True + elif grains['os'] == 'Amazon': + cls._PKG_TARGETS = ['lynx', 'gnuplot'] + elif grains['os_family'] == 'RedHat': + cls._PKG_TARGETS = ['units', 'zsh-html'] + cls._WILDCARDS_SUPPORTED = True + if grains['os'] == 'CentOS': + if grains['osmajorrelease'] == 5: + cls._PKG_32_TARGETS = ['xz-devel.i386'] + else: + cls._PKG_32_TARGETS.append('xz-devel.i686') + if grains['osmajorrelease'] == 5: + cls._PKG_DOT_TARGETS = ['python-migrate0.5'] + elif grains['osmajorrelease'] == 6: + cls._PKG_DOT_TARGETS = ['tomcat6-el-2.1-api'] + elif grains['osmajorrelease'] == 7: + cls._PKG_DOT_TARGETS = ['tomcat-el-2.2-api'] + cls._PKG_EPOCH_TARGETS = ['comps-extras'] + elif grains['os_family'] == 'Suse': + cls._PKG_TARGETS = ['lynx', 'htop'] + if grains['os'] == 'SUSE': + cls._PKG_CAP_TARGETS = [('perl(ZNC)', 'znc-perl')] @classmethod def tearDownClass(cls): @@ -126,18 +105,30 @@ def latest_version(self, *names): return ret[names[0]] return ret - def setUp(self): + @requires_system_grains + def setUp(self, grains=None): # pylint:disable=W0221 super(PkgTest, self).setUp() if 'refresh' not in self.ctx: self.run_function('pkg.refresh_db') self.ctx['refresh'] = True - @requires_salt_modules('pkg.version', 'pkg.installed', 'pkg.removed') + # If this is Arch Linux, check if pacman is in use by another process + if grains['os_family'] == 'Arch': + for _ in range(12): + if not os.path.isfile('/var/lib/pacman/db.lck'): + break + else: + time.sleep(5) + else: + raise Exception('Package database locked after 60 seconds, bailing out') + + @requires_salt_modules('pkg.version') + @requires_salt_state_modules('pkg.installed', 'pkg.removed') def test_pkg_001_installed(self): ''' This is a destructive test as it installs and then removes a package ''' - target = _PKG_TARGETS[0] + target = self._PKG_TARGETS[0] version = self.run_function('pkg.version', [target]) # If this assert fails, we need to find new targets, this test needs to @@ -151,21 +142,12 @@ def test_pkg_001_installed(self): self.assertSaltTrueReturn(ret) @skipIf(not _VERSION_SPEC_SUPPORTED, 'Version specification not supported') - @requires_salt_modules('pkg.installed', 'pkg.removed') + @requires_salt_state_modules('pkg.installed', 'pkg.removed') def test_pkg_002_installed_with_version(self): ''' This is a destructive test as it installs and then removes a package ''' - if pre_grains and 'arch' in pre_grains.like(): - for idx in range(13): - if idx == 12: - raise Exception('Package database locked after 60 seconds, ' - 'bailing out') - if not os.path.isfile('/var/lib/pacman/db.lck'): - break - time.sleep(5) - - target = _PKG_TARGETS[0] + target = self._PKG_TARGETS[0] version = self.latest_version(target) # If this assert fails, we need to find new targets, this test needs to @@ -181,52 +163,43 @@ def test_pkg_002_installed_with_version(self): ret = self.run_state('pkg.removed', name=target) self.assertSaltTrueReturn(ret) - @requires_salt_modules('pkg.installed', 'pkg.removed') + @requires_salt_state_modules('pkg.installed', 'pkg.removed') def test_pkg_003_installed_multipkg(self): ''' This is a destructive test as it installs and then removes two packages ''' - version = self.run_function('pkg.version', _PKG_TARGETS) + version = self.run_function('pkg.version', self._PKG_TARGETS) # If this assert fails, we need to find new targets, this test needs to # be able to test successful installation of packages, so these # packages need to not be installed before we run the states below self.assertFalse(any(version.values())) - self.assertSaltTrueReturn(self.run_state('pkg.removed', name=None, pkgs=_PKG_TARGETS)) + self.assertSaltTrueReturn(self.run_state('pkg.removed', name=None, pkgs=self._PKG_TARGETS)) try: ret = self.run_state('pkg.installed', name=None, - pkgs=_PKG_TARGETS, + pkgs=self._PKG_TARGETS, refresh=False) self.assertSaltTrueReturn(ret) finally: - ret = self.run_state('pkg.removed', name=None, pkgs=_PKG_TARGETS) + ret = self.run_state('pkg.removed', name=None, pkgs=self._PKG_TARGETS) self.assertSaltTrueReturn(ret) @skipIf(not _VERSION_SPEC_SUPPORTED, 'Version specification not supported') - @requires_salt_modules('pkg.installed', 'pkg.removed') + @requires_salt_state_modules('pkg.installed', 'pkg.removed') def test_pkg_004_installed_multipkg_with_version(self): ''' This is a destructive test as it installs and then removes two packages ''' - if pre_grains and 'arch' in pre_grains.like(): - for idx in range(13): - if idx == 12: - raise Exception('Package database locked after 60 seconds, ' - 'bailing out') - if not os.path.isfile('/var/lib/pacman/db.lck'): - break - time.sleep(5) - - version = self.latest_version(_PKG_TARGETS[0]) + version = self.latest_version(self._PKG_TARGETS[0]) # If this assert fails, we need to find new targets, this test needs to # be able to test successful installation of packages, so these # packages need to not be installed before we run the states below self.assertTrue(bool(version)) - pkgs = [{_PKG_TARGETS[0]: version}, _PKG_TARGETS[1]] + pkgs = [{self._PKG_TARGETS[0]: version}, self._PKG_TARGETS[1]] try: ret = self.run_state('pkg.installed', @@ -235,16 +208,17 @@ def test_pkg_004_installed_multipkg_with_version(self): refresh=False) self.assertSaltTrueReturn(ret) finally: - ret = self.run_state('pkg.removed', name=None, pkgs=_PKG_TARGETS) + ret = self.run_state('pkg.removed', name=None, pkgs=self._PKG_TARGETS) self.assertSaltTrueReturn(ret) @skipIf(not _PKG_32_TARGETS, 'No 32 bit packages have been specified for testing') - @requires_salt_modules('pkg.version', 'pkg.installed', 'pkg.removed') + @requires_salt_modules('pkg.version') + @requires_salt_state_modules('pkg.installed', 'pkg.removed') def test_pkg_005_installed_32bit(self): ''' This is a destructive test as it installs and then removes a package ''' - target = _PKG_32_TARGETS[0] + target = self._PKG_32_TARGETS[0] # _PKG_TARGETS_32 is only populated for platforms for which Salt has to # munge package names for 32-bit-on-x86_64 (Currently only Ubuntu and @@ -265,25 +239,16 @@ def test_pkg_005_installed_32bit(self): self.assertSaltTrueReturn(ret) @skipIf(not _PKG_32_TARGETS, 'No 32 bit packages have been specified for testing') - @requires_salt_modules('pkg.installed', 'pkg.removed') + @requires_salt_state_modules('pkg.installed', 'pkg.removed') def test_pkg_006_installed_32bit_with_version(self): ''' This is a destructive test as it installs and then removes a package ''' - target = _PKG_32_TARGETS[0] + target = self._PKG_32_TARGETS[0] # _PKG_TARGETS_32 is only populated for platforms for which Salt has to # munge package names for 32-bit-on-x86_64 (Currently only Ubuntu and # RHEL-based). Don't actually perform this test on other platforms. - if pre_grains and 'arch' in pre_grains.like(): - for idx in range(13): - if idx == 12: - raise Exception('Package database locked after 60 seconds, ' - 'bailing out') - if not os.path.isfile('/var/lib/pacman/db.lck'): - break - time.sleep(5) - version = self.latest_version(target) # If this assert fails, we need to find a new target. This test @@ -301,7 +266,7 @@ def test_pkg_006_installed_32bit_with_version(self): self.assertSaltTrueReturn(ret) @skipIf(not _PKG_DOT_TARGETS, 'No packages with "." in their name have been configured for') - @requires_salt_modules('pkg.installed', 'pkg.removed') + @requires_salt_state_modules('pkg.installed', 'pkg.removed') def test_pkg_007_with_dot_in_pkgname(self=None): ''' This tests for the regression found in the following issue: @@ -309,7 +274,7 @@ def test_pkg_007_with_dot_in_pkgname(self=None): This is a destructive test as it installs a package ''' - target = _PKG_DOT_TARGETS[0] + target = self._PKG_DOT_TARGETS[0] version = self.latest_version(target) # If this assert fails, we need to find a new target. This test @@ -323,7 +288,7 @@ def test_pkg_007_with_dot_in_pkgname(self=None): self.assertSaltTrueReturn(ret) @skipIf(not _PKG_EPOCH_TARGETS, 'No targets have been configured with "epoch" in the version') - @requires_salt_modules('pkg.installed', 'pkg.removed') + @requires_salt_state_modules('pkg.installed', 'pkg.removed') def test_pkg_008_epoch_in_version(self): ''' This tests for the regression found in the following issue: @@ -331,7 +296,7 @@ def test_pkg_008_epoch_in_version(self): This is a destructive test as it installs a package ''' - target = _PKG_EPOCH_TARGETS[0] + target = self._PKG_EPOCH_TARGETS[0] version = self.latest_version(target) # If this assert fails, we need to find a new target. This test @@ -347,8 +312,10 @@ def test_pkg_008_epoch_in_version(self): ret = self.run_state('pkg.removed', name=target) self.assertSaltTrueReturn(ret) - @requires_salt_modules('pkg.version', 'pkg.info_installed', 'pkg.installed', 'pkg.removed') - @runs_on(platforms=['linux'], reason='This test only runs on linux') + @requires_salt_modules('pkg.version', 'pkg.info_installed') + @requires_salt_state_modules('pkg.installed', 'pkg.removed') + @runs_on(kernel='linux') + @not_runs_on(os='Amazon') def test_pkg_009_latest_with_epoch(self): ''' This tests for the following issue: @@ -367,13 +334,13 @@ def test_pkg_009_latest_with_epoch(self): ret = self.run_function('pkg.info_installed', [package]) self.assertTrue(pkgquery in six.text_type(ret)) - @requires_salt_modules('pkg.latest', 'pkg.removed') + @requires_salt_state_modules('pkg.latest', 'pkg.removed') def test_pkg_010_latest(self): ''' This tests pkg.latest with a package that has no epoch (or a zero epoch). ''' - target = _PKG_TARGETS[0] + target = self._PKG_TARGETS[0] version = self.latest_version(target) # If this assert fails, we need to find new targets, this test needs to @@ -386,14 +353,15 @@ def test_pkg_010_latest(self): ret = self.run_state('pkg.removed', name=target) self.assertSaltTrueReturn(ret) - @requires_salt_modules('pkg.latest', 'pkg.list_pkgs', 'pkg.list_upgrades', 'pkg.version') - @runs_on(platforms=['linux'], os_like=['debian'], reason='This test only runs on Debian based linux distributions') + @requires_salt_modules('pkg.list_pkgs', 'pkg.list_upgrades', 'pkg.version') + @requires_salt_state_modules('pkg.latest') + @runs_on(kernel='linux', os_family='Debian') def test_pkg_011_latest_only_upgrade(self): ''' WARNING: This test will pick a package with an available upgrade (if there is one) and upgrade it to the latest version. ''' - target = _PKG_TARGETS[0] + target = self._PKG_TARGETS[0] # If this assert fails, we need to find new targets, this test needs to # be able to test that the state fails when you try to run the state @@ -437,12 +405,13 @@ def test_pkg_011_latest_only_upgrade(self): ) @skipIf(not _WILDCARDS_SUPPORTED, 'Wildcards in pkg.install are not supported') - @requires_salt_modules('pkg.version', 'pkg.installed', 'pkg.removed') + @requires_salt_modules('pkg.version') + @requires_salt_state_modules('pkg.installed', 'pkg.removed') def test_pkg_012_installed_with_wildcard_version(self): ''' This is a destructive test as it installs and then removes a package ''' - target = _PKG_TARGETS[0] + target = self._PKG_TARGETS[0] version = self.run_function('pkg.version', [target]) # If this assert fails, we need to find new targets, this test needs to @@ -486,13 +455,14 @@ def test_pkg_012_installed_with_wildcard_version(self): ret = self.run_state('pkg.removed', name=target) self.assertSaltTrueReturn(ret) - @requires_salt_modules('pkg.version', 'pkg.latest_version', 'pkg.installed', 'pkg.removed') - @runs_on(platforms=['linux'], os_like=['debian', 'redhat'], reason='Comparison operator not specially implemented') + @requires_salt_modules('pkg.version', 'pkg.latest_version') + @requires_salt_state_modules('pkg.installed', 'pkg.removed') + @runs_on(kernel='linux', os_family=['Debian', 'RedHat']) def test_pkg_013_installed_with_comparison_operator(self): ''' This is a destructive test as it installs and then removes a package ''' - target = _PKG_TARGETS[0] + target = self._PKG_TARGETS[0] version = self.run_function('pkg.version', [target]) # If this assert fails, we need to find new targets, this test needs to @@ -522,14 +492,15 @@ def test_pkg_013_installed_with_comparison_operator(self): ret = self.run_state('pkg.removed', name=target) self.assertSaltTrueReturn(ret) - @requires_salt_modules('pkg.version', 'pkg.installed', 'pkg.removed') - @runs_on(platforms=['linux'], os_like=['redhat'], reason='Comparison operator not specially implemented') + @requires_salt_modules('pkg.version') + @requires_salt_state_modules('pkg.installed', 'pkg.removed') + @runs_on(kernel='linux', os_familiy='RedHat') def test_pkg_014_installed_missing_release(self): ''' Tests that a version number missing the release portion still resolves as correctly installed. For example, version 2.0.2 instead of 2.0.2-1.el7 ''' - target = _PKG_TARGETS[0] + target = self._PKG_TARGETS[0] version = self.run_function('pkg.version', [target]) # If this assert fails, we need to find new targets, this test needs to @@ -549,23 +520,31 @@ def test_pkg_014_installed_missing_release(self): ret = self.run_state('pkg.removed', name=target) self.assertSaltTrueReturn(ret) - @requires_salt_modules('pkg.hold', 'pkg.unhold', 'pkg.installed', 'pkg.removed') - def test_pkg_015_installed_held(self): + @requires_salt_modules('pkg.hold', 'pkg.unhold', 'pkg.version', 'pkg.list_pkgs') + @requires_salt_state_modules('pkg.installed', 'pkg.removed') + @requires_system_grains + def test_pkg_015_installed_held(self, grains=None): ''' Tests that a package can be held even when the package is already installed. ''' + versionlock_pkg = None + if grains['os_family'] == 'RedHat': + pkgs = {p for p in self.run_function('pkg.list_pkgs') if '-versionlock' in p} + if not pkgs: + self.skipTest('No versionlock package found in repositories') + for versionlock_pkg in pkgs: + ret = self.run_state('pkg.installed', name=versionlock_pkg, refresh=False) + # Exit loop if a versionlock package installed correctly + try: + self.assertSaltTrueReturn(ret) + log.debug('Installed versionlock package: {}'.format(versionlock_pkg)) + break + except AssertionError as e: + log.debug('Versionlock package not found:\n{}'.format(e)) + else: + self.fail('Could not install versionlock package from {}'.format(pkgs)) - if pre_grains and 'redhat' in pre_grains.like(): - # If we're in the Red Hat family first we ensure that - # the yum-plugin-versionlock package is installed - ret = self.run_state( - 'pkg.installed', - name='yum-plugin-versionlock', - refresh=False, - ) - self.assertSaltTrueReturn(ret) - - target = _PKG_TARGETS[0] + target = self._PKG_TARGETS[0] # First we ensure that the package is installed ret = self.run_state( @@ -583,12 +562,15 @@ def test_pkg_015_installed_held(self): refresh=False, ) + if versionlock_pkg and '-versionlock is not installed' in str(ret): + self.skipTest(str(ret) + ' `{}` is installed'.format(versionlock_pkg)) + # changes from pkg.hold for Red Hat family are different - if pre_grains: - if 'redhat' in pre_grains.like(): - target_changes = {'new': 'hold', 'old': ''} - elif 'debian' in pre_grains.like(): - target_changes = {'new': 'hold', 'old': 'install'} + target_changes = {} + if grains['os_family'] == 'RedHat': + target_changes = {'new': 'hold', 'old': ''} + elif grains['os_family'] == 'Debian': + target_changes = {'new': 'hold', 'old': 'install'} try: tag = 'pkg_|-{0}_|-{0}_|-installed'.format(target) @@ -596,33 +578,36 @@ def test_pkg_015_installed_held(self): self.assertIn(tag, ret) self.assertIn('changes', ret[tag]) self.assertIn(target, ret[tag]['changes']) + if not target_changes: + self.skipTest( + 'Test needs to be configured for {}: {}'.format(grains['os'], ret[tag]['changes'][target])) self.assertEqual(ret[tag]['changes'][target], target_changes) finally: # Clean up, unhold package and remove self.run_function('pkg.unhold', name=target) ret = self.run_state('pkg.removed', name=target) self.assertSaltTrueReturn(ret) - if pre_grains and 'redhat' in pre_grains.like(): - ret = self.run_state('pkg.removed', - name='yum-plugin-versionlock') + if versionlock_pkg: + ret = self.run_state('pkg.removed', name=versionlock_pkg) self.assertSaltTrueReturn(ret) @skipIf(not _PKG_CAP_TARGETS, 'Capability not provided') - @requires_salt_modules('pkg.version', 'pkg.installed', 'pkg.removed') + @requires_salt_modules('pkg.version') + @requires_salt_state_modules('pkg.installed', 'pkg.removed') def test_pkg_cap_001_installed(self): ''' This is a destructive test as it installs and then removes a package ''' - target, realpkg = _PKG_CAP_TARGETS[0] + target, realpkg = self._PKG_CAP_TARGETS[0] version = self.run_function('pkg.version', [target]) realver = self.run_function('pkg.version', [realpkg]) - # If this assert fails, we need to find new targets, this test needs to - # be able to test successful installation of packages, so this package - # needs to not be installed before we run the states below - self.assertFalse(version) - self.assertFalse(realver) + # If this condition is False, we need to find new targets. + # This needs to be able to test successful installation of packages. + # These packages need to not be installed before we run the states below + if not (version and realver): + self.skipTest('TODO: New pkg cap targets required') try: ret = self.run_state('pkg.installed', name=target, refresh=False, resolve_capabilities=True, test=True) @@ -634,20 +619,20 @@ def test_pkg_cap_001_installed(self): self.assertSaltTrueReturn(ret) @skipIf(not _PKG_CAP_TARGETS, 'Capability not available') - @requires_salt_modules('pkg.installed', 'pkg.removed') + @requires_salt_state_modules('pkg.installed', 'pkg.removed') def test_pkg_cap_002_already_installed(self): ''' This is a destructive test as it installs and then removes a package ''' - target, realpkg = _PKG_CAP_TARGETS[0] + target, realpkg = self._PKG_CAP_TARGETS[0] version = self.run_function('pkg.version', [target]) realver = self.run_function('pkg.version', [realpkg]) - # If this assert fails, we need to find new targets, this test needs to - # be able to test successful installation of packages, so this package - # needs to not be installed before we run the states below - self.assertFalse(version) - self.assertFalse(realver) + # If this condition is False, we need to find new targets. + # This needs to be able to test successful installation of packages. + # These packages need to not be installed before we run the states below + if not (version and realver): + self.skipTest('TODO: New pkg cap targets required') try: # install the package @@ -668,32 +653,24 @@ def test_pkg_cap_002_already_installed(self): @skipIf(not _PKG_CAP_TARGETS, 'Capability not available') @skipIf(not _VERSION_SPEC_SUPPORTED, 'Version specification not supported') - @requires_salt_modules('pkg.installed', 'pkg.removed') + @requires_salt_state_modules('pkg.installed', 'pkg.removed') def test_pkg_cap_003_installed_multipkg_with_version(self): ''' This is a destructive test as it installs and then removes two packages ''' - if pre_grains and ('arch' in pre_grains.like() or 'archlinux' in pre_grains.like()): - for idx in range(13): - if idx == 12: - raise Exception('Package database locked after 60 seconds, ' - 'bailing out') - if not os.path.isfile('/var/lib/pacman/db.lck'): - break - time.sleep(5) - - target, realpkg = _PKG_CAP_TARGETS[0] + target, realpkg = self._PKG_CAP_TARGETS[0] version = self.latest_version(target) realver = self.latest_version(realpkg) - # If this assert fails, we need to find new targets, this test needs to - # be able to test successful installation of packages, so these - # packages need to not be installed before we run the states below - self.assertTrue(version, 'new pkg cap targets required') - self.assertTrue(realver, 'new pkg cap targets required') + # If this condition is False, we need to find new targets. + # This needs to be able to test successful installation of packages. + # These packages need to not be installed before we run the states below + if not (version and realver): + self.skipTest('TODO: New pkg cap targets required') + cleanup_pkgs = self._PKG_TARGETS try: - pkgs = [{_PKG_TARGETS[0]: version}, _PKG_TARGETS[1], {target: realver}] + pkgs = [{self._PKG_TARGETS[0]: version}, self._PKG_TARGETS[1], {target: realver}] ret = self.run_state('pkg.installed', name='test_pkg_cap_003_installed_multipkg_with_version-install', pkgs=pkgs, @@ -712,7 +689,6 @@ def test_pkg_cap_003_installed_multipkg_with_version(self): pkgs=pkgs, refresh=False, resolve_capabilities=True) self.assertSaltTrueReturn(ret) - cleanup_pkgs = _PKG_TARGETS cleanup_pkgs.append(realpkg) finally: ret = self.run_state('pkg.removed', @@ -721,21 +697,22 @@ def test_pkg_cap_003_installed_multipkg_with_version(self): self.assertSaltTrueReturn(ret) @skipIf(not _PKG_CAP_TARGETS, 'Capability not available') - @requires_salt_modules('pkg.version', 'pkg.latest', 'pkg.removed') + @requires_salt_modules('pkg.version') + @requires_salt_state_modules('pkg.latest', 'pkg.removed') def test_pkg_cap_004_latest(self): ''' This tests pkg.latest with a package that has no epoch (or a zero epoch). ''' - target, realpkg = _PKG_CAP_TARGETS[0] + target, realpkg = self._PKG_CAP_TARGETS[0] version = self.run_function('pkg.version', [target]) realver = self.run_function('pkg.version', [realpkg]) - # If this assert fails, we need to find new targets, this test needs to - # be able to test successful installation of packages, so this package - # needs to not be installed before we run the states below - self.assertFalse(version) - self.assertFalse(realver) + # If this condition is False, we need to find new targets. + # This needs to be able to test successful installation of packages. + # These packages need to not be installed before we run the states below + if not (version and realver): + self.skipTest('TODO: New pkg cap targets required') try: ret = self.run_state('pkg.latest', name=target, refresh=False, resolve_capabilities=True, test=True) @@ -751,20 +728,21 @@ def test_pkg_cap_004_latest(self): self.assertSaltTrueReturn(ret) @skipIf(not _PKG_CAP_TARGETS, 'Capability not available') - @requires_salt_modules('pkg.version', 'pkg.installed', 'pkg.removed', 'pkg.downloaded') + @requires_salt_modules('pkg.version') + @requires_salt_state_modules('pkg.installed', 'pkg.removed', 'pkg.downloaded') def test_pkg_cap_005_downloaded(self): ''' This is a destructive test as it installs and then removes a package ''' - target, realpkg = _PKG_CAP_TARGETS[0] + target, realpkg = self._PKG_CAP_TARGETS[0] version = self.run_function('pkg.version', [target]) realver = self.run_function('pkg.version', [realpkg]) - # If this assert fails, we need to find new targets, this test needs to - # be able to test successful installation of packages, so this package - # needs to not be installed before we run the states below - self.assertFalse(version) - self.assertFalse(realver) + # If this condition is False, we need to find new targets. + # This needs to be able to test successful installation of packages. + # These packages need to not be installed before we run the states below + if not (version and realver): + self.skipTest('TODO: New pkg cap targets required') ret = self.run_state('pkg.downloaded', name=target, refresh=False) self.assertSaltFalseReturn(ret) @@ -776,20 +754,21 @@ def test_pkg_cap_005_downloaded(self): self.assertSaltTrueReturn(ret) @skipIf(not _PKG_CAP_TARGETS, 'Capability not available') - @requires_salt_modules('pkg.version', 'pkg.installed', 'pkg.removed', 'pkg.uptodate') + @requires_salt_modules('pkg.version') + @requires_salt_state_modules('pkg.installed', 'pkg.removed', 'pkg.uptodate') def test_pkg_cap_006_uptodate(self): ''' This is a destructive test as it installs and then removes a package ''' - target, realpkg = _PKG_CAP_TARGETS[0] + target, realpkg = self._PKG_CAP_TARGETS[0] version = self.run_function('pkg.version', [target]) realver = self.run_function('pkg.version', [realpkg]) - # If this assert fails, we need to find new targets, this test needs to - # be able to test successful installation of packages, so this package - # needs to not be installed before we run the states below - self.assertFalse(version) - self.assertFalse(realver) + # If this condition is False, we need to find new targets. + # This needs to be able to test successful installation of packages. + # These packages need to not be installed before we run the states below + if not (version and realver): + self.skipTest('TODO: New pkg cap targets required') try: ret = self.run_state('pkg.installed', name=target, diff --git a/tests/integration/states/test_pkgrepo.py b/tests/integration/states/test_pkgrepo.py index 0101fb138c72..e132a1149a16 100644 --- a/tests/integration/states/test_pkgrepo.py +++ b/tests/integration/states/test_pkgrepo.py @@ -14,6 +14,7 @@ from tests.support.helpers import ( destructiveTest, requires_salt_modules, + requires_salt_state_modules, requires_system_grains, ) @@ -77,7 +78,7 @@ def test_pkgrepo_02_absent(self, grains): for state_id, state_result in six.iteritems(ret): self.assertSaltTrueReturn(dict([(state_id, state_result)])) - @requires_salt_modules('pkgrepo.absent', 'pkgrepo.managed') + @requires_salt_state_modules('pkgrepo.absent', 'pkgrepo.managed') @requires_system_grains def test_pkgrepo_03_with_comments(self, grains): ''' @@ -127,7 +128,7 @@ def test_pkgrepo_03_with_comments(self, grains): # Clean up self.run_state('pkgrepo.absent', name=kwargs['name']) - @requires_salt_modules('pkgrepo.managed') + @requires_salt_state_modules('pkgrepo.managed') @requires_system_grains def test_pkgrepo_04_apt_with_architectures(self, grains): ''' diff --git a/tests/support/helpers.py b/tests/support/helpers.py index 6fc0e79bc060..668d3e1a6b80 100644 --- a/tests/support/helpers.py +++ b/tests/support/helpers.py @@ -1067,6 +1067,111 @@ def decorator(*args, **kwargs): return decorator +@requires_system_grains +def runs_on(grains=None, **kwargs): + ''' + Skip the test if grains don't match the values passed into **kwargs + if a kwarg value is a list then skip if the grains don't match any item in the list + ''' + def decorator(caller): + @functools.wraps(caller) + def wrapper(cls): + for kw, value in kwargs.items(): + if isinstance(value, list): + if not any(str(grains.get(kw)).lower() != str(v).lower() for v in value): + cls.skipTest('This test does not run on {}={}'.format(kw, grains.get(kw))) + else: + if str(grains.get(kw)).lower() != str(value).lower(): + cls.skipTest('This test runs on {}={}, not {}'.format(kw, value, grains.get(kw))) + return caller(cls) + return wrapper + return decorator + + +@requires_system_grains +def not_runs_on(grains=None, **kwargs): + ''' + Reverse of `runs_on`. + Skip the test if any grains match the values passed into **kwargs + if a kwarg value is a list then skip if the grains match any item in the list + ''' + def decorator(caller): + @functools.wraps(caller) + def wrapper(cls): + for kw, value in kwargs.items(): + if isinstance(value, list): + if any(str(grains.get(kw)).lower() == str(v).lower() for v in value): + cls.skipTest('This test does not run on {}={}'.format(kw, grains.get(kw))) + else: + if str(grains.get(kw)).lower() == str(value).lower(): + cls.skipTest('This test does not run on {}={}, got {}'.format(kw, value, grains.get(kw))) + return caller(cls) + return wrapper + return decorator + + +def requires_salt_state_modules(*names): + ''' + Makes sure the passed salt state module is available. Skips the test if not + + .. versionadded:: 3000 + ''' + + def _check_required_salt_state_modules(*required_salt_modules): + # Late import + from tests.support.sminion import create_sminion + required_salt_modules = set(required_salt_modules) + sminion = create_sminion(minion_id='runtests-internal-sminion') + available_modules = list(sminion.states) + not_available_modules = set() + try: + cached_not_available_modules = sminion.__not_availiable_modules__ + except AttributeError: + cached_not_available_modules = sminion.__not_availiable_modules__ = set() + + if cached_not_available_modules: + for not_available_module in cached_not_available_modules: + if not_available_module in required_salt_modules: + not_available_modules.add(not_available_module) + required_salt_modules.remove(not_available_module) + + for required_module_name in required_salt_modules: + search_name = required_module_name + if '.' not in search_name: + search_name += '.*' + if not fnmatch.filter(available_modules, search_name): + not_available_modules.add(required_module_name) + cached_not_available_modules.add(required_module_name) + + if not_available_modules: + if len(not_available_modules) == 1: + raise SkipTest('Salt module \'{}\' is not available'.format(*not_available_modules)) + raise SkipTest('Salt modules not available: {}'.format(', '.join(not_available_modules))) + + def decorator(caller): + if inspect.isclass(caller): + # We're decorating a class + old_setup = getattr(caller, 'setUp', None) + + def setUp(self, *args, **kwargs): + _check_required_salt_state_modules(*names) + if old_setup is not None: + old_setup(self, *args, **kwargs) + + caller.setUp = setUp + return caller + + # We're simply decorating functions + @functools.wraps(caller) + def wrapper(cls): + _check_required_salt_state_modules(*names) + return caller(cls) + + return wrapper + + return decorator + + def requires_salt_modules(*names): ''' Makes sure the passed salt module is available. Skips the test if not @@ -1219,7 +1324,7 @@ def test_sometimes_works(self): @functools.wraps(caller) def wrap(cls): result = None - for attempt in range(1, times+1): + for attempt in range(1, times + 1): log.info('%s test run %d of %s times', cls, attempt, times) caller(cls) return cls From b3903ed2898787327d217e1113a5dce686eee293 Mon Sep 17 00:00:00 2001 From: Tyler Johnson Date: Fri, 3 Jan 2020 12:18:22 -0700 Subject: [PATCH 2/4] Fix mac_brew_pkg returning dict instead of string --- salt/modules/mac_brew_pkg.py | 7 +++++-- tests/integration/modules/test_pkg.py | 2 +- tests/integration/states/test_pkg.py | 4 ++-- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/salt/modules/mac_brew_pkg.py b/salt/modules/mac_brew_pkg.py index d29f3a647e60..5484955edc14 100644 --- a/salt/modules/mac_brew_pkg.py +++ b/salt/modules/mac_brew_pkg.py @@ -574,10 +574,13 @@ def _fix_cask_namespace(name=None, pkgs=None): if pkgs: pkgs_ = [] for pkg in pkgs: - if pkg.startswith('caskroom/cask/'): + if isinstance(pkg, str) and pkg.startswith('caskroom/cask/'): show_warning = True pkg = pkg.replace("caskroom/cask/", "homebrew/cask/") - pkgs_.append(pkg) + pkgs_.append(pkg) + else: + pkgs_.append(pkg) + continue pkgs = pkgs_ if show_warning: diff --git a/tests/integration/modules/test_pkg.py b/tests/integration/modules/test_pkg.py index 1dfe55eee629..680c37b30924 100644 --- a/tests/integration/modules/test_pkg.py +++ b/tests/integration/modules/test_pkg.py @@ -210,7 +210,7 @@ def test_hold_unhold(self, grains): try: hold_ret = self.run_function('pkg.hold', [self.pkg]) if versionlock_pkg and '-versionlock is not installed' in str(hold_ret): - self.skipTest(str(hold_ret) + ' `{}` is installed'.format(versionlock_pkg)) + self.skipTest('{} `{}` is installed'.format(hold_ret, versionlock_pkg)) self.assertIn(self.pkg, hold_ret) self.assertTrue(hold_ret[self.pkg]['result']) diff --git a/tests/integration/states/test_pkg.py b/tests/integration/states/test_pkg.py index c5b059afe7e8..4dd6bfa12748 100644 --- a/tests/integration/states/test_pkg.py +++ b/tests/integration/states/test_pkg.py @@ -37,7 +37,6 @@ @destructiveTest class PkgTest(ModuleCase, SaltReturnAssertsMixin): _PKG_EPOCH_TARGETS = [] - _PKG_TARGETS = ['figlet', 'sl'] _PKG_32_TARGETS = [] _PKG_CAP_TARGETS = [] _PKG_DOT_TARGETS = [] @@ -48,6 +47,7 @@ class PkgTest(ModuleCase, SaltReturnAssertsMixin): @requires_system_grains def setUpClass(cls, grains=None): # pylint:disable=W0221 cls.ctx = {} + cls._PKG_TARGETS = ['figlet', 'sl'] if grains['os'] == 'Windows': cls._PKG_TARGETS = ['7zip', 'putty'] elif grains['os'] == 'freebsd': @@ -563,7 +563,7 @@ def test_pkg_015_installed_held(self, grains=None): ) if versionlock_pkg and '-versionlock is not installed' in str(ret): - self.skipTest(str(ret) + ' `{}` is installed'.format(versionlock_pkg)) + self.skipTest('{} `{}` is installed'.format(ret, versionlock_pkg)) # changes from pkg.hold for Red Hat family are different target_changes = {} From 7225053fe2db645893806c976f9d01eb831c074f Mon Sep 17 00:00:00 2001 From: Tyler Johnson Date: Fri, 3 Jan 2020 23:00:29 -0700 Subject: [PATCH 3/4] Separated check for sminion attributes to top level --- tests/integration/modules/test_pkg.py | 6 +- tests/integration/states/test_pkg.py | 44 ++++----- tests/integration/states/test_pkgrepo.py | 6 +- tests/support/helpers.py | 115 ++++++++++------------- 4 files changed, 76 insertions(+), 95 deletions(-) diff --git a/tests/integration/modules/test_pkg.py b/tests/integration/modules/test_pkg.py index 680c37b30924..af15f8612bc9 100644 --- a/tests/integration/modules/test_pkg.py +++ b/tests/integration/modules/test_pkg.py @@ -12,7 +12,7 @@ destructiveTest, requires_network, requires_salt_modules, - requires_salt_state_modules, + requires_salt_states, requires_system_grains, skip_if_not_root) from tests.support.unit import skipIf @@ -182,7 +182,7 @@ def test_remove(): @destructiveTest @requires_salt_modules('pkg.hold', 'pkg.unhold', 'pkg.install', 'pkg.version', 'pkg.remove', 'pkg.list_pkgs') - @requires_salt_state_modules('pkg.installed') + @requires_salt_states('pkg.installed') @requires_network() @requires_system_grains def test_hold_unhold(self, grains): @@ -355,7 +355,7 @@ def test_pkg_upgrade_has_pending_upgrades(self, grains): @destructiveTest @skipIf(salt.utils.platform.is_darwin(), 'The jenkins user is equivalent to root on mac, causing the test to be unrunnable') @requires_salt_modules('pkg.remove', 'pkg.latest_version') - @requires_salt_state_modules('pkg.removed') + @requires_salt_states('pkg.removed') @requires_system_grains def test_pkg_latest_version(self, grains): ''' diff --git a/tests/integration/states/test_pkg.py b/tests/integration/states/test_pkg.py index 4dd6bfa12748..046757e041d9 100644 --- a/tests/integration/states/test_pkg.py +++ b/tests/integration/states/test_pkg.py @@ -16,7 +16,7 @@ from tests.support.helpers import ( destructiveTest, requires_salt_modules, - requires_salt_state_modules, + requires_salt_states, requires_system_grains, runs_on, not_runs_on) @@ -123,7 +123,7 @@ def setUp(self, grains=None): # pylint:disable=W0221 raise Exception('Package database locked after 60 seconds, bailing out') @requires_salt_modules('pkg.version') - @requires_salt_state_modules('pkg.installed', 'pkg.removed') + @requires_salt_states('pkg.installed', 'pkg.removed') def test_pkg_001_installed(self): ''' This is a destructive test as it installs and then removes a package @@ -142,7 +142,7 @@ def test_pkg_001_installed(self): self.assertSaltTrueReturn(ret) @skipIf(not _VERSION_SPEC_SUPPORTED, 'Version specification not supported') - @requires_salt_state_modules('pkg.installed', 'pkg.removed') + @requires_salt_states('pkg.installed', 'pkg.removed') def test_pkg_002_installed_with_version(self): ''' This is a destructive test as it installs and then removes a package @@ -163,7 +163,7 @@ def test_pkg_002_installed_with_version(self): ret = self.run_state('pkg.removed', name=target) self.assertSaltTrueReturn(ret) - @requires_salt_state_modules('pkg.installed', 'pkg.removed') + @requires_salt_states('pkg.installed', 'pkg.removed') def test_pkg_003_installed_multipkg(self): ''' This is a destructive test as it installs and then removes two packages @@ -187,7 +187,7 @@ def test_pkg_003_installed_multipkg(self): self.assertSaltTrueReturn(ret) @skipIf(not _VERSION_SPEC_SUPPORTED, 'Version specification not supported') - @requires_salt_state_modules('pkg.installed', 'pkg.removed') + @requires_salt_states('pkg.installed', 'pkg.removed') def test_pkg_004_installed_multipkg_with_version(self): ''' This is a destructive test as it installs and then removes two packages @@ -213,7 +213,7 @@ def test_pkg_004_installed_multipkg_with_version(self): @skipIf(not _PKG_32_TARGETS, 'No 32 bit packages have been specified for testing') @requires_salt_modules('pkg.version') - @requires_salt_state_modules('pkg.installed', 'pkg.removed') + @requires_salt_states('pkg.installed', 'pkg.removed') def test_pkg_005_installed_32bit(self): ''' This is a destructive test as it installs and then removes a package @@ -239,7 +239,7 @@ def test_pkg_005_installed_32bit(self): self.assertSaltTrueReturn(ret) @skipIf(not _PKG_32_TARGETS, 'No 32 bit packages have been specified for testing') - @requires_salt_state_modules('pkg.installed', 'pkg.removed') + @requires_salt_states('pkg.installed', 'pkg.removed') def test_pkg_006_installed_32bit_with_version(self): ''' This is a destructive test as it installs and then removes a package @@ -266,7 +266,7 @@ def test_pkg_006_installed_32bit_with_version(self): self.assertSaltTrueReturn(ret) @skipIf(not _PKG_DOT_TARGETS, 'No packages with "." in their name have been configured for') - @requires_salt_state_modules('pkg.installed', 'pkg.removed') + @requires_salt_states('pkg.installed', 'pkg.removed') def test_pkg_007_with_dot_in_pkgname(self=None): ''' This tests for the regression found in the following issue: @@ -288,7 +288,7 @@ def test_pkg_007_with_dot_in_pkgname(self=None): self.assertSaltTrueReturn(ret) @skipIf(not _PKG_EPOCH_TARGETS, 'No targets have been configured with "epoch" in the version') - @requires_salt_state_modules('pkg.installed', 'pkg.removed') + @requires_salt_states('pkg.installed', 'pkg.removed') def test_pkg_008_epoch_in_version(self): ''' This tests for the regression found in the following issue: @@ -313,7 +313,7 @@ def test_pkg_008_epoch_in_version(self): self.assertSaltTrueReturn(ret) @requires_salt_modules('pkg.version', 'pkg.info_installed') - @requires_salt_state_modules('pkg.installed', 'pkg.removed') + @requires_salt_states('pkg.installed', 'pkg.removed') @runs_on(kernel='linux') @not_runs_on(os='Amazon') def test_pkg_009_latest_with_epoch(self): @@ -334,7 +334,7 @@ def test_pkg_009_latest_with_epoch(self): ret = self.run_function('pkg.info_installed', [package]) self.assertTrue(pkgquery in six.text_type(ret)) - @requires_salt_state_modules('pkg.latest', 'pkg.removed') + @requires_salt_states('pkg.latest', 'pkg.removed') def test_pkg_010_latest(self): ''' This tests pkg.latest with a package that has no epoch (or a zero @@ -354,7 +354,7 @@ def test_pkg_010_latest(self): self.assertSaltTrueReturn(ret) @requires_salt_modules('pkg.list_pkgs', 'pkg.list_upgrades', 'pkg.version') - @requires_salt_state_modules('pkg.latest') + @requires_salt_states('pkg.latest') @runs_on(kernel='linux', os_family='Debian') def test_pkg_011_latest_only_upgrade(self): ''' @@ -406,7 +406,7 @@ def test_pkg_011_latest_only_upgrade(self): @skipIf(not _WILDCARDS_SUPPORTED, 'Wildcards in pkg.install are not supported') @requires_salt_modules('pkg.version') - @requires_salt_state_modules('pkg.installed', 'pkg.removed') + @requires_salt_states('pkg.installed', 'pkg.removed') def test_pkg_012_installed_with_wildcard_version(self): ''' This is a destructive test as it installs and then removes a package @@ -456,7 +456,7 @@ def test_pkg_012_installed_with_wildcard_version(self): self.assertSaltTrueReturn(ret) @requires_salt_modules('pkg.version', 'pkg.latest_version') - @requires_salt_state_modules('pkg.installed', 'pkg.removed') + @requires_salt_states('pkg.installed', 'pkg.removed') @runs_on(kernel='linux', os_family=['Debian', 'RedHat']) def test_pkg_013_installed_with_comparison_operator(self): ''' @@ -493,7 +493,7 @@ def test_pkg_013_installed_with_comparison_operator(self): self.assertSaltTrueReturn(ret) @requires_salt_modules('pkg.version') - @requires_salt_state_modules('pkg.installed', 'pkg.removed') + @requires_salt_states('pkg.installed', 'pkg.removed') @runs_on(kernel='linux', os_familiy='RedHat') def test_pkg_014_installed_missing_release(self): ''' @@ -521,7 +521,7 @@ def test_pkg_014_installed_missing_release(self): self.assertSaltTrueReturn(ret) @requires_salt_modules('pkg.hold', 'pkg.unhold', 'pkg.version', 'pkg.list_pkgs') - @requires_salt_state_modules('pkg.installed', 'pkg.removed') + @requires_salt_states('pkg.installed', 'pkg.removed') @requires_system_grains def test_pkg_015_installed_held(self, grains=None): ''' @@ -593,7 +593,7 @@ def test_pkg_015_installed_held(self, grains=None): @skipIf(not _PKG_CAP_TARGETS, 'Capability not provided') @requires_salt_modules('pkg.version') - @requires_salt_state_modules('pkg.installed', 'pkg.removed') + @requires_salt_states('pkg.installed', 'pkg.removed') def test_pkg_cap_001_installed(self): ''' This is a destructive test as it installs and then removes a package @@ -619,7 +619,7 @@ def test_pkg_cap_001_installed(self): self.assertSaltTrueReturn(ret) @skipIf(not _PKG_CAP_TARGETS, 'Capability not available') - @requires_salt_state_modules('pkg.installed', 'pkg.removed') + @requires_salt_states('pkg.installed', 'pkg.removed') def test_pkg_cap_002_already_installed(self): ''' This is a destructive test as it installs and then removes a package @@ -653,7 +653,7 @@ def test_pkg_cap_002_already_installed(self): @skipIf(not _PKG_CAP_TARGETS, 'Capability not available') @skipIf(not _VERSION_SPEC_SUPPORTED, 'Version specification not supported') - @requires_salt_state_modules('pkg.installed', 'pkg.removed') + @requires_salt_states('pkg.installed', 'pkg.removed') def test_pkg_cap_003_installed_multipkg_with_version(self): ''' This is a destructive test as it installs and then removes two packages @@ -698,7 +698,7 @@ def test_pkg_cap_003_installed_multipkg_with_version(self): @skipIf(not _PKG_CAP_TARGETS, 'Capability not available') @requires_salt_modules('pkg.version') - @requires_salt_state_modules('pkg.latest', 'pkg.removed') + @requires_salt_states('pkg.latest', 'pkg.removed') def test_pkg_cap_004_latest(self): ''' This tests pkg.latest with a package that has no epoch (or a zero @@ -729,7 +729,7 @@ def test_pkg_cap_004_latest(self): @skipIf(not _PKG_CAP_TARGETS, 'Capability not available') @requires_salt_modules('pkg.version') - @requires_salt_state_modules('pkg.installed', 'pkg.removed', 'pkg.downloaded') + @requires_salt_states('pkg.installed', 'pkg.removed', 'pkg.downloaded') def test_pkg_cap_005_downloaded(self): ''' This is a destructive test as it installs and then removes a package @@ -755,7 +755,7 @@ def test_pkg_cap_005_downloaded(self): @skipIf(not _PKG_CAP_TARGETS, 'Capability not available') @requires_salt_modules('pkg.version') - @requires_salt_state_modules('pkg.installed', 'pkg.removed', 'pkg.uptodate') + @requires_salt_states('pkg.installed', 'pkg.removed', 'pkg.uptodate') def test_pkg_cap_006_uptodate(self): ''' This is a destructive test as it installs and then removes a package diff --git a/tests/integration/states/test_pkgrepo.py b/tests/integration/states/test_pkgrepo.py index e132a1149a16..2982745eae3a 100644 --- a/tests/integration/states/test_pkgrepo.py +++ b/tests/integration/states/test_pkgrepo.py @@ -14,7 +14,7 @@ from tests.support.helpers import ( destructiveTest, requires_salt_modules, - requires_salt_state_modules, + requires_salt_states, requires_system_grains, ) @@ -78,7 +78,7 @@ def test_pkgrepo_02_absent(self, grains): for state_id, state_result in six.iteritems(ret): self.assertSaltTrueReturn(dict([(state_id, state_result)])) - @requires_salt_state_modules('pkgrepo.absent', 'pkgrepo.managed') + @requires_salt_states('pkgrepo.absent', 'pkgrepo.managed') @requires_system_grains def test_pkgrepo_03_with_comments(self, grains): ''' @@ -128,7 +128,7 @@ def test_pkgrepo_03_with_comments(self, grains): # Clean up self.run_state('pkgrepo.absent', name=kwargs['name']) - @requires_salt_state_modules('pkgrepo.managed') + @requires_salt_states('pkgrepo.managed') @requires_system_grains def test_pkgrepo_04_apt_with_architectures(self, grains): ''' diff --git a/tests/support/helpers.py b/tests/support/helpers.py index 668d3e1a6b80..3a02fdfcc03b 100644 --- a/tests/support/helpers.py +++ b/tests/support/helpers.py @@ -1110,43 +1110,48 @@ def wrapper(cls): return decorator -def requires_salt_state_modules(*names): +def check_required_sminion_attributes(sminion_attr, *required_items): ''' - Makes sure the passed salt state module is available. Skips the test if not + :param sminion_attr: The name of the sminion attribute to check, such as 'functions' or 'states' + :param required_items: The items that must be part of the designated sminion attribute for the decorated test + :return The packages that are not available + ''' + # Late import + from tests.support.sminion import create_sminion + required_salt_items = set(required_items) + sminion = create_sminion(minion_id='runtests-internal-sminion') + available_items = list(getattr(sminion, sminion_attr)) + not_available_items = set() - .. versionadded:: 3000 + name = '__not_available_{items}s__'.format(items=sminion_attr) + if not hasattr(sminion, name): + setattr(sminion, name, set()) + + cached_not_available_items = getattr(sminion, name) + + for not_available_item in cached_not_available_items: + if not_available_item in required_salt_items: + not_available_items.add(not_available_item) + required_salt_items.remove(not_available_item) + + for required_item_name in required_salt_items: + search_name = required_item_name + if '.' not in search_name: + search_name += '.*' + if not fnmatch.filter(available_items, search_name): + not_available_items.add(required_item_name) + cached_not_available_items.add(required_item_name) + + return not_available_items + + +def requires_salt_states(*names): ''' + Makes sure the passed salt state is available. Skips the test if not - def _check_required_salt_state_modules(*required_salt_modules): - # Late import - from tests.support.sminion import create_sminion - required_salt_modules = set(required_salt_modules) - sminion = create_sminion(minion_id='runtests-internal-sminion') - available_modules = list(sminion.states) - not_available_modules = set() - try: - cached_not_available_modules = sminion.__not_availiable_modules__ - except AttributeError: - cached_not_available_modules = sminion.__not_availiable_modules__ = set() - - if cached_not_available_modules: - for not_available_module in cached_not_available_modules: - if not_available_module in required_salt_modules: - not_available_modules.add(not_available_module) - required_salt_modules.remove(not_available_module) - - for required_module_name in required_salt_modules: - search_name = required_module_name - if '.' not in search_name: - search_name += '.*' - if not fnmatch.filter(available_modules, search_name): - not_available_modules.add(required_module_name) - cached_not_available_modules.add(required_module_name) - - if not_available_modules: - if len(not_available_modules) == 1: - raise SkipTest('Salt module \'{}\' is not available'.format(*not_available_modules)) - raise SkipTest('Salt modules not available: {}'.format(', '.join(not_available_modules))) + .. versionadded:: 3000 + ''' + not_available = check_required_sminion_attributes('states', *names) def decorator(caller): if inspect.isclass(caller): @@ -1154,7 +1159,9 @@ def decorator(caller): old_setup = getattr(caller, 'setUp', None) def setUp(self, *args, **kwargs): - _check_required_salt_state_modules(*names) + if not_available: + raise SkipTest('Unavailable salt states: {}'.format(*not_available)) + if old_setup is not None: old_setup(self, *args, **kwargs) @@ -1164,7 +1171,8 @@ def setUp(self, *args, **kwargs): # We're simply decorating functions @functools.wraps(caller) def wrapper(cls): - _check_required_salt_state_modules(*names) + if not_available: + raise SkipTest('Unavailable salt states: {}'.format(*not_available)) return caller(cls) return wrapper @@ -1178,36 +1186,7 @@ def requires_salt_modules(*names): .. versionadded:: 0.5.2 ''' - def _check_required_salt_modules(*required_salt_modules): - # Late import - from tests.support.sminion import create_sminion - required_salt_modules = set(required_salt_modules) - sminion = create_sminion(minion_id='runtests-internal-sminion') - available_modules = list(sminion.functions) - not_available_modules = set() - try: - cached_not_available_modules = sminion.__not_availiable_modules__ - except AttributeError: - cached_not_available_modules = sminion.__not_availiable_modules__ = set() - - if cached_not_available_modules: - for not_available_module in cached_not_available_modules: - if not_available_module in required_salt_modules: - not_available_modules.add(not_available_module) - required_salt_modules.remove(not_available_module) - - for required_module_name in required_salt_modules: - search_name = required_module_name - if '.' not in search_name: - search_name += '.*' - if not fnmatch.filter(available_modules, search_name): - not_available_modules.add(required_module_name) - cached_not_available_modules.add(required_module_name) - - if not_available_modules: - if len(not_available_modules) == 1: - raise SkipTest('Salt module \'{}\' is not available'.format(*not_available_modules)) - raise SkipTest('Salt modules not available: {}'.format(', '.join(not_available_modules))) + not_available = check_required_sminion_attributes('functions', *names) def decorator(caller): if inspect.isclass(caller): @@ -1215,7 +1194,8 @@ def decorator(caller): old_setup = getattr(caller, 'setUp', None) def setUp(self, *args, **kwargs): - _check_required_salt_modules(*names) + if not_available: + raise SkipTest('Unavailable salt modules: {}'.format(*not_available)) if old_setup is not None: old_setup(self, *args, **kwargs) @@ -1225,7 +1205,8 @@ def setUp(self, *args, **kwargs): # We're simply decorating functions @functools.wraps(caller) def wrapper(cls): - _check_required_salt_modules(*names) + if not_available: + raise SkipTest('Unavailable salt modules: {}'.format(*not_available)) return caller(cls) return wrapper From b5ae0bb6e7d67c1af9cc8fc60e5af5f6b4228cb3 Mon Sep 17 00:00:00 2001 From: Tyler Johnson Date: Sat, 4 Jan 2020 10:54:44 -0700 Subject: [PATCH 4/4] Fixed undefined variable, hid internal helper function --- tests/integration/states/test_pkgrepo.py | 8 +++++--- tests/support/helpers.py | 6 +++--- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/tests/integration/states/test_pkgrepo.py b/tests/integration/states/test_pkgrepo.py index 2982745eae3a..21c764560610 100644 --- a/tests/integration/states/test_pkgrepo.py +++ b/tests/integration/states/test_pkgrepo.py @@ -20,6 +20,7 @@ # Import Salt libs import salt.utils.platform +import salt.utils.files # Import 3rd-party libs from salt.ext import six @@ -84,15 +85,16 @@ def test_pkgrepo_03_with_comments(self, grains): ''' Test adding a repo with comments ''' - if grains['os_family'] in ('redhat',): + kwargs = {} + if grains['os_family'] == 'RedHat': kwargs = { 'name': 'examplerepo', 'baseurl': 'http://example.com/repo', 'enabled': False, 'comments': ['This is a comment'] } - elif grains['os_family'] in ('debian',): - self.skipTest('Debian/Ubuntu test case needed') + else: + self.skipTest('{}/{} test case needed'.format(grains['os_family'], grains['os'])) try: # Run the state to add the repo diff --git a/tests/support/helpers.py b/tests/support/helpers.py index 3a02fdfcc03b..7714fc219f59 100644 --- a/tests/support/helpers.py +++ b/tests/support/helpers.py @@ -1110,7 +1110,7 @@ def wrapper(cls): return decorator -def check_required_sminion_attributes(sminion_attr, *required_items): +def _check_required_sminion_attributes(sminion_attr, *required_items): ''' :param sminion_attr: The name of the sminion attribute to check, such as 'functions' or 'states' :param required_items: The items that must be part of the designated sminion attribute for the decorated test @@ -1151,7 +1151,7 @@ def requires_salt_states(*names): .. versionadded:: 3000 ''' - not_available = check_required_sminion_attributes('states', *names) + not_available = _check_required_sminion_attributes('states', *names) def decorator(caller): if inspect.isclass(caller): @@ -1186,7 +1186,7 @@ def requires_salt_modules(*names): .. versionadded:: 0.5.2 ''' - not_available = check_required_sminion_attributes('functions', *names) + not_available = _check_required_sminion_attributes('functions', *names) def decorator(caller): if inspect.isclass(caller):