diff --git a/changelogs/fragments/6989-npm-cmdrunner.yml b/changelogs/fragments/6989-npm-cmdrunner.yml new file mode 100644 index 0000000000..bc55fce24b --- /dev/null +++ b/changelogs/fragments/6989-npm-cmdrunner.yml @@ -0,0 +1,2 @@ +minor_changes: + - npm - module now using ``CmdRunner`` to execute external commands (https://github.com/ansible-collections/community.general/pull/6989). diff --git a/plugins/modules/npm.py b/plugins/modules/npm.py index 3ad2a64793..e6dc0b772a 100644 --- a/plugins/modules/npm.py +++ b/plugins/modules/npm.py @@ -150,6 +150,7 @@ import re from ansible.module_utils.basic import AnsibleModule from ansible.module_utils.common.text.converters import to_native +from ansible_collections.community.general.plugins.module_utils.cmd_runner import CmdRunner, cmd_runner_fmt class Npm(object): @@ -172,33 +173,29 @@ class Npm(object): else: self.executable = [module.get_bin_path('npm', True)] - if kwargs['version'] and self.state != 'absent': - self.name_version = self.name + '@' + str(self.version) + if kwargs['version'] and kwargs['state'] != 'absent': + self.name_version = self.name + '@' + str(kwargs['version']) else: self.name_version = self.name + self.runner = CmdRunner( + module, + command=self.executable, + arg_formats=dict( + exec_args=cmd_runner_fmt.as_list(), + global_=cmd_runner_fmt.as_bool('--global'), + production=cmd_runner_fmt.as_bool('--production'), + ignore_scripts=cmd_runner_fmt.as_bool('--ignore-scripts'), + unsafe_perm=cmd_runner_fmt.as_bool('--unsafe-perm'), + name_version=cmd_runner_fmt.as_list(), + registry=cmd_runner_fmt.as_opt_val('--registry'), + no_optional=cmd_runner_fmt.as_bool('--no-optional'), + no_bin_links=cmd_runner_fmt.as_bool('--no-bin-links'), + ) + ) + def _exec(self, args, run_in_check_mode=False, check_rc=True, add_package_name=True): if not self.module.check_mode or (self.module.check_mode and run_in_check_mode): - cmd = self.executable + args - - if self.glbl: - cmd.append('--global') - if self.production and ('install' in cmd or 'update' in cmd or 'ci' in cmd): - cmd.append('--production') - if self.ignore_scripts: - cmd.append('--ignore-scripts') - if self.unsafe_perm: - cmd.append('--unsafe-perm') - if self.name_version and add_package_name: - cmd.append(self.name_version) - if self.registry: - cmd.append('--registry') - cmd.append(self.registry) - if self.no_optional: - cmd.append('--no-optional') - if self.no_bin_links: - cmd.append('--no-bin-links') - # If path is specified, cd into that path and run the command. cwd = None if self.path: @@ -208,8 +205,19 @@ class Npm(object): self.module.fail_json(msg="path %s is not a directory" % self.path) cwd = self.path - rc, out, err = self.module.run_command(cmd, check_rc=check_rc, cwd=cwd) + params = dict(self.module.params) + params['exec_args'] = args + params['global_'] = self.glbl + params['production'] = self.production and ('install' in args or 'update' in args or 'ci' in args) + params['name_version'] = self.name_version if add_package_name else None + + with self.runner( + "exec_args global_ production ignore_scripts unsafe_perm name_version registry no_optional no_bin_links", + check_rc=check_rc, cwd=cwd + ) as ctx: + rc, out, err = ctx.run(**params) return out + return '' def list(self): @@ -269,12 +277,12 @@ class Npm(object): def main(): arg_spec = dict( - name=dict(default=None, type='str'), - path=dict(default=None, type='path'), - version=dict(default=None, type='str'), + name=dict(type='str'), + path=dict(type='path'), + version=dict(type='str'), production=dict(default=False, type='bool'), - executable=dict(default=None, type='path'), - registry=dict(default=None, type='str'), + executable=dict(type='path'), + registry=dict(type='str'), state=dict(default='present', choices=['present', 'absent', 'latest']), ignore_scripts=dict(default=False, type='bool'), unsafe_perm=dict(default=False, type='bool'), @@ -293,25 +301,27 @@ def main(): path = module.params['path'] version = module.params['version'] glbl = module.params['global'] - production = module.params['production'] - executable = module.params['executable'] - registry = module.params['registry'] state = module.params['state'] - ignore_scripts = module.params['ignore_scripts'] - unsafe_perm = module.params['unsafe_perm'] - ci = module.params['ci'] - no_optional = module.params['no_optional'] - no_bin_links = module.params['no_bin_links'] if not path and not glbl: module.fail_json(msg='path must be specified when not using global') - npm = Npm(module, name=name, path=path, version=version, glbl=glbl, production=production, - executable=executable, registry=registry, ignore_scripts=ignore_scripts, - unsafe_perm=unsafe_perm, state=state, no_optional=no_optional, no_bin_links=no_bin_links) + npm = Npm(module, + name=name, + path=path, + version=version, + glbl=glbl, + production=module.params['production'], + executable=module.params['executable'], + registry=module.params['registry'], + ignore_scripts=module.params['ignore_scripts'], + unsafe_perm=module.params['unsafe_perm'], + state=state, + no_optional=module.params['no_optional'], + no_bin_links=module.params['no_bin_links']) changed = False - if ci: + if module.params['ci']: npm.ci_install() changed = True elif state == 'present': diff --git a/tests/unit/plugins/modules/test_npm.py b/tests/unit/plugins/modules/test_npm.py index f5d3127759..cc4d651726 100644 --- a/tests/unit/plugins/modules/test_npm.py +++ b/tests/unit/plugins/modules/test_npm.py @@ -48,8 +48,8 @@ class NPMModuleTestCase(ModuleTestCase): self.assertTrue(result['changed']) self.module_main_command.assert_has_calls([ - call(['/testbin/npm', 'list', '--json', '--long', '--global'], check_rc=False, cwd=None), - call(['/testbin/npm', 'install', '--global', 'coffee-script'], check_rc=True, cwd=None), + call(['/testbin/npm', 'list', '--json', '--long', '--global'], check_rc=False, cwd=None, environ_update={'LANGUAGE': 'C', 'LC_ALL': 'C'}), + call(['/testbin/npm', 'install', '--global', 'coffee-script'], check_rc=True, cwd=None, environ_update={'LANGUAGE': 'C', 'LC_ALL': 'C'}), ]) def test_present_missing(self): @@ -67,8 +67,8 @@ class NPMModuleTestCase(ModuleTestCase): self.assertTrue(result['changed']) self.module_main_command.assert_has_calls([ - call(['/testbin/npm', 'list', '--json', '--long', '--global'], check_rc=False, cwd=None), - call(['/testbin/npm', 'install', '--global', 'coffee-script'], check_rc=True, cwd=None), + call(['/testbin/npm', 'list', '--json', '--long', '--global'], check_rc=False, cwd=None, environ_update={'LANGUAGE': 'C', 'LC_ALL': 'C'}), + call(['/testbin/npm', 'install', '--global', 'coffee-script'], check_rc=True, cwd=None, environ_update={'LANGUAGE': 'C', 'LC_ALL': 'C'}), ]) def test_present_version(self): @@ -87,8 +87,8 @@ class NPMModuleTestCase(ModuleTestCase): self.assertTrue(result['changed']) self.module_main_command.assert_has_calls([ - call(['/testbin/npm', 'list', '--json', '--long', '--global'], check_rc=False, cwd=None), - call(['/testbin/npm', 'install', '--global', 'coffee-script@2.5.1'], check_rc=True, cwd=None), + call(['/testbin/npm', 'list', '--json', '--long', '--global'], check_rc=False, cwd=None, environ_update={'LANGUAGE': 'C', 'LC_ALL': 'C'}), + call(['/testbin/npm', 'install', '--global', 'coffee-script@2.5.1'], check_rc=True, cwd=None, environ_update={'LANGUAGE': 'C', 'LC_ALL': 'C'}), ]) def test_present_version_update(self): @@ -107,8 +107,8 @@ class NPMModuleTestCase(ModuleTestCase): self.assertTrue(result['changed']) self.module_main_command.assert_has_calls([ - call(['/testbin/npm', 'list', '--json', '--long', '--global'], check_rc=False, cwd=None), - call(['/testbin/npm', 'install', '--global', 'coffee-script@2.5.1'], check_rc=True, cwd=None), + call(['/testbin/npm', 'list', '--json', '--long', '--global'], check_rc=False, cwd=None, environ_update={'LANGUAGE': 'C', 'LC_ALL': 'C'}), + call(['/testbin/npm', 'install', '--global', 'coffee-script@2.5.1'], check_rc=True, cwd=None, environ_update={'LANGUAGE': 'C', 'LC_ALL': 'C'}), ]) def test_present_version_exists(self): @@ -127,7 +127,7 @@ class NPMModuleTestCase(ModuleTestCase): self.assertFalse(result['changed']) self.module_main_command.assert_has_calls([ - call(['/testbin/npm', 'list', '--json', '--long', '--global'], check_rc=False, cwd=None), + call(['/testbin/npm', 'list', '--json', '--long', '--global'], check_rc=False, cwd=None, environ_update={'LANGUAGE': 'C', 'LC_ALL': 'C'}), ]) def test_absent(self): @@ -145,8 +145,8 @@ class NPMModuleTestCase(ModuleTestCase): self.assertTrue(result['changed']) self.module_main_command.assert_has_calls([ - call(['/testbin/npm', 'list', '--json', '--long', '--global'], check_rc=False, cwd=None), - call(['/testbin/npm', 'uninstall', '--global', 'coffee-script'], check_rc=True, cwd=None), + call(['/testbin/npm', 'list', '--json', '--long', '--global'], check_rc=False, cwd=None, environ_update={'LANGUAGE': 'C', 'LC_ALL': 'C'}), + call(['/testbin/npm', 'uninstall', '--global', 'coffee-script'], check_rc=True, cwd=None, environ_update={'LANGUAGE': 'C', 'LC_ALL': 'C'}), ]) def test_absent_version(self): @@ -165,8 +165,8 @@ class NPMModuleTestCase(ModuleTestCase): self.assertTrue(result['changed']) self.module_main_command.assert_has_calls([ - call(['/testbin/npm', 'list', '--json', '--long', '--global'], check_rc=False, cwd=None), - call(['/testbin/npm', 'uninstall', '--global', 'coffee-script'], check_rc=True, cwd=None), + call(['/testbin/npm', 'list', '--json', '--long', '--global'], check_rc=False, cwd=None, environ_update={'LANGUAGE': 'C', 'LC_ALL': 'C'}), + call(['/testbin/npm', 'uninstall', '--global', 'coffee-script'], check_rc=True, cwd=None, environ_update={'LANGUAGE': 'C', 'LC_ALL': 'C'}), ]) def test_absent_version_different(self): @@ -185,8 +185,8 @@ class NPMModuleTestCase(ModuleTestCase): self.assertTrue(result['changed']) self.module_main_command.assert_has_calls([ - call(['/testbin/npm', 'list', '--json', '--long', '--global'], check_rc=False, cwd=None), - call(['/testbin/npm', 'uninstall', '--global', 'coffee-script'], check_rc=True, cwd=None), + call(['/testbin/npm', 'list', '--json', '--long', '--global'], check_rc=False, cwd=None, environ_update={'LANGUAGE': 'C', 'LC_ALL': 'C'}), + call(['/testbin/npm', 'uninstall', '--global', 'coffee-script'], check_rc=True, cwd=None, environ_update={'LANGUAGE': 'C', 'LC_ALL': 'C'}), ]) def test_present_package_json(self): @@ -203,7 +203,7 @@ class NPMModuleTestCase(ModuleTestCase): self.assertTrue(result['changed']) self.module_main_command.assert_has_calls([ - call(['/testbin/npm', 'install', '--global'], check_rc=True, cwd=None), + call(['/testbin/npm', 'install', '--global'], check_rc=True, cwd=None, environ_update={'LANGUAGE': 'C', 'LC_ALL': 'C'}), ]) def test_present_package_json_production(self): @@ -221,7 +221,7 @@ class NPMModuleTestCase(ModuleTestCase): self.assertTrue(result['changed']) self.module_main_command.assert_has_calls([ - call(['/testbin/npm', 'install', '--global', '--production'], check_rc=True, cwd=None), + call(['/testbin/npm', 'install', '--global', '--production'], check_rc=True, cwd=None, environ_update={'LANGUAGE': 'C', 'LC_ALL': 'C'}), ]) def test_present_package_json_ci(self): @@ -239,7 +239,7 @@ class NPMModuleTestCase(ModuleTestCase): self.assertTrue(result['changed']) self.module_main_command.assert_has_calls([ - call(['/testbin/npm', 'ci', '--global'], check_rc=True, cwd=None), + call(['/testbin/npm', 'ci', '--global'], check_rc=True, cwd=None, environ_update={'LANGUAGE': 'C', 'LC_ALL': 'C'}), ]) def test_present_package_json_ci_production(self): @@ -258,5 +258,5 @@ class NPMModuleTestCase(ModuleTestCase): self.assertTrue(result['changed']) self.module_main_command.assert_has_calls([ - call(['/testbin/npm', 'ci', '--global', '--production'], check_rc=True, cwd=None), + call(['/testbin/npm', 'ci', '--global', '--production'], check_rc=True, cwd=None, environ_update={'LANGUAGE': 'C', 'LC_ALL': 'C'}), ])