Add tests for WANT_JSON and old style modules #50555
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
maxamillion
approved these changes
Jan 4, 2019
|
Awesome. 👍 |
ansibot
added
affects_2.8
ansibot
added
core_review
ansibot
added
needs_revision
|
The test |
ansibot
added
core_review
|
shipit |
ansibot
added
shipit
mattclay
requested changes
Jan 8, 2019
| else | ||
| echo '{"msg": "No argument file provided", "failed": True}' | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
Since this module includes an error check for no argument file, I'd include testing that in the integration test. Just execute it as a script instead of a module to verify it fails as expected.
There was a problem hiding this comment.
Addressed in the latest commit
| data = json.load(f) | ||
| except (IOError, OSError, IndexError): | ||
| print(json.dumps(dict(msg="No argument file provided", failed=True))) | ||
| sys.exit(1) |
There was a problem hiding this comment.
Since this module includes an error check for no argument file, I'd include testing that in the integration test. Just execute it as a script instead of a module to verify it fails as expected.
There was a problem hiding this comment.
Addressed in the latest commit
ansibot
added
needs_revision
mattclay
reviewed
Jan 8, 2019
| - assert: | ||
| that: | ||
| - direct is failed | ||
| - | |
There was a problem hiding this comment.
Why the multi-line syntax here?
There was a problem hiding this comment.
I had done it originally due to simplifying quoting, but after a change it's no longer needed, I just didn't remove it. It wasn't done for multiline, but rather as a block literal, to avoid the need for outer quotes which would have complicated things.
mattclay
approved these changes
Jan 8, 2019
kbreit
pushed a commit
to kbreit/ansible
that referenced
this pull request
Jan 11, 2019
* Add tests for WANT_JSON and old style modules * quote source path * Attempt to further appease tests * Check for file and not just exists * omg don't be dumb * moar fixes * shellcheck is the worst :) * Test the custom modules for failure without arg files
knumskull
pushed a commit
to knumskull/ansible
that referenced
this pull request
Jan 21, 2019
* Add tests for WANT_JSON and old style modules * quote source path * Attempt to further appease tests * Check for file and not just exists * omg don't be dumb * moar fixes * shellcheck is the worst :) * Test the custom modules for failure without arg files
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
SUMMARY
Add tests for WANT_JSON and old style modules
ISSUE TYPE
COMPONENT NAME
test/integration/targets/old_style_modules_posix
test/integration/targets/want_json_modules_posix
ADDITIONAL INFORMATION