Repository: ansible/ansible
Pull request: Restore ansible --version output
##### SUMMARY

Fixes #55710

The default formatting class used by `argparse` strips newline characters, resulting in the output of `ansible --version` being a single string.

Implement a custom action to print out the already formatted version string and add tests to prevent future regressions.

##### ISSUE TYPE
<!--- Pick one below and delete the rest -->
- Bugfix Pull Request

##### COMPONENT NAME
`/lib/ansible/cli/arguments/option_helpers.py`

##### ADDITIONAL INFORMATION
This only affects 2.9.

Problem statement:
Ansible --version no longer prints version on new line
##### SUMMARY
The syntax of the `ansible --version` changed unexpectedly, and is now more complex to parse, and breaks version parsing tools.

##### ISSUE TYPE
- Bug Report

##### COMPONENT NAME
lib/ansible/cli/adhoc.py

##### ANSIBLE VERSION
```paste below
ansible --version
ansible 2.9.0.dev0 config file = None configured module search path =
['/Users/alancoding/.ansible/plugins/modules',
'/usr/share/ansible/plugins/modules'] ansible python module location =
/Users/alancoding/Documents/repos/ansible/lib/ansible executable location =
/Users/alancoding/.virtualenvs/ansible3/bin/ansible python version = 3.6.5
(default, Apr 25 2018, 14:23:58) [GCC 4.2.1 Compatible Apple LLVM 9.1.0
(clang-902.0.39.1)]

```

##### CONFIGURATION
(only defaults)

##### OS / ENVIRONMENT
Local machine, N/A


##### STEPS TO REPRODUCE
`ansible --version`

##### EXPECTED RESULTS
A line break after the version number

```
ansible --version
ansible 2.5.0
  config file = None
  configured module search path = [u'/Users/alancoding/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /Users/alancoding/.virtualenvs/cli/lib/python2.7/site-packages/ansible
  executable location = /Users/alancoding/.virtualenvs/cli/bin/ansible
  python version = 2.7.11 (default, Oct 17 2016, 14:59:40) [GCC 4.2.1 Compatible Apple LLVM 7.0.2 (clang-700.1.81)]
```


##### ACTUAL RESULTS
See the section on Ansible version.

It seems that line breaks are missing in:

> ansible 2.9.0.dev0 config file = None configured module search path =
['/Users/alancoding/.ansible/plugins/modules',
'/usr/share/ansible/plugins/modules'] ansible python module location =
/Users/alancoding/Documents/repos/ansible/lib/ansible executable location =
/Users/alancoding/.virtualenvs/ansible3/bin/ansible python version = 3.6.5
(default, Apr 25 2018, 14:23:58) [GCC 4.2.1 Compatible Apple LLVM 9.1.0
(clang-902.0.39.1)]

Hints from issue discussion:
Files identified in the description:
* [lib/ansible/cli/adhoc.py](https://github.com/ansible/ansible/blob/devel/lib/ansible/cli/adhoc.py)

If these files are inaccurate, please update the `component name` section of the description or use the `!component` bot command.

[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: components_banner --->
Introduced in db6cc603528c5d1c15acf89e092f99f5d24ca848.

cc @sivel
marking tower_blocker but sivel and sdoran have diagnosed that this is part of the switch from optparse to argparse.  So it does not affect stable-2.8.  It is devel/2.9+ only.  (Will not affect the release of 2.8.0)

Commit message: Add integration test for checking ansible --version

Patch to review:
diff --git a/lib/ansible/cli/arguments/option_helpers.py b/lib/ansible/cli/arguments/option_helpers.py
index 7ea478c5b0005a..595e3615284d6e 100644
--- a/lib/ansible/cli/arguments/option_helpers.py
+++ b/lib/ansible/cli/arguments/option_helpers.py
@@ -29,6 +29,13 @@ def add_arguments(self, actions):
         super(SortingHelpFormatter, self).add_arguments(actions)
 
 
+class AnsibleVersion(argparse.Action):
+    def __call__(self, parser, namespace, values, option_string=None):
+        ansible_version = to_native(version(getattr(parser, 'prog')))
+        print(ansible_version)
+        parser.exit()
+
+
 class PrependListAction(argparse.Action):
     """A near clone of ``argparse._AppendAction``, but designed to prepend list values
     instead of appending.
@@ -184,7 +191,8 @@ def create_base_parser(usage="", desc=None, epilog=None):
     )
     version_help = "show program's version number, config file location, configured module search path," \
                    " module location, executable location and exit"
-    parser.add_argument('--version', action='version', version=to_native(version("%(prog)s")), help=version_help)
+
+    parser.add_argument('--version', action=AnsibleVersion, nargs=0, help=version_help)
     add_verbosity_options(parser)
     return parser
 
diff --git a/test/integration/targets/ansible/runme.sh b/test/integration/targets/ansible/runme.sh
index b3ddb65fd7cec4..60344da724c993 100755
--- a/test/integration/targets/ansible/runme.sh
+++ b/test/integration/targets/ansible/runme.sh
@@ -5,6 +5,8 @@ set -eux -o pipefail
 ansible --version
 ansible --help
 
+python test_version.py
+
 ansible testhost -i ../../inventory -m ping  "$@"
 ansible testhost -i ../../inventory -m setup "$@"
 
diff --git a/test/integration/targets/ansible/test_version.py b/test/integration/targets/ansible/test_version.py
new file mode 100644
index 00000000000000..145fd0557a7f81
--- /dev/null
+++ b/test/integration/targets/ansible/test_version.py
@@ -0,0 +1,17 @@
+#!/usr/bin/env python
+
+import re
+import subprocess
+
+
+p = subprocess.Popen(['ansible', '--version'], stdout=subprocess.PIPE)
+version_string = p.communicate()[0]
+version_lines = version_string.splitlines()
+
+assert len(version_lines) == 6, 'Incorrect number of lines in "ansible --version" output'
+assert re.match(b'ansible [0-9.a-z]+$', version_lines[0]), 'Incorrect ansible version line in "ansible --version" output'
+assert re.match(b'  config file = .*$', version_lines[1]), 'Incorrect config file line in "ansible --version" output'
+assert re.match(b'  configured module search path = .*$', version_lines[2]), 'Incorrect module search path in "ansible --version" output'
+assert re.match(b'  ansible python module location = .*$', version_lines[3]), 'Incorrect python module location in "ansible --version" output'
+assert re.match(b'  executable location = .*$', version_lines[4]), 'Incorrect executable locaction in "ansible --version" output'
+assert re.match(b'  python version = .*$', version_lines[5]), 'Incorrect python version in "ansible --version" output'


Code review comments:
diff_hunk: @@ -0,0 +1,17 @@
+#!/usr/bin/env python
path: test/integration/targets/ansible/test_version.py
text: @user1:
Instead of using a script that subprocesses ansible, you can add a test to `test/units/cli/test_adhoc.py` that utilizes the `capsys` pytest fixture to get the output: https://docs.pytest.org/en/latest/capture.html

@author:
Nice! I will check that out.

@author:
I have this mostly working. The only issue I have when running using `AdHocCLI` is the `%(prog)s` is not `ansible` but a `pytest` string:

```
test/units/cli/test_adhoc.py pytest 2.9.0.dev0
  config file = /Users/sdoran/.ansible.cfg
  configured module search path = ['/Users/sdoran/Source/ansible/library']
  ansible python module location = /Users/sdoran/Source/ansible/lib/ansible
  executable location = /Users/sdoran/.pyenv/versions/3.7.3/bin/pytest
  python version = 3.7.3 (default, Apr 18 2019, 13:51:13) [Clang 10.0.1 (clang-1001.0.46.3)]
```

Is there any way to fix that? Because I think testing the content of that first line is the most critical part.

@user1:
There are likely a few ways to address that.  

Looking at how you are calling the `version` function:

```
ansible_version = to_native(version(getattr(parser, 'prog')))
```

That is using `prog` from `parser`.  `parser` is available at `adhoc_cli.parser` so you can likely just monkeypatch that.

Otherwise, maybe monkeypatch `sys.argv[0]`.

@author:
I wasn't having any luck patching the `parser` object, but patching `sys.argv` did work.