sekikn commented on a change in pull request #6401: [AIRFLOW-5732] Add unit
test for the version command
URL: https://github.com/apache/airflow/pull/6401#discussion_r339555551
##########
File path: tests/core.py
##########
@@ -1795,6 +1796,12 @@ def test_variables(self):
os.remove('variables2.json')
os.remove('variables3.json')
+ def test_cli_version(self):
Review comment:
Thanks for the comment @mik-laj!
> I'm afraid this test is in the wrong place. tests/cli/test_cli.py is a
better file.
The reason that I added the new test to test/core.py was it has many
cli-related tests in its TestCli class. I have no objection to move the new
test into tests/cli/test_cli.py, but is there any criteria to put a new
cli-related test into it (or should we basically use tests/cli/test_cli.py in
the future)?
> I am also afraid of sys.stdout mocking. Python has a special mechanism for
this.
I didn't use `contextlib.redirect_stdout` because of concern for the
compatibility with Python2, but I've just noticed [we're going to drop it on
the master
branch](https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-3+Drop+support+for+Python+2).
I'll address it on another PR, thanks :)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
