From 24c713ccc8402828b5e03fc87ffacbfeda618c76 Mon Sep 17 00:00:00 2001 From: Alfredo Deza Date: Mon, 9 Sep 2019 13:40:06 -0400 Subject: [PATCH 1/7] ceph-volume create a new tox.ini for shell-based tests Signed-off-by: Alfredo Deza (cherry picked from commit 4612ab3ae62a3e8e1cc9e51489feab0c938296d3) (cherry picked from commit 7c636503bbe4cf0f58b85c1c2d5b2fb719cf430e) --- src/ceph-volume/shell_tox.ini | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 src/ceph-volume/shell_tox.ini diff --git a/src/ceph-volume/shell_tox.ini b/src/ceph-volume/shell_tox.ini new file mode 100644 index 00000000000000..5cd4606e4c5d7c --- /dev/null +++ b/src/ceph-volume/shell_tox.ini @@ -0,0 +1,11 @@ +[tox] +envlist = py27, py35, py36 +skip_missing_interpreters = true + +[testenv] +passenv=* +whitelist_externals= + bash + grep + mktemp +commands=bash {posargs:ceph_volume/tests/functional/scripts/test_unicode.sh} {posargs:ceph_volume/tests/functional/scripts/output.py} From a21d95dba8370fc06bdb8f2da2d30a9a4c438675 Mon Sep 17 00:00:00 2001 From: Alfredo Deza Date: Mon, 9 Sep 2019 13:44:17 -0400 Subject: [PATCH 2/7] ceph-volume create a logger for the terminal Signed-off-by: Alfredo Deza (cherry picked from commit 95b16b516c0736839fe50f12ecbeb7cd62c67335) (cherry picked from commit bdf6a8b57e7cefd9a136d58ab02a9847bd8ffb1b) --- src/ceph-volume/ceph_volume/log.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/ceph-volume/ceph_volume/log.py b/src/ceph-volume/ceph_volume/log.py index 890b6da1b7a5c0..802b5fec290dc2 100644 --- a/src/ceph-volume/ceph_volume/log.py +++ b/src/ceph-volume/ceph_volume/log.py @@ -31,3 +31,18 @@ def setup(name='ceph-volume.log', log_path=None): fh.setFormatter(logging.Formatter(FILE_FORMAT)) root_logger.addHandler(fh) + + +def setup_console(): + # TODO: At some point ceph-volume should stop using the custom logger + # interface that exists in terminal.py and use the logging module to + # produce output for the terminal + # Console Logger + sh = logging.StreamHandler() + sh.setFormatter(logging.Formatter('[terminal] %(message)s')) + sh.setLevel(logging.DEBUG) + + terminal_logger = logging.getLogger('terminal') + + # allow all levels at root_logger, handlers control individual levels + terminal_logger.addHandler(sh) From a0f0f1710c8a62ebde246c58ce1acc35aef6bbde Mon Sep 17 00:00:00 2001 From: Alfredo Deza Date: Mon, 9 Sep 2019 13:44:36 -0400 Subject: [PATCH 3/7] ceph-volume: instantiate the new terminal logger in main() Signed-off-by: Alfredo Deza (cherry picked from commit bfb8422e262dcce49c50610a3977bfe1edc2a435) (cherry picked from commit 26b97ecbc4c7f9b0d4e01b8d8c79b0b6d7457d80) --- src/ceph-volume/ceph_volume/main.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/ceph-volume/ceph_volume/main.py b/src/ceph-volume/ceph_volume/main.py index 4685cb41c39515..f396daf0025460 100644 --- a/src/ceph-volume/ceph_volume/main.py +++ b/src/ceph-volume/ceph_volume/main.py @@ -131,6 +131,7 @@ def main(self, argv): if os.path.isdir(conf.log_path): conf.log_path = os.path.join(args.log_path, 'ceph-volume.log') log.setup() + log.setup_console() logger = logging.getLogger(__name__) logger.info("Running command: ceph-volume %s %s", " ".join(main_args), " ".join(subcommand_args)) # set all variables from args and load everything needed according to From e75be2701355fcd1056620abfe66f6af060094ed Mon Sep 17 00:00:00 2001 From: Alfredo Deza Date: Mon, 9 Sep 2019 13:45:08 -0400 Subject: [PATCH 4/7] ceph-volume terminal remove unicode stream handler This caused problems in environments where stderr was redirected, since stderr sets the encoding to None. Getting it back again allows everything to work correctly, and keeps all the current unit tests passing Signed-off-by: Alfredo Deza (cherry picked from commit fe66e01f6a4b19bb15695537713a629a951b28ce) (cherry picked from commit d1615c0da2fd785c34719175888170164105ef63) --- src/ceph-volume/ceph_volume/terminal.py | 43 +++++++++---------------- 1 file changed, 16 insertions(+), 27 deletions(-) diff --git a/src/ceph-volume/ceph_volume/terminal.py b/src/ceph-volume/ceph_volume/terminal.py index aaff47962d85c3..a34946f9294470 100644 --- a/src/ceph-volume/ceph_volume/terminal.py +++ b/src/ceph-volume/ceph_volume/terminal.py @@ -1,8 +1,10 @@ -import codecs import logging import sys +terminal_logger = logging.getLogger('terminal') + + class colorize(str): """ Pretty simple to use:: @@ -80,34 +82,13 @@ def make(cls, string): class _Write(object): def __init__(self, _writer=None, prefix='', suffix='', flush=False): - # we can't set sys.stderr as the default for _writer. Otherwise + # we can't set sys.stderr as the default for _writer. otherwise # pytest's capturing gets confused - if not _writer: - _writer = sys.stderr - self._writer = _Write._unicode_output_stream(_writer) - sys.stderr = self._writer + self._writer = _writer or sys.stderr self.suffix = suffix self.prefix = prefix self.flush = flush - @staticmethod - def _unicode_output_stream(stream): - # wrapper for given stream, so it can write unicode without throwing - # exception - # sys.stderr.encoding is None if !isatty - encoding = stream.encoding or '' - if encoding.upper() in ('UTF-8', 'UTF8'): - # already using unicode encoding, nothing to do - return stream - encoding = encoding or 'UTF-8' - if sys.version_info >= (3, 0): - # try to use whatever writer class the stream was - return stream.__class__(stream.buffer, encoding, 'replace', - stream.newlines, stream.line_buffering) - else: - # in python2, stderr is but a "file" - return codecs.getwriter(encoding)(stream, 'replace') - def bold(self, string): self.write(bold(string)) @@ -117,9 +98,17 @@ def raw(self, string): self.write(string) def write(self, line): - self._writer.write(self.prefix + line + self.suffix) - if self.flush: - self._writer.flush() + entry = self.prefix + line + self.suffix + + try: + self._writer.write(entry) + if self.flush: + self._writer.flush() + except (UnicodeDecodeError, UnicodeEncodeError): + try: + terminal_logger.info(entry.strip('\n')) + except (AttributeError, TypeError): + terminal_logger.info(entry) def stdout(msg): From a571b6866a83028d9155fea5020aa6e0048c40c5 Mon Sep 17 00:00:00 2001 From: Alfredo Deza Date: Mon, 9 Sep 2019 15:20:13 -0400 Subject: [PATCH 5/7] ceph-volume tests verify new logging fallback and encodings in terminal Signed-off-by: Alfredo Deza (cherry picked from commit ee18ebc078b2d380b88f3726737184f34345e2f3) (cherry picked from commit cd82ab70190b9744dfbd1e969538260401d8f97b) --- .../ceph_volume/tests/test_terminal.py | 50 +++++++++++++------ 1 file changed, 35 insertions(+), 15 deletions(-) diff --git a/src/ceph-volume/ceph_volume/tests/test_terminal.py b/src/ceph-volume/ceph_volume/tests/test_terminal.py index 2570a47a247606..fdf21907057904 100644 --- a/src/ceph-volume/ceph_volume/tests/test_terminal.py +++ b/src/ceph-volume/ceph_volume/tests/test_terminal.py @@ -2,9 +2,14 @@ import codecs import io +try: + from io import StringIO +except ImportError: + from StringIO import StringIO import pytest import sys from ceph_volume import terminal +from ceph_volume.log import setup_console class SubCommand(object): @@ -97,27 +102,42 @@ class TestWriteUnicode(object): def setup(self): self.octpus_and_squid_en = u'octpus and squid' - octpus_and_squid_zh = u'章鱼和鱿鱼' - self.message = self.octpus_and_squid_en + octpus_and_squid_zh + self.octpus_and_squid_zh = u'章鱼和鱿鱼' + self.message = self.octpus_and_squid_en + self.octpus_and_squid_zh + setup_console() def test_stdout_writer(self, capsys): # should work with whatever stdout is terminal.stdout(self.message) _, err = capsys.readouterr() assert self.octpus_and_squid_en in err + assert self.octpus_and_squid_zh in err @pytest.mark.parametrize('encoding', ['ascii', 'utf8']) - def test_writer(self, encoding, stream, monkeypatch, capsys): - if encoding == 'ascii' and sys.version_info > (3,): + def test_writer_log(self, stream, encoding, monkeypatch, caplog): + writer = StringIO() + terminal._Write(_writer=writer).raw(self.message) + writer.flush() + writer.seek(0) + output = writer.readlines()[0] + assert self.octpus_and_squid_en in output + + @pytest.mark.parametrize('encoding', ['utf8']) + def test_writer(self, encoding, stream, monkeypatch, capsys, caplog): + buffer = io.BytesIO() + writer = stream(buffer, encoding) + terminal._Write(_writer=writer).raw(self.message) + writer.flush() + writer.seek(0) + val = buffer.getvalue() + assert self.octpus_and_squid_en.encode(encoding) in val + + def test_writer_uses_log_on_unicodeerror(self, stream, monkeypatch, capture): + + if sys.version_info > (3,): pytest.skip("Something breaks inside of pytest's capsys") - # should keep writer alive - with capsys.disabled(): - buffer = io.BytesIO() - # we want to have access to the sys.stdout's attributes in - # make_stream(), not the ones of pytest.capture.EncodedFile - writer = stream(buffer, encoding) - monkeypatch.setattr(sys, 'stderr', writer) - terminal.stdout(self.message) - writer.flush() - val = buffer.getvalue() - assert self.octpus_and_squid_en.encode(encoding) in val + monkeypatch.setattr(terminal.terminal_logger, 'info', capture) + buffer = io.BytesIO() + writer = stream(buffer, 'ascii') + terminal._Write(_writer=writer).raw(self.message) + assert self.octpus_and_squid_en in capture.calls[0]['args'][0] From e5cf3d0886adbd66e776a38c328542ad5d282bc5 Mon Sep 17 00:00:00 2001 From: Alfredo Deza Date: Mon, 9 Sep 2019 18:54:51 -0400 Subject: [PATCH 6/7] ceph-volume tests create a shell test for functional unicode Signed-off-by: Alfredo Deza (cherry picked from commit 595e492783283359b613d199a75f3e7495bfb9ed) (cherry picked from commit 1c2894dc7ca7366cea57410e3bda34ed1e56ca06) --- .../tests/functional/scripts/test_unicode.sh | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 src/ceph-volume/ceph_volume/tests/functional/scripts/test_unicode.sh diff --git a/src/ceph-volume/ceph_volume/tests/functional/scripts/test_unicode.sh b/src/ceph-volume/ceph_volume/tests/functional/scripts/test_unicode.sh new file mode 100644 index 00000000000000..e4ba4f0a675164 --- /dev/null +++ b/src/ceph-volume/ceph_volume/tests/functional/scripts/test_unicode.sh @@ -0,0 +1,35 @@ +#!/bin/bash + +# Not entirely sure why these executables don't seem to be available in the +# $PATH when running from tox. Calling out to `which` seems to fix it, at the +# expense of making the script a bit obtuse + +mktemp=$(which mktemp) +cat=$(which cat) +grep=$(which grep) +PYTHON_EXECUTABLE=`which python` +STDERR_FILE=$($mktemp) +INVALID="→" + +echo "stderr file created: $STDERR_FILE" + +INVALID="$INVALID" $PYTHON_EXECUTABLE $1 2> ${STDERR_FILE} + +retVal=$? + +if [ $retVal -ne 0 ]; then + echo "Failed test: Unexpected failure from running Python script" + echo "Below is output of stderr captured:" + $cat "${STDERR_FILE}" + exit $retVal +fi + +$grep --quiet "$INVALID" ${STDERR_FILE} + +retVal=$? +if [ $retVal -ne 0 ]; then + echo "Failed test: expected to find \"${INVALID}\" character in tmpfile: \"${STDERR_FILE}\"" + echo "Below is output of stderr captured:" + $cat "${STDERR_FILE}" +fi +exit $retVal From 0fb803ebc3e99411f272abb9c83fa6f53cad539e Mon Sep 17 00:00:00 2001 From: Alfredo Deza Date: Mon, 9 Sep 2019 18:55:13 -0400 Subject: [PATCH 7/7] ceph-volume tests create a test file for checking unicode output Signed-off-by: Alfredo Deza (cherry picked from commit 79196354aca3b3d30c7de1e2ac8b5a3c06ee8330) (cherry picked from commit 4183d29c22584e6bf1c052aacdc31177ff9380d6) --- .../ceph_volume/tests/functional/scripts/output.py | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 src/ceph-volume/ceph_volume/tests/functional/scripts/output.py diff --git a/src/ceph-volume/ceph_volume/tests/functional/scripts/output.py b/src/ceph-volume/ceph_volume/tests/functional/scripts/output.py new file mode 100644 index 00000000000000..1607194440a3aa --- /dev/null +++ b/src/ceph-volume/ceph_volume/tests/functional/scripts/output.py @@ -0,0 +1,5 @@ +import os +from ceph_volume import terminal + +char = os.environ.get('INVALID') +terminal.stdout(char)