Patchwork run-tests: check if stream is a tty before using color

login
register
mail settings
Submitter Matthieu Laneuville
Date July 17, 2017, 10:33 p.m.
Message ID <7f5f97697162331b5acf.1500330793@carbon>
Download mbox | patch
Permalink /patch/22458/
State Accepted
Headers show

Comments

Matthieu Laneuville - July 17, 2017, 10:33 p.m.
# HG changeset patch
# User Matthieu Laneuville <matthieu.laneuville@octobus.net>
# Date 1500329966 -32400
#      Tue Jul 18 07:19:26 2017 +0900
# Node ID 7f5f97697162331b5acf78aef091ae3fd341e308
# Parent  9b2647a7a70a270e85457ea55583b6eba0551258
run-tests: check if stream is a tty before using color

Previous implementation (e80041832eec) checked only if sys.stderr was a tty
which was less general. Also makes sure that colors is never used if pygments is
not available, irrespective of --color flag value.
Yuya Nishihara - July 18, 2017, 1:13 p.m.
On Tue, 18 Jul 2017 07:33:13 +0900, mlaneuville@gmail.com wrote:
> # HG changeset patch
> # User Matthieu Laneuville <matthieu.laneuville@octobus.net>
> # Date 1500329966 -32400
> #      Tue Jul 18 07:19:26 2017 +0900
> # Node ID 7f5f97697162331b5acf78aef091ae3fd341e308
> # Parent  9b2647a7a70a270e85457ea55583b6eba0551258
> run-tests: check if stream is a tty before using color

Queued, thanks.

> @@ -1574,6 +1565,17 @@
>          self.successes = []
>          self.faildata = {}
>  
> +        global with_color
> +        if not self.stream.isatty(): # check if the terminal is capable
> +            with_color = False
> +
> +        if options.color != 'auto':
> +            if options.color == 'never':
> +                with_color = False
> +            else: # 'always', for testing purposes
> +                if pygmentspresent:
> +                    with_color = True

Perhaps this no longer needs to be a global variable.
via Mercurial-devel - July 18, 2017, 2:13 p.m.
Also see my phabricator series D114-D118. Looks like this series was sent
ahead of mine and I missed it :-(

On Jul 18, 2017 6:19 AM, "Yuya Nishihara" <yuya@tcha.org> wrote:

> On Tue, 18 Jul 2017 07:33:13 +0900, mlaneuville@gmail.com wrote:
> > # HG changeset patch
> > # User Matthieu Laneuville <matthieu.laneuville@octobus.net>
> > # Date 1500329966 -32400
> > #      Tue Jul 18 07:19:26 2017 +0900
> > # Node ID 7f5f97697162331b5acf78aef091ae3fd341e308
> > # Parent  9b2647a7a70a270e85457ea55583b6eba0551258
> > run-tests: check if stream is a tty before using color
>
> Queued, thanks.
>
> > @@ -1574,6 +1565,17 @@
> >          self.successes = []
> >          self.faildata = {}
> >
> > +        global with_color
> > +        if not self.stream.isatty(): # check if the terminal is capable
> > +            with_color = False
> > +
> > +        if options.color != 'auto':
> > +            if options.color == 'never':
> > +                with_color = False
> > +            else: # 'always', for testing purposes
> > +                if pygmentspresent:
> > +                    with_color = True
>
> Perhaps this no longer needs to be a global variable.
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
Yuya Nishihara - July 18, 2017, 2:51 p.m.
On Tue, 18 Jul 2017 07:13:00 -0700, Martin von Zweigbergk wrote:
> Also see my phabricator series D114-D118. Looks like this series was sent
> ahead of mine and I missed it :-(

Yeah, they look nice, but these two series conflict each other. How should
we proceed?
via Mercurial-devel - July 18, 2017, 2:58 p.m.
On Jul 18, 2017 7:51 AM, "Yuya Nishihara" <yuya@tcha.org> wrote:

On Tue, 18 Jul 2017 07:13:00 -0700, Martin von Zweigbergk wrote:
> Also see my phabricator series D114-D118. Looks like this series was sent
> ahead of mine and I missed it :-(

Yeah, they look nice, but these two series conflict each other. How should
we proceed?


Seems like I should rebase my series on top of this.
via Mercurial-devel - July 18, 2017, 4:14 p.m.
On Tue, Jul 18, 2017 at 7:58 AM, Martin von Zweigbergk
<martinvonz@google.com> wrote:
>
>
> On Jul 18, 2017 7:51 AM, "Yuya Nishihara" <yuya@tcha.org> wrote:
>
> On Tue, 18 Jul 2017 07:13:00 -0700, Martin von Zweigbergk wrote:
>> Also see my phabricator series D114-D118. Looks like this series was sent
>> ahead of mine and I missed it :-(
>
> Yeah, they look nice, but these two series conflict each other. How should
> we proceed?
>
>
> Seems like I should rebase my series on top of this.

Done. D114, D116, D117, D118, D136 are now ready (D115 was no longer needed).
via Mercurial-devel - July 18, 2017, 4:30 p.m.
On Tue, Jul 18, 2017 at 9:14 AM, Martin von Zweigbergk
<martinvonz@google.com> wrote:
> On Tue, Jul 18, 2017 at 7:58 AM, Martin von Zweigbergk
> <martinvonz@google.com> wrote:
>>
>>
>> On Jul 18, 2017 7:51 AM, "Yuya Nishihara" <yuya@tcha.org> wrote:
>>
>> On Tue, 18 Jul 2017 07:13:00 -0700, Martin von Zweigbergk wrote:
>>> Also see my phabricator series D114-D118. Looks like this series was sent
>>> ahead of mine and I missed it :-(
>>
>> Yeah, they look nice, but these two series conflict each other. How should
>> we proceed?
>>
>>
>> Seems like I should rebase my series on top of this.
>
> Done. D114, D116, D117, D118, D136 are now ready (D115 was no longer needed).

Sigh, D114 is broken (test-run-tests.t fails). I'll look into it. Sorry.
via Mercurial-devel - July 18, 2017, 5 p.m.
On Tue, Jul 18, 2017 at 9:30 AM, Martin von Zweigbergk
<martinvonz@google.com> wrote:
> On Tue, Jul 18, 2017 at 9:14 AM, Martin von Zweigbergk
> <martinvonz@google.com> wrote:
>> On Tue, Jul 18, 2017 at 7:58 AM, Martin von Zweigbergk
>> <martinvonz@google.com> wrote:
>>>
>>>
>>> On Jul 18, 2017 7:51 AM, "Yuya Nishihara" <yuya@tcha.org> wrote:
>>>
>>> On Tue, 18 Jul 2017 07:13:00 -0700, Martin von Zweigbergk wrote:
>>>> Also see my phabricator series D114-D118. Looks like this series was sent
>>>> ahead of mine and I missed it :-(
>>>
>>> Yeah, they look nice, but these two series conflict each other. How should
>>> we proceed?
>>>
>>>
>>> Seems like I should rebase my series on top of this.
>>
>> Done. D114, D116, D117, D118, D136 are now ready (D115 was no longer needed).
>
> Sigh, D114 is broken (test-run-tests.t fails). I'll look into it. Sorry.

Okay, that should now be done. I dropped D136 again.

Patch

diff -r 9b2647a7a70a -r 7f5f97697162 tests/run-tests.py
--- a/tests/run-tests.py	Mon Jul 17 11:45:38 2017 -0700
+++ b/tests/run-tests.py	Tue Jul 18 07:19:26 2017 +0900
@@ -101,8 +101,6 @@ 
     except ImportError:
         pass
 
-if not sys.stderr.isatty(): # check if the terminal is capable
-    with_color = False
 
 if sys.version_info > (3, 5, 0):
     PYTHON3 = True
@@ -416,13 +414,6 @@ 
         parser.error('--chg does not work when --with-hg is specified '
                      '(use --with-chg instead)')
 
-    global with_color
-    if options.color != 'auto':
-        if options.color == 'never':
-            with_color = False
-        else: # 'always', for testing purposes
-            with_color = True
-
     global useipv6
     if options.ipv6:
         useipv6 = checksocketfamily('AF_INET6')
@@ -1574,6 +1565,17 @@ 
         self.successes = []
         self.faildata = {}
 
+        global with_color
+        if not self.stream.isatty(): # check if the terminal is capable
+            with_color = False
+
+        if options.color != 'auto':
+            if options.color == 'never':
+                with_color = False
+            else: # 'always', for testing purposes
+                if pygmentspresent:
+                    with_color = True
+
     def addFailure(self, test, reason):
         self.failures.append((test, reason))