Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix test-module failing to validate args #41004

Merged
merged 6 commits into from
Jun 1, 2018
Merged

Conversation

HD650
Copy link
Contributor

@HD650 HD650 commented Jun 1, 2018

SUMMARY

The test-module pass a wrong argument _ansible_tmp cause the validation failed.
Change the argument _ansible_tmp to _ansible_tmpdir to fix this.

Fixes #40817
Co-authored-by: Sviat wk@sydorenko.org.ua

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

test-module

ANSIBLE VERSION
devel
ADDITIONAL INFORMATION

N/A


The test-module pass a wrong argument _ansible_tmp cause the validation failed.
Change the argument _ansible_tmp to _ansible_tmpdir to fix this.
@ansibot ansibot added affects_2.7 This issue/PR affects Ansible v2.7 bug This issue/PR relates to a bug. needs_triage Needs a first human triage before being processed. new_contributor This PR is the first contribution by a new community member. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Jun 1, 2018
Prior to this change, we don't have a test for test-module.

This change ensure the correctness of test-module script.
@ansibot ansibot added support:community This issue/PR relates to code supported by the Ansible community. test This PR relates to tests. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Jun 1, 2018
zhikangzhang added 2 commits June 1, 2018 10:16
Prior to this change,

This change
@abadger
Copy link
Contributor

abadger commented Jun 1, 2018

Tests are failing for an unrelated reason.

+1 to merge

EDIT Oops, frrebsd tests are failing for a valid reason. See sivel's message.



PING_MODULE_PATH="$(pwd)/../../../../lib/ansible/modules/system/ping.py"
../../../../hacking/test-module -m "$PING_MODULE_PATH"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests are failing because freebsd doesn't have /usr/bin/python, so you will need to adjust this to use something like:

../../../../hacking/test-module -m "$PING_MODULE_PATH" -I ansible_python_interpreter=$(which python)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All right, I will try this, thanks!

@s-hertel s-hertel removed the needs_triage Needs a first human triage before being processed. label Jun 1, 2018
zhikangzhang added 2 commits June 1, 2018 11:27
Prior to this change, test-module will raise "No such file" error since freebsd doesn't have /usr/bin/python.
Make sure runme.sh can pass the shell sanity check.
@ansibot
Copy link
Contributor

ansibot commented Jun 1, 2018

The test ansible-test sanity --test shellcheck [explain] failed with 1 error:

test/integration/targets/test_infra/runme.sh:27:86: SC2046 Quote this to prevent word splitting.

click here for bot help

@ansibot ansibot added the ci_verified Changes made in this PR are causing tests to fail. label Jun 1, 2018

# ensure test-module script works well
PING_MODULE_PATH="$(pwd)/../../../../lib/ansible/modules/system/ping.py"
../../../../hacking/test-module -m "$PING_MODULE_PATH" -I ansible_python_interpreter=$(which python)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test/integration/targets/test_infra/runme.sh:27:86: SC2046 Quote this to prevent word splitting.

@HD650 this linter message means that in the event of command in $(...) expression returning smth with space the command fill likely break. For example, if it returns /usr/bin/env python it will result in smth like ... -I ansible_python_interpreter=/usr/bin/env python, which is not what you want. Instead, you expect all of it to go to the same arg, like ... -I ansible_python_interpreter="/usr/bin/env python".

To fix this, just wrap argument with double quotes " (single quotes prevent bash expression interpolation, which will break things).

@ansibot ansibot removed ci_verified Changes made in this PR are causing tests to fail. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Jun 1, 2018
@abadger abadger merged commit b578bf9 into ansible:devel Jun 1, 2018
@abadger
Copy link
Contributor

abadger commented Jun 1, 2018

Good work! Merged to devel for 2.7

@webknjaz
Copy link
Member

webknjaz commented Jun 1, 2018

FTR: this regression has been introduced by https://github.com/ansible/ansible/pull/39833/files#diff-3934fe5332a78d0a3719f8e468f72272L128 18 days ago

HD650 pushed a commit to HD650/ansible that referenced this pull request Jun 4, 2018
* Fix test-module failing to validate args

The test-module pass a wrong argument _ansible_tmp cause the validation failed.
Change the argument _ansible_tmp to _ansible_tmpdir to fix this.

* Add a integration test for test-module.

Prior to this change, we don't have a test for test-module.

This change ensure the correctness of test-module script.
mattclay pushed a commit that referenced this pull request Jun 4, 2018
* Fix test-module failing to validate args

The test-module pass a wrong argument _ansible_tmp cause the validation failed.
Change the argument _ansible_tmp to _ansible_tmpdir to fix this.

* Add a integration test for test-module.

Prior to this change, we don't have a test for test-module.

This change ensure the correctness of test-module script.
jacum pushed a commit to jacum/ansible that referenced this pull request Jun 26, 2018
* Fix test-module failing to validate args

The test-module pass a wrong argument _ansible_tmp cause the validation failed.
Change the argument _ansible_tmp to _ansible_tmpdir to fix this.

* Add a integration test for test-module.

Prior to this change, we don't have a test for test-module.

This change ensure the correctness of test-module script.
@ansible ansible locked and limited conversation to collaborators Jun 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.7 This issue/PR affects Ansible v2.7 bug This issue/PR relates to a bug. new_contributor This PR is the first contribution by a new community member. support:community This issue/PR relates to code supported by the Ansible community. support:core This issue/PR relates to code supported by the Ansible Engineering Team. test This PR relates to tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

hacking/test-module adds an unexpected "_ansible_tmp" parameter
6 participants