fix most ansible-test issues, suppress the rest

Automation Hub, and possibly Galaxy in the future, require the
collection to be screened with `ansible-test sanity` among other
checks.  The role had a number of issues:
* Use `AssertionError` instead of `assert`
* Use of `logging` module not in accordance with standards, but these
  are ok and the errors were suppressed
* Several import errors which are ok because they are checked
  elsewhere
* __init__.py in the module_utils directories must be empty, so a
  new file myerror.py was added to move the code from __init__.py
  * NOTE: network_lsr/nm/__init__.py is not empty
* The documentation block in the module was not properly constructed
  or formatted.
* shellcheck issues, including removing unused files
* use `unused` instead of `_` (underscore) for variables that are
  unused

add WARNING to module docs - collection users should not use directly

Signed-off-by: Rich Megginson <rmeggins@redhat.com>

.github/workflows/tox.yml

@@ -38,7 +38,7 @@ jobs:
           toxenvs="py${toxpyver}"
           case "$toxpyver" in
           27) toxenvs="${toxenvs},coveralls,flake8,pylint" ;;
-          36) toxenvs="${toxenvs},coveralls,black,yamllint,ansible-lint,collection" ;;
+          36) toxenvs="${toxenvs},coveralls,black,yamllint,ansible-lint,collection,ansible-test" ;;
           37) toxenvs="${toxenvs},coveralls" ;;
           38) toxenvs="${toxenvs},coveralls" ;;
           esac

.sanity-ansible-ignore-2.9.txt

@@ -0,0 +1,48 @@
+plugins/module_utils/network_lsr/nm/__init__.py empty-init!skip
+plugins/module_utils/network_lsr/nm/active_connection.py import-2.6!skip
+plugins/module_utils/network_lsr/nm/active_connection.py import-2.7!skip
+plugins/module_utils/network_lsr/nm/active_connection.py import-3.5!skip
+plugins/module_utils/network_lsr/nm/active_connection.py import-3.6!skip
+plugins/module_utils/network_lsr/nm/active_connection.py import-3.7!skip
+plugins/module_utils/network_lsr/nm/active_connection.py import-3.8!skip
+plugins/module_utils/network_lsr/nm/client.py import-2.6!skip
+plugins/module_utils/network_lsr/nm/client.py import-2.7!skip
+plugins/module_utils/network_lsr/nm/client.py import-3.5!skip
+plugins/module_utils/network_lsr/nm/client.py import-3.6!skip
+plugins/module_utils/network_lsr/nm/client.py import-3.7!skip
+plugins/module_utils/network_lsr/nm/client.py import-3.8!skip
+plugins/module_utils/network_lsr/nm/connection.py import-2.6!skip
+plugins/module_utils/network_lsr/nm/connection.py import-2.7!skip
+plugins/module_utils/network_lsr/nm/connection.py import-3.5!skip
+plugins/module_utils/network_lsr/nm/connection.py import-3.6!skip
+plugins/module_utils/network_lsr/nm/connection.py import-3.7!skip
+plugins/module_utils/network_lsr/nm/connection.py import-3.8!skip
+plugins/module_utils/network_lsr/nm/provider.py import-2.6!skip
+plugins/module_utils/network_lsr/nm/provider.py import-2.7!skip
+plugins/module_utils/network_lsr/nm/provider.py import-3.5!skip
+plugins/module_utils/network_lsr/nm/provider.py import-3.6!skip
+plugins/module_utils/network_lsr/nm/provider.py import-3.7!skip
+plugins/module_utils/network_lsr/nm/provider.py import-3.8!skip
+plugins/modules/network_connections.py validate-modules:missing-examples
+plugins/modules/network_connections.py validate-modules:missing-gplv3-license
+plugins/modules/network_connections.py validate-modules:no-default-for-required-parameter
+plugins/modules/network_connections.py validate-modules:parameter-type-not-in-doc
+plugins/modules/network_connections.py validate-modules:undocumented-parameter
+tests/network/covstats shebang!skip
+tests/network/ensure_provider_tests.py compile-2.6!skip
+tests/network/ensure_provider_tests.py compile-2.7!skip
+tests/network/ensure_provider_tests.py compile-3.5!skip
+tests/network/ensure_provider_tests.py future-import-boilerplate!skip
+tests/network/ensure_provider_tests.py metaclass-boilerplate!skip
+tests/network/ensure_provider_tests.py shebang!skip
+tests/network/get_coverage.sh shebang!skip
+tests/network/get_total_coverage.sh shebang!skip
+tests/network/integration/conftest.py future-import-boilerplate!skip
+tests/network/integration/conftest.py metaclass-boilerplate!skip
+tests/network/integration/test_ethernet.py future-import-boilerplate!skip
+tests/network/integration/test_ethernet.py metaclass-boilerplate!skip
+tests/network/merge_coverage.sh shebang!skip
+tests/network/unit/test_network_connections.py future-import-boilerplate!skip
+tests/network/unit/test_network_connections.py metaclass-boilerplate!skip
+tests/network/unit/test_nm_provider.py future-import-boilerplate!skip
+tests/network/unit/test_nm_provider.py metaclass-boilerplate!skip

README.md

@@ -158,7 +158,7 @@ a consequence, `state: up` always changes the system.
 You can deactivate a connection profile, even if is currently not active. As a
 consequence, `state: down` always changes the system.
 
-Note that if the `state` option is unset, the connection profile’s runtime state will
+Note that if the `state` option is unset, the connection profile's runtime state will
 not be changed.
 
 ### `persistent_state`

library/network_connections.py

@@ -2,6 +2,30 @@
 # -*- coding: utf-8 -*-
 # SPDX-License-Identifier: BSD-3-Clause
 
+from __future__ import absolute_import, division, print_function
+
+__metaclass__ = type
+
+DOCUMENTATION = """
+---
+module: network_connections
+author: Thomas Haller (@thom311)
+short_description: module for network role to manage connection profiles
+requirements: [pygobject, dbus, NetworkManager]
+version_added: "2.0"
+description:
+  - "WARNING: Do not use this module directly! It is only for role internal use."
+  - |
+    Manage networking profiles (connections) for NetworkManager and
+    initscripts networking providers. Documentation needs to be written. Note
+    that the network_connections module tightly integrates with the network
+    role and currently it is not expected to use this module outside the role.
+    Thus, consult README.md for examples for the role.  The requirements are
+    only for the NetworkManager (nm) provider.
+options: {}
+"""
+
+
 import errno
 import functools
 import os
@@ -16,7 +40,7 @@ import logging
 # pylint: disable=import-error, no-name-in-module
 from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils.network_lsr import ethtool  # noqa:E501
-from ansible.module_utils.network_lsr import MyError  # noqa:E501
+from ansible.module_utils.network_lsr.myerror import MyError  # noqa:E501
 
 from ansible.module_utils.network_lsr.argument_validator import (  # noqa:E501
     ArgUtil,
@@ -30,22 +54,6 @@ from ansible.module_utils.network_lsr import nm_provider  # noqa:E501
 # pylint: enable=import-error, no-name-in-module
 
 
-DOCUMENTATION = """
----
-module: network_connections
-author: "Thomas Haller (thaller@redhat.com)"
-short_description: module for network role to manage connection profiles
-requirements: for 'nm' provider requires pygobject, dbus and NetworkManager.
-version_added: "2.0"
-description: Manage networking profiles (connections) for NetworkManager and
-  initscripts networking providers.
-options: Documentation needs to be written. Note that the network_connections
-  module tightly integrates with the network role and currently it is not
-  expected to use this module outside the role. Thus, consult README.md for
-  examples for the role.
-"""
-
-
 ###############################################################################
 PERSISTENT_STATE = "persistent_state"
 ABSENT_STATE = "absent"
@@ -774,7 +782,7 @@ class NMUtil:
         if compare_flags is None:
             compare_flags = NM.SettingCompareFlags.IGNORE_TIMESTAMP
 
-        return not (not (con_a.compare(con_b, compare_flags)))
+        return con_a.compare(con_b, compare_flags)
 
     def connection_is_active(self, con):
         NM = Util.NM()
@@ -1401,7 +1409,7 @@ class RunEnvironment(object):
     def check_mode_set(self, check_mode, connections=None):
         c = self._check_mode
         self._check_mode = check_mode
-        assert (
+        if not (
             (c is None and check_mode in [CheckMode.PREPARE])
             or (
                 c == CheckMode.PREPARE
@@ -1410,7 +1418,12 @@ class RunEnvironment(object):
             or (c == CheckMode.PRE_RUN and check_mode in [CheckMode.REAL_RUN])
             or (c == CheckMode.REAL_RUN and check_mode in [CheckMode.DONE])
             or (c == CheckMode.DRY_RUN and check_mode in [CheckMode.DONE])
-        )
+        ):
+            raise AssertionError(
+                "updating check_mode value from {0} into {1} is incorrect".format(
+                    c, check_mode
+                )
+            )
         self._check_mode_changed(c, check_mode, connections)
 
 
@@ -1472,7 +1485,8 @@ class RunEnvironmentAnsible(RunEnvironment):
         warn_traceback=False,
         force_fail=False,
     ):
-        assert idx >= -1
+        if not idx >= -1:
+            raise AssertionError("idx {0} is less than -1".format(idx))
         self._log_idx += 1
         self.run_results[idx]["log"].append((severity, msg, self._log_idx))
         if severity == LogLevel.ERROR:
@@ -1609,14 +1623,15 @@ class Cmd(object):
     def connections_data(self):
         c = self._connections_data
         if c is None:
-            assert self.check_mode in [
+            if self.check_mode not in [
                 CheckMode.DRY_RUN,
                 CheckMode.PRE_RUN,
                 CheckMode.REAL_RUN,
-            ]
+            ]:
-            c = []
+                raise AssertionError(
-            for _ in range(0, len(self.connections)):
+                    "invalid value {0} for self.check_mode".format(self.check_mode)
-                c.append({"changed": False})
+                )
+            c = [{"changed": False}] * len(self.connections)
             self._connections_data = c
         return c
 
@@ -1625,11 +1640,14 @@ class Cmd(object):
             c["changed"] = False
 
     def connections_data_set_changed(self, idx, changed=True):
-        assert self._check_mode in [
+        if self._check_mode not in [
             CheckMode.PRE_RUN,
             CheckMode.DRY_RUN,
             CheckMode.REAL_RUN,
-        ]
+        ]:
+            raise AssertionError(
+                "invalid value {0} for self._check_mode".format(self._check_mode)
+            )
         if not changed:
             return
         self.connections_data[idx]["changed"] = changed
@@ -1699,7 +1717,10 @@ class Cmd(object):
         # modify the connection.
 
         con = self.connections[idx]
-        assert con["state"] in ["up", "down"]
+        if con["state"] not in ["up", "down"]:
+            raise AssertionError(
+                "connection state {0} not 'up' or 'down'".format(con["state"])
+            )
 
         # also check, if the current profile is 'up' with a 'type' (which
         # possibly modifies the connection as well)
@@ -1747,7 +1768,9 @@ class Cmd(object):
         elif self._check_mode != CheckMode.DONE:
             c = CheckMode.DONE
         else:
-            assert False
+            raise AssertionError(
+                "invalid value {0} for self._check_mode".format(self._check_mode)
+            )
         self._check_mode = c
         self.run_env.check_mode_set(c)
         return c
@@ -1913,7 +1936,12 @@ class Cmd_nm(Cmd):
 
             name = connection["name"]
             if not name:
-                assert connection["persistent_state"] == "absent"
+                if not connection["persistent_state"] == "absent":
+                    raise AssertionError(
+                        "persistent_state must be 'absent' not {0} when there is no connection 'name'".format(
+                            connection["persistent_state"]
+                        )
+                    )
                 continue
             if name in names:
                 exists = names[name]["nm.exists"]
@@ -1990,7 +2018,7 @@ class Cmd_nm(Cmd):
                     idx, "ethtool.%s specified but not supported by NM", specified
                 )
 
-            for option, _ in specified.items():
+            for option in specified.keys():
                 nm_name = nm_get_name_fcnt(option)
                 if not nm_name:
                     self.log_fatal(

module_utils/network_lsr/__init__.py

@@ -1,7 +0,0 @@
-#!/usr/bin/python3 -tt
-# vim: fileencoding=utf8
-# SPDX-License-Identifier: BSD-3-Clause
-
-
-class MyError(Exception):
-    pass

module_utils/network_lsr/argument_validator.py

@@ -1,13 +1,16 @@
-#!/usr/bin/python3 -tt
 # vim: fileencoding=utf8
 # SPDX-License-Identifier: BSD-3-Clause
 
+from __future__ import absolute_import, division, print_function
+
+__metaclass__ = type
+
 import posixpath
 import socket
 import re
 
 # pylint: disable=import-error, no-name-in-module
-from ansible.module_utils.network_lsr import MyError  # noqa:E501
+from ansible.module_utils.network_lsr.myerror import MyError  # noqa:E501
 from ansible.module_utils.network_lsr.utils import Util  # noqa:E501
 
 UINT32_MAX = 0xFFFFFFFF
@@ -72,7 +75,8 @@ class ArgUtil:
 
 class ValidationError(MyError):
     def __init__(self, name, message):
-        Exception.__init__(self, name + ": " + message)
+        # pylint: disable=non-parent-init-called
+        super(ValidationError, self).__init__(name + ": " + message)
         self.error_message = message
         self.name = name
 

module_utils/network_lsr/ethtool.py

@@ -1,5 +1,9 @@
 # SPDX-License-Identifier: BSD-3-Clause
 
+from __future__ import absolute_import, division, print_function
+
+__metaclass__ = type
+
 import array
 import struct
 import fcntl
@@ -46,7 +50,7 @@ def get_perm_addr(ifname):
             res = ecmd.tobytes()
         except AttributeError:  # tobytes() is not available in python2
             res = ecmd.tostring()
-        _, size, perm_addr = struct.unpack("II%is" % MAX_ADDR_LEN, res)
+        unused, size, perm_addr = struct.unpack("II%is" % MAX_ADDR_LEN, res)
         perm_addr = Util.mac_ntoa(perm_addr[:size])
     except IOError:
         perm_addr = None

module_utils/network_lsr/myerror.py

@@ -0,0 +1,10 @@
+# vim: fileencoding=utf8
+# SPDX-License-Identifier: BSD-3-Clause
+
+from __future__ import absolute_import, division, print_function
+
+__metaclass__ = type
+
+
+class MyError(Exception):
+    pass

module_utils/network_lsr/nm/__init__.py

@@ -1,5 +1,9 @@
 # Relative import is not support by ansible 2.8 yet
 # pylint: disable=import-error, no-name-in-module
+from __future__ import absolute_import, division, print_function
+
+__metaclass__ = type
+
 from ansible.module_utils.network_lsr.nm import provider  # noqa:E501
 
 # pylint: enable=import-error, no-name-in-module

module_utils/network_lsr/nm/active_connection.py

@@ -2,6 +2,10 @@
 
 # Handle NM.ActiveConnection
 
+from __future__ import absolute_import, division, print_function
+
+__metaclass__ = type
+
 import logging
 
 # Relative import is not support by ansible 2.8 yet
@@ -21,19 +25,15 @@ def deactivate_active_connection(nm_ac, timeout, check_mode):
         return False
     if not check_mode:
         main_loop = client.get_mainloop(timeout)
-        logging.debug(
+        logging.debug("Deactivating %s with timeout %s", nm_ac.get_id(), timeout)
-            "Deactivating {id} with timeout {timeout}".format(
-                id=nm_ac.get_id(), timeout=timeout
-            )
-        )
         user_data = main_loop
         handler_id = nm_ac.connect(
             NM_AC_STATE_CHANGED_SIGNAL, _nm_ac_state_change_callback, user_data
         )
         logging.debug(
-            "Registered {signal} on client.NM.ActiveConnection {id}".format(
+            "Registered %s on client.NM.ActiveConnection %s",
-                signal=NM_AC_STATE_CHANGED_SIGNAL, id=nm_ac.get_id()
+            NM_AC_STATE_CHANGED_SIGNAL,
-            )
+            nm_ac.get_id(),
         )
         if nm_ac.props.state != client.NM.ActiveConnectionState.DEACTIVATING:
             nm_client = client.get_client()
@@ -44,9 +44,7 @@ def deactivate_active_connection(nm_ac, timeout, check_mode):
                 _nm_ac_deactivate_call_back,
                 user_data,
             )
-            logging.debug(
+            logging.debug("Deactivating client.NM.ActiveConnection %s", nm_ac.get_id())
-                "Deactivating client.NM.ActiveConnection {0}".format(nm_ac.get_id())
-            )
         main_loop.run()
     return True
 
@@ -56,14 +54,13 @@ def _nm_ac_state_change_callback(nm_ac, state, reason, user_data):
     if main_loop.is_cancelled:
         return
     logging.debug(
-        "Got client.NM.ActiveConnection state change: {id}: {state} {reason}".format(
+        "Got client.NM.ActiveConnection state change: %s: %s %s",
-            id=nm_ac.get_id(), state=state, reason=reason
+        nm_ac.get_id(),
-        )
+        state,
+        reason,
     )
     if nm_ac.props.state == client.NM.ActiveConnectionState.DEACTIVATED:
-        logging.debug(
+        logging.debug("client.NM.ActiveConnection %s is deactivated", nm_ac.get_id())
-            "client.NM.ActiveConnection {0} is deactivated".format(nm_ac.get_id())
-        )
         main_loop.quit()
 
 
@@ -82,9 +79,7 @@ def _nm_ac_deactivate_call_back(nm_client, result, user_data):
             client.NM.ManagerError.quark(), client.NM.ManagerError.CONNECTIONNOTACTIVE
         ):
             logging.info(
-                "Connection is not active on {0}, no need to deactivate".format(
+                "Connection is not active on %s, no need to deactivate", nm_ac_id
-                    nm_ac_id
-                )
             )
             if nm_ac:
                 nm_ac.handler_disconnect(handler_id)

module_utils/network_lsr/nm/client.py

@@ -1,5 +1,9 @@
 # SPDX-License-Identifier: BSD-3-Clause
 
+from __future__ import absolute_import, division, print_function
+
+__metaclass__ = type
+
 import logging
 
 # Relative import is not support by ansible 2.8 yet

module_utils/network_lsr/nm/connection.py

@@ -2,6 +2,10 @@
 
 # Handle NM.RemoteConnection
 
+from __future__ import absolute_import, division, print_function
+
+__metaclass__ = type
+
 import logging
 
 # Relative import is not support by ansible 2.8 yet
@@ -26,9 +30,10 @@ def delete_remote_connection(nm_profile, timeout, check_mode):
             user_data,
         )
         logging.debug(
-            "Deleting profile {id}/{uuid} with timeout {timeout}".format(
+            "Deleting profile %s/%s with timeout %s",
-                id=nm_profile.get_id(), uuid=nm_profile.get_uuid(), timeout=timeout
+            nm_profile.get_id(),
-            )
+            nm_profile.get_uuid(),
+            timeout,
         )
         main_loop.run()
     return True
@@ -78,9 +83,10 @@ def volatilize_remote_connection(nm_profile, timeout, check_mode):
             user_data,
         )
         logging.debug(
-            "Volatilizing profile {id}/{uuid} with timeout {timeout}".format(
+            "Volatilizing profile %s/%s with timeout %s",
-                id=nm_profile.get_id(), uuid=nm_profile.get_uuid(), timeout=timeout
+            nm_profile.get_id(),
-            )
+            nm_profile.get_uuid(),
+            timeout,
         )
         main_loop.run()
     return True

module_utils/network_lsr/nm/error.py

@@ -1,5 +1,9 @@
 # SPDX-License-Identifier: BSD-3-Clause
 
+from __future__ import absolute_import, division, print_function
+
+__metaclass__ = type
+
 
 class LsrNetworkNmError(Exception):
     pass

module_utils/network_lsr/nm/provider.py

@@ -1,5 +1,9 @@
 # SPDX-License-Identifier: BSD-3-Clause
 
+from __future__ import absolute_import, division, print_function
+
+__metaclass__ = type
+
 import logging
 
 # Relative import is not support by ansible 2.8 yet
@@ -25,7 +29,7 @@ class NetworkManagerProvider:
                     nm_ac, timeout, check_mode
                 )
         if not changed:
-            logging.info("No active connection for {0}".format(connection_name))
+            logging.info("No active connection for %s", connection_name)
 
         return changed
 
@@ -49,7 +53,7 @@ class NetworkManagerProvider:
                         nm_profile, timeout, check_mode
                     )
         if not changed:
-            logging.info("No connection with UUID {0} to volatilize".format(uuid))
+            logging.info("No connection with UUID %s to volatilize", uuid)
 
         return changed
 

module_utils/network_lsr/nm_provider.py

@@ -1,6 +1,10 @@
 # SPDX-License-Identifier: BSD-3-Clause
 """ Support for NetworkManager aka the NM provider """
 
+from __future__ import absolute_import, division, print_function
+
+__metaclass__ = type
+
 # pylint: disable=import-error, no-name-in-module
 from ansible.module_utils.network_lsr.utils import Util  # noqa:E501
 

module_utils/network_lsr/utils.py

@@ -1,19 +1,23 @@
-#!/usr/bin/python3 -tt
 # SPDX-License-Identifier: BSD-3-Clause
 # vim: fileencoding=utf8
 
+from __future__ import absolute_import, division, print_function
+
+__metaclass__ = type
+
 import socket
 import sys
 import uuid
 
 # pylint: disable=import-error, no-name-in-module
-from ansible.module_utils.network_lsr import MyError  # noqa:E501
+from ansible.module_utils.network_lsr.myerror import MyError  # noqa:E501
 
 
 class Util:
 
     PY3 = sys.version_info[0] == 3
 
+    # pylint: disable=undefined-variable
     STRING_TYPE = str if PY3 else basestring  # noqa:F821
 
     @staticmethod
@@ -241,7 +245,8 @@ class Util:
                     n = int(c, 16) * 16
                     i = 1
                 else:
-                    assert i == 1
+                    if not i == 1:
+                        raise AssertionError("i != 1 - value is {0}".format(i))
                     n = n + int(c, 16)
                     i = 2
                     b.append(n)

tests/covstats

@@ -1,4 +1,4 @@
-#! /bin/bash
+#!/bin/bash
 # SPDX-License-Identifier: BSD-3-Clause
 
 if [ "$#" -lt 1 ]

tests/ensure_provider_tests.py

@@ -76,8 +76,6 @@ NM_ONLY_TESTS = {
         MINIMUM_VERSION: "'1.25.1'",
         "comment": "# NetworkManager 1.25.1 introduced ethtool coalesce support",
     },
-    "playbooks/tests_802_1x_updated.yml": {},
-    "playbooks/tests_802_1x.yml": {},
     "playbooks/tests_reapply.yml": {},
     # team interface is not supported on Fedora
     "playbooks/tests_team.yml": {
@@ -122,9 +120,7 @@ def create_nm_playbook(test_playbook):
         EXTRA_RUN_CONDITION, ""
     )
     if extra_run_condition:
-        extra_run_condition = "{}{}\n".format(
+        extra_run_condition = f"{EXTRA_RUN_CONDITION_PREFIX}{extra_run_condition}\n"
-            EXTRA_RUN_CONDITION_PREFIX, extra_run_condition
-        )
 
     nm_version_check = ""
     if minimum_nm_version:
@@ -217,7 +213,7 @@ def main():
 
     if missing:
         print("ERROR: No NM or initscripts tests found for:\n" + ", \n".join(missing))
-        print("Try to generate them with '{} generate'".format(sys.argv[0]))
+        print(f"Try to generate them with '{sys.argv[0]} generate'")
         returncode = 1
 
     return returncode

tests/get_coverage.sh

@@ -1,4 +1,4 @@
-#! /bin/bash
+#!/bin/bash
 # SPDX-License-Identifier: BSD-3-Clause
 
 if [ -n "${DEBUG}" ]
@@ -19,7 +19,6 @@ shift
 playbook="${1}"
 
 coverage_data="remote-coveragedata-${host}-${playbook%.yml}"
-coverage="/root/.local/bin/coverage"
 
 echo "Getting coverage for ${playbook} on ${host}" >&2
 
@@ -32,10 +31,15 @@ call_ansible() {
 }
 
 remote_coverage_dir="$(mktemp -d /tmp/remote_coverage-XXXXXX)"
+# we want to expand ${remote_coverage_dir} here, so tell SC to be quiet
+# https://github.com/koalaman/shellcheck/wiki/SC2064
+# shellcheck disable=SC2064
 trap "rm -rf '${remote_coverage_dir}'" EXIT
 ansible-playbook -i "${host}", get_coverage.yml -e "test_playbook=${playbook} destdir=${remote_coverage_dir}"
 
 #COVERAGE_FILE=remote-coverage coverage combine remote-coverage/tests_*/*/root/.coverage
+# https://github.com/koalaman/shellcheck/wiki/SC2046
+# shellcheck disable=SC2046
 ./merge_coverage.sh coverage "${coverage_data}"-tmp $(find "${remote_coverage_dir}" -type f | tr , _)
 
 cat > tmp_merge_coveragerc <<EOF

tests/get_total_coverage.sh

@@ -1,4 +1,4 @@
-#! /bin/bash
+#!/bin/bash
 # SPDX-License-Identifier: BSD-3-Clause
 
 set -e
@@ -12,7 +12,7 @@ then
     exit 1
 fi
 
-rm -f remote-coveragedata* "${coveragedata}"
+rm -f remote-coveragedata* "${coverage_data}"
 
 
 # collect pytest coverage

tests/integration/test_ethernet.py

@@ -25,10 +25,10 @@ with mock.patch.dict(
 class PytestRunEnvironment(nc.RunEnvironment):
     def log(self, connections, idx, severity, msg, **kwargs):
         if severity == nc.LogLevel.ERROR:
-            logging.error("Error: {}".format(connections[idx]))
+            logging.error("Error: %s", connections[idx])
             raise RuntimeError(msg)
         else:
-            logging.debug("Log: {}".format(connections[idx]))
+            logging.debug("Log: %s", connections[idx])
 
     def run_command(self, argv, encoding=None):
         command = subprocess.Popen(

tests/merge_coverage.sh

@@ -1,4 +1,4 @@
-#! /bin/bash
+#!/bin/bash
 # SPDX-License-Identifier: BSD-3-Clause
 
 if [ -n "${DEBUG}" ]
@@ -23,6 +23,9 @@ export COVERAGE_FILE="${1}"
 shift
 
 tempdir="$(mktemp -d /tmp/coverage_merge-XXXXXX)"
+# we want to expand ${tempdir} here, so tell SC to be quiet
+# https://github.com/koalaman/shellcheck/wiki/SC2064
+# shellcheck disable=SC2064
 trap "rm -rf '${tempdir}'" EXIT
 
 cp --backup=numbered -- "${@}" "${tempdir}"

tests/setup_module_utils.sh

@@ -1,41 +0,0 @@
-#!/bin/bash
-# SPDX-License-Identifier: MIT
-
-set -euo pipefail
-
-if [ -n "${DEBUG:-}" ] ; then
-    set -x
-fi
-
-if [ ! -d "${1:-}" ] ; then
-    echo Either ansible is not installed, or there is no ansible/module_utils
-    echo in $1 - Skipping
-    exit 0
-fi
-
-if [ ! -d "${2:-}" ] ; then
-    echo Role has no module_utils - Skipping
-    exit 0
-fi
-
-# we need absolute path for $2
-absmoddir=$( readlink -f "$2" )
-
-# clean up old links to module_utils
-for item in "$1"/* ; do
-    if lnitem=$( readlink "$item" ) && test -n "$lnitem" ; then
-        case "$lnitem" in
-            *"${2}"*) rm -f "$item" ;;
-        esac
-    fi
-done
-
-# add new links to module_utils
-for item in "$absmoddir"/* ; do
-    case "$item" in
-        *__pycache__) continue;;
-        *.pyc) continue;;
-    esac
-    bnitem=$( basename "$item" )
-    ln -s "$item" "$1/$bnitem"
-done

tox.ini

@@ -17,6 +17,3 @@ setenv =
     RUN_PYTEST_EXTRA_ARGS = -v
     RUN_FLAKE8_EXTRA_ARGS = --exclude tests/ensure_provider_tests.py,scripts/print_all_options.py,tests/network/ensure_provider_tests.py,.svn,CVS,.bzr,.hg,.git,__pycache__,.tox,.eggs,*.egg
     LSR_PUBLISH_COVERAGE = normal
-
-[testenv:shellcheck]
-commands = bash -c 'echo shellcheck is currently not enabled - please fix this'