On Thu, Oct 31, 2013 at 03:40:06PM +0100, Petr Pudlak wrote: > * Use different visual marks at the beginning of lines (such as ">>>>" for > starting a test and "<<<<" for finishing it) > > * Show more specifically if a test fails/passes. > > * If stdout is a terminal, use colors to distinguish different messages > > Signed-off-by: Petr Pudlak <[email protected]> > --- > qa/colors.py | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++ > qa/ganeti-qa.py | 20 +++++++++++++++----- > qa/qa_utils.py | 4 +++- > 3 files changed, 69 insertions(+), 6 deletions(-) > create mode 100644 qa/colors.py > > diff --git a/qa/colors.py b/qa/colors.py > new file mode 100644 > index 0000000..f4a063b > --- /dev/null > +++ b/qa/colors.py > @@ -0,0 +1,51 @@ > +#!/usr/bin/python -u > +# > + > +# Copyright (C) 2007, 2008, 2009, 2010, 2011, 2012, 2013 Google Inc.
Only 2013 here. > +# > +# This program is free software; you can redistribute it and/or modify > +# it under the terms of the GNU General Public License as published by > +# the Free Software Foundation; either version 2 of the License, or > +# (at your option) any later version. > +# > +# This program is distributed in the hope that it will be useful, but > +# WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > +# General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with this program; if not, write to the Free Software > +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA > +# 02110-1301, USA. > + > + > +"""Script for adding colorized output to Ganeti. > + > +Colors are enabled only if the standard output is a proper terminal. > + > +""" > + > +import os > +import sys > + > +# It would really be nice if we aligned those constants, but PEP8 complains > +# with E222. Should we disable the warning or keep the constants as they are? This message is not necessary: we never align these things in Pyhton because of PEP8. > +DEFAULT = '\033[0m' > +RED = '\033[91m' > +GREEN = '\033[92m' > +BLUE = '\033[94m' > +CYAN = '\033[96m' > +WHITE = '\033[97m' > +YELLOW = '\033[93m' > +MAGENTA = '\033[95m' > +GREY = '\033[90m' > +BLACK = '\033[90m' > + > +_enabled = sys.stdout.isatty() > + > + > +def colorize(line, color=None): > + if _enabled and color is not None: > + return color + line + DEFAULT > + else: > + return line > diff --git a/qa/ganeti-qa.py b/qa/ganeti-qa.py > index 221fc12..a1fa1d0 100755 > --- a/qa/ganeti-qa.py > +++ b/qa/ganeti-qa.py > @@ -31,6 +31,8 @@ import datetime > import optparse > import sys > > +import colors > + > import qa_cluster > import qa_config > import qa_daemon We usually separate group imports like so: * standard library imports * third-party imports * local imports This is true for Haskell https://code.google.com/p/ganeti/wiki/HaskellStyleGuide and even though it is not written in the Python style guide, it is the usual practise. That means you can delete the newline after 'import colors' > @@ -57,13 +59,14 @@ import ganeti.rapi.client # pylint: disable=W0611 > from ganeti.rapi.client import UsesRapiClient > > > -def _FormatHeader(line, end=72): > +def _FormatHeader(line, end=72, mark="-", color=None): > """Fill a line up to the end column. > > """ > - line = "---- " + line + " " > + line = (mark * 4) + " " + line + " " > line += "-" * (end - len(line)) > line = line.rstrip() > + line = colors.colorize(line, color) > return line > > > @@ -89,15 +92,21 @@ def RunTest(fn, *args, **kwargs): > desc = _DescriptionOf(fn) > > print > - print _FormatHeader("%s start %s" % (tstart, desc)) > + print _FormatHeader("%s start %s" % (tstart, desc), > + color=colors.YELLOW, mark=">") Perhaps we can use the 'git diff' syntax given that we are already familiar with it, that means the start and end signs should be flipped. > > try: > retval = fn(*args, **kwargs) > + print _FormatHeader("PASSED %s" % (desc, ), color=colors.GREEN) > return retval > + except Exception, e: > + print _FormatHeader("FAILED %s: %s" % (desc, e), color=colors.RED) > + raise There's a chance people with think this is standard error if they don't read 'FAILED', given that buildbot already colorizes lines from standard error with red. On the other hand, red is for sure the color for error messages. So I would probably leave this as it is. What about the buildbot? Is there a way to make this work there? Thanks, Jose > finally: > tstop = datetime.datetime.now() > tdelta = tstop - tstart > - print _FormatHeader("%s time=%s %s" % (tstop, tdelta, desc)) > + print _FormatHeader("%s time=%s %s" % (tstop, tdelta, desc), > + color=colors.MAGENTA, mark="<") > > > def RunTestIf(testnames, fn, *args, **kwargs): > @@ -114,7 +123,8 @@ def RunTestIf(testnames, fn, *args, **kwargs): > desc = _DescriptionOf(fn) > # TODO: Formatting test names when non-string names are involved > print _FormatHeader("%s skipping %s, test(s) %s disabled" % > - (tstart, desc, testnames)) > + (tstart, desc, testnames), > + color=colors.BLUE, mark="*") > > > def RunEnvTests(): > diff --git a/qa/qa_utils.py b/qa/qa_utils.py > index 3bbd85e..d5bc3de 100644 > --- a/qa/qa_utils.py > +++ b/qa/qa_utils.py > @@ -45,6 +45,7 @@ from ganeti import ht > from ganeti import pathutils > from ganeti import vcluster > > +import colors > import qa_config > import qa_error > > @@ -285,7 +286,8 @@ def StartLocalCommand(cmd, _nolog_opts=False, > log_cmd=True, **kwargs): > pcmd = [i for i in cmd if not i.startswith("-")] > else: > pcmd = cmd > - print "Command: %s" % utils.ShellQuoteArgs(pcmd) > + print "%s %s" % (colors.colorize("Command:", colors.CYAN), > + utils.ShellQuoteArgs(pcmd)) > return subprocess.Popen(cmd, shell=False, **kwargs) > > > -- > 1.8.4.1 > -- Jose Antonio Lopes Ganeti Engineering Google Germany GmbH Dienerstr. 12, 80331, München Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Geschäftsführer: Graham Law, Christine Elizabeth Flores Steuernummer: 48/725/00206 Umsatzsteueridentifikationsnummer: DE813741370
