Skip to content

Commit

Permalink
Remove --delegated pytest option (#2458)
Browse files Browse the repository at this point in the history
This extension prevented execution of pytest from root of project and
introduced risks of bypassing funtional testing by default.

If someone want to avoid delegate testing the use of pytest marks is
a much easier approach, one that does not need extra code.

Signed-off-by: Sorin Sbarnea <[email protected]>
rm
  • Loading branch information
ssbarnea committed Dec 6, 2019
1 parent 5ae718c commit 36fc6bd
Show file tree
Hide file tree
Showing 13 changed files with 66 additions and 110 deletions.
5 changes: 4 additions & 1 deletion molecule/driver/docker.py
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,10 @@ def login_options(self, instance_name):
return {'instance': instance_name}

def ansible_connection_options(self, instance_name):
return {'ansible_connection': 'docker'}
x = {'ansible_connection': 'docker'}
if 'DOCKER_HOST' in os.environ:
x['ansible_docker_extra_args'] = "-H={}".format(os.environ['DOCKER_HOST'])
return x

@lru_cache()
def sanity_checks(self):
Expand Down
6 changes: 0 additions & 6 deletions molecule/test/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,12 +110,6 @@ def molecule_ephemeral_directory(_fixture_uuid):
)


def pytest_addoption(parser):
parser.addoption(
'--delegated', action='store_true', help='Run delegated driver tests.'
)


def pytest_collection_modifyitems(items, config):

marker = config.getoption('-m')
Expand Down
13 changes: 2 additions & 11 deletions molecule/test/functional/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,15 +68,11 @@ def with_scenario(request, scenario_to_test, driver_name, scenario_name, skip_te

@pytest.fixture
def skip_test(request, driver_name):
msg_tmpl = (
"Ignoring '{}' tests for now"
if driver_name == 'delegated'
else "Skipped '{}' not supported"
)
msg_tmpl = "Skipped '{}' not supported"
support_checks_map = {
'docker': supports_docker,
'podman': lambda: min_ansible("2.8.6") and platform.system() != 'Darwin',
'delegated': demands_delegated,
'delegated': lambda: True,
}
try:
check_func = support_checks_map[driver_name]
Expand Down Expand Up @@ -264,8 +260,3 @@ def min_ansible(version):
except ImportError as exception:
LOG.error('Unable to parse Ansible version', exc_info=exception)
return False


@pytest.helpers.register
def demands_delegated():
return pytest.config.getoption('--delegated')
39 changes: 17 additions & 22 deletions molecule/test/functional/test_command.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ def driver_name(request):
'scenario_to_test, driver_name, scenario_name',
[
('driver/docker', 'docker', 'default'),
('driver/delegated', 'delegated', 'docker'),
('driver/delegated', 'delegated', 'default'),
('driver/podman', 'podman', 'default'),
],
indirect=['scenario_to_test', 'driver_name', 'scenario_name'],
Expand All @@ -62,7 +62,7 @@ def test_command_check(scenario_to_test, with_scenario, scenario_name):
'scenario_to_test, driver_name, scenario_name',
[
('driver/docker', 'docker', 'default'),
('driver/delegated', 'delegated', 'docker'),
('driver/delegated', 'delegated', 'default'),
('driver/podman', 'podman', 'default'),
],
indirect=['scenario_to_test', 'driver_name', 'scenario_name'],
Expand All @@ -77,7 +77,7 @@ def test_command_cleanup(scenario_to_test, with_scenario, scenario_name):
'scenario_to_test, driver_name, scenario_name',
[
('driver/docker', 'docker', 'default'),
('driver/delegated', 'delegated', 'docker'),
('driver/delegated', 'delegated', 'default'),
('driver/podman', 'podman', 'default'),
],
indirect=['scenario_to_test', 'driver_name', 'scenario_name'],
Expand All @@ -92,7 +92,7 @@ def test_command_converge(scenario_to_test, with_scenario, scenario_name):
'scenario_to_test, driver_name, scenario_name',
[
('driver/docker', 'docker', 'default'),
('driver/delegated', 'delegated', 'docker'),
('driver/delegated', 'delegated', 'default'),
('driver/podman', 'podman', 'default'),
],
indirect=['scenario_to_test', 'driver_name', 'scenario_name'],
Expand Down Expand Up @@ -171,7 +171,7 @@ def test_command_dependency_shell(
'scenario_to_test, driver_name, scenario_name',
[
('driver/docker', 'docker', 'default'),
('driver/delegated', 'delegated', 'docker'),
('driver/delegated', 'delegated', 'default'),
('driver/podman', 'podman', 'default'),
],
indirect=['scenario_to_test', 'driver_name', 'scenario_name'],
Expand All @@ -186,7 +186,7 @@ def test_command_destroy(scenario_to_test, with_scenario, scenario_name):
'scenario_to_test, driver_name, scenario_name',
[
('driver/docker', 'docker', 'default'),
('driver/delegated', 'delegated', 'docker'),
('driver/delegated', 'delegated', 'default'),
('driver/podman', 'podman', 'default'),
],
indirect=['scenario_to_test', 'driver_name', 'scenario_name'],
Expand All @@ -213,7 +213,7 @@ def test_command_init_scenario(temp_dir, driver_name, skip_test):
'scenario_to_test, driver_name, scenario_name',
[
('driver/docker', 'docker', 'default'),
('driver/delegated', 'delegated', 'docker'),
('driver/delegated', 'delegated', 'default'),
('driver/podman', 'podman', 'default'),
],
indirect=['scenario_to_test', 'driver_name', 'scenario_name'],
Expand Down Expand Up @@ -243,9 +243,9 @@ def test_command_lint(scenario_to_test, with_scenario, scenario_name):
'driver/delegated',
'delegated',
"""
Instance Name Driver Name Provisioner Name Scenario Name Created Converged
---------------------------- ------------- ------------------ --------------- --------- -----------
delegated-instance-docker delegated ansible docker unknown true
Instance Name Driver Name Provisioner Name Scenario Name Created Converged
--------------- ------------- ------------------ --------------- --------- -----------
instance delegated ansible default false false
""".strip(),
), # noqa
(
Expand Down Expand Up @@ -284,7 +284,7 @@ def test_command_list(scenario_to_test, with_scenario, expected):
'driver/delegated',
'delegated',
"""
delegated-instance-docker delegated ansible docker unknown true
instance delegated ansible default false false
""".strip(),
),
(
Expand Down Expand Up @@ -314,12 +314,7 @@ def test_command_list_with_format_plain(scenario_to_test, with_scenario, expecte
[['instance-1', '.*instance-1.*'], ['instance-2', '.*instance-2.*']],
'multi-node',
),
(
'driver/delegated',
'delegated',
[['delegated-instance-docker', '.*delegated-instance-docker.*']],
'docker',
),
('driver/delegated', 'delegated', [['instance', '.*instance.*']], 'default',),
(
'driver/podman',
'podman',
Expand All @@ -337,7 +332,7 @@ def test_command_login(scenario_to_test, with_scenario, login_args, scenario_nam
'scenario_to_test, driver_name, scenario_name',
[
('driver/docker', 'docker', 'default'),
('driver/delegated', 'delegated', 'docker'),
('driver/delegated', 'delegated', 'default'),
('driver/podman', 'podman', 'default'),
],
indirect=['scenario_to_test', 'driver_name', 'scenario_name'],
Expand All @@ -356,7 +351,7 @@ def test_command_prepare(scenario_to_test, with_scenario, scenario_name):
'scenario_to_test, driver_name, scenario_name',
[
('driver/docker', 'docker', 'default'),
('driver/delegated', 'delegated', 'docker'),
('driver/delegated', 'delegated', 'default'),
('driver/podman', 'podman', 'default'),
],
indirect=['scenario_to_test', 'driver_name', 'scenario_name'],
Expand All @@ -371,7 +366,7 @@ def test_command_side_effect(scenario_to_test, with_scenario, scenario_name):
'scenario_to_test, driver_name, scenario_name',
[
('driver/docker', 'docker', 'default'),
('driver/delegated', 'delegated', 'docker'),
('driver/delegated', 'delegated', 'default'),
('driver/podman', 'podman', 'default'),
],
indirect=['scenario_to_test', 'driver_name', 'scenario_name'],
Expand All @@ -388,7 +383,7 @@ def test_command_syntax(scenario_to_test, with_scenario, scenario_name):
('driver/docker', 'docker', 'default'),
('driver/docker', 'docker', 'ansible-verifier'),
('driver/docker', 'docker', 'multi-node'),
('driver/delegated', 'delegated', 'docker'),
('driver/delegated', 'delegated', 'default'),
('driver/podman', 'podman', 'default'),
],
indirect=['scenario_to_test', 'driver_name', 'scenario_name'],
Expand All @@ -401,7 +396,7 @@ def test_command_test(scenario_to_test, with_scenario, scenario_name, driver_nam
'scenario_to_test, driver_name, scenario_name',
[
('driver/docker', 'docker', 'default'),
('driver/delegated', 'delegated', 'docker'),
('driver/delegated', 'delegated', 'default'),
('driver/podman', 'podman', 'default'),
],
indirect=['scenario_to_test', 'driver_name', 'scenario_name'],
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
- name: Create
hosts: all
gather_facts: false
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
- name: Destroy
hosts: all
gather_facts: false
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
---
driver:
name: delegated
platforms:
- name: instance
provisioner:
name: ansible
inventory:
hosts:
all:
hosts:
instance:
ansible_host: localhost
scenario:
name: default
default_sequence:
- converge
test_sequence:
# - prepare
- converge
# - verify
verify_sequence:
- converge
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
- name: Converge
hosts: all
gather_facts: false
become: false

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

9 changes: 7 additions & 2 deletions molecule/test/unit/command/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
# DEALINGS IN THE SOFTWARE.

import os
import pytest


Expand All @@ -28,7 +28,7 @@ def command_patched_ansible_create(mocker):

@pytest.fixture
def command_driver_delegated_section_data():
return {
x = {
'driver': {
'name': 'delegated',
'options': {
Expand All @@ -38,6 +38,11 @@ def command_driver_delegated_section_data():
},
}
}
if 'DOCKER_HOST' in os.environ:
x['driver']['options']['ansible_docker_extra_args'] = "-H={}".format(
os.environ['DOCKER_HOST']
)
return x


@pytest.fixture
Expand Down

0 comments on commit 36fc6bd

Please sign in to comment.