Move ping and win_ping closer together

[dagwieers]

SUMMARY

So in an effort to verify if Windows modules are feature complete
compared to the python equivalent, I stumbled upon these differences.

ISSUE TYPE

• Feature Pull Request
• Docs Pull Request

COMPONENT NAME

ping and win_ping

ANSIBLE VERSION

v2.4

[ansibot]

cc @nitzmahone
click here for bot help

[ansibot]

The test ansible-test sanity --test validate-modules failed with the following errors:

lib/ansible/modules/system/ping.py:0:0: E309 version_added for new option (data) should be 2.4. Currently 0.0
lib/ansible/modules/system/ping.py:53:1: E311 EXAMPLES is not valid YAML

click here for bot help

[ansibot]

The test ansible-test sanity --test validate-modules failed with the following error:

lib/ansible/modules/system/ping.py:0:0: E309 version_added for new option (data) should be 2.4. Currently 0.0

click here for bot help

[dagwieers]

@mattclay The data parameter always existed, but was undocumented.

[jhawkesworth]

Code looks good to me. Not sure currently how to get around the version_added check failure.

[dagwieers]

@jhawkesworth I assume that once merged it no longer is an issue, so I don't mind except that the powers that be won't merge it because of the CI failure :-(

[jhawkesworth]

Yeah, I don't think the CI will check it on every build from now on, but I don't want to press merge and find out the hard way.

[dagwieers]

@jhawkesworth No, I don't think we want to merge this PR just now. This is a core change.

[dagwieers]

!needs_revision

[ansibot]

The test ansible-test sanity --test validate-modules failed with the following error:

lib/ansible/modules/system/ping.py:0:0: E309 version_added for new option (data) should be 2.4. Currently 0.0

click here for bot help

[willthames]

This change looks good to me. The version_added failure is annoying - can only hope that this would be the only failure and that future PRs based on this change won't fail. My suggestion is to merge and watch.

[ansibot]

The test ansible-test sanity --test validate-modules failed with the following error:

lib/ansible/modules/system/ping.py:0:0: E309 version_added for new option (data) should be 2.4. Currently 0.0

click here for bot help

[mattclay]

CI failure in integration tests test/integration/targets/ping/tasks/main.yml:48 / [testhost] testhost: ping : assert the ping failed with data=boom that=[u'result|failed', u'not result|changed', u"'Exception: boom' in result.module_stderr", u'result.exception == true']:

{
    "assertion": "result.exception == true", 
    "changed": false, 
    "evaluated_to": false, 
    "failed": true
}

[mattclay]

Once all the errors besides E309 have been resolved, merging it shouldn't be an issue. You'll have to look at the Shippable results carefully though -- the bot won't report every failure here.

[dagwieers]

Well, the exception integration test was recently added to improve coverage.

[ansibot]

The test ansible-test sanity --test validate-modules failed with the following error:

lib/ansible/modules/system/ping.py:0:0: E309 version_added for new option (data) should be 2.4. Currently 0.0

click here for bot help

[ansibot]

The test ansible-test sanity --test validate-modules failed with the following error:

lib/ansible/modules/system/ping.py:0:0: E309 version_added for new option (data) should be 2.4. Currently 0.0

click here for bot help

[mattclay]

Currently the only CI failure is the E309 error, which can be ignored.

[dagwieers]

Yeah, so I think it is again in a state to be merged.
Sorry for the confusion, I was convinced E309 was the only issue left.

[abadger]

Looks good; +1 from me. @nitzmahone or @jborean93 ?