GeezerVison

I think we should also change the British spelling 'colour' to 'color' in the PR's title, which will become the commit message after merging.

What about also colorising the divider line that's used to separate failed examples? So it would look like this? It might help to see more clearly where one failed example ends and another begins.

This patch does it:

diff --git a/Lib/doctest.py b/Lib/doctest.py
index 00ff1a107c..80d0b70592 100644
--- a/Lib/doctest.py
+++ b/Lib/doctest.py
@@ -1307,7 +1307,10 @@ def report_unexpected_exception(self, out, test, example, exc_info):
             'Exception raised:\n' + _indent(_exception_traceback(exc_info)))
 
     def _failure_header(self, test, example):
-        out = [self.DIVIDER]
+        red, reset = (
+            (ANSIColors.RED, ANSIColors.RESET) if can_colorize() else ("", "")
+        )
+        out = [f"{red}{self.DIVIDER}{reset}"]
         if test.filename:
             if test.lineno is not None and example.lineno is not None:
                 lineno = test.lineno + example.lineno + 1
@@ -1621,7 +1624,7 @@ def summarize(self, verbose=None):
                     print(f" {green}{count:3d} test{s} in {name}{reset}")
 
         if failed:
-            print(self.DIVIDER)
+            print(f"{red}{self.DIVIDER}{reset}")
             print(f"{red}{_n_items(failed)} had failures:{reset}")

Of these three summary lines at the end:

1 item had failures:
   2 of   4 in typing._should_unflatten_callable_args
***Test Failed*** 2 failures.

You currently have all three colored in red if there are errors. I think I might be inclined to keep the last line colored in red (as you have it), but not colorize the first two of those lines. I feel like having everything in red makes it harder to read, and having the last line in red is enough to draw your attention to it and communicate "bad!". What do you think?

I've added red divider lines.

Edit: need to update tests to disable colour locally.

Of these three summary lines at the end:

1 item had failures:
   2 of   4 in typing._should_unflatten_callable_args
***Test Failed*** 2 failures.

You currently have all three colored in red if there are errors. I think I might be inclined to keep the last line colored in red (as you have it), but not colorize the first two of those lines. I feel like having everything in red makes it harder to read, and having the last line in red is enough to draw your attention to it and communicate "bad!". What do you think?

Can you share a screenshot and a diff to trigger it?

I don't get all in red:

Screenshots

Can you share a screenshot and a diff to trigger it?

Screenshot in #117583 (comment). For how to trigger it — I was testing by doing some silly things to some doctests in Lib/typing.py, then running ./python.exe -m doctest Lib/typing.py directly!

It looks like many tests fail if you run tests with FORCE_COLOR, e.g. FORCE_COLOR=1 ./python.exe -m test (see #117605), and test_doctest is one of them. This patch does seem to make a few more parts of test_doctest fail with FORCE_COLOR, though:

Since there are so many that already fail with that environment variable set, I'm not sure it's really an issue, but thought I'd mention it anyway.

I was pleased to see that colorize now follows the ISP. I love the idea of colorization, and I think that we can continue this trend with unittests as well. Thank you for your contribution!

The minor improvement that could be done is to use stub/other way to return empty colors so instead of this:

        if can_colorize():
            bold_green = ANSIColors.BOLD_GREEN
            bold_red = ANSIColors.BOLD_RED
            green = ANSIColors.GREEN
            red = ANSIColors.RED
            reset = ANSIColors.RESET
            yellow = ANSIColors.YELLOW
        else:
            bold_green = ""
            bold_red = ""
            green = ""
            red = ""
            reset = ""
            yellow = ""

it could be something like this:

        ANSIColors = get_ansi_colors()
        bold_green = ANSIColors.BOLD_GREEN
        bold_red = ANSIColors.BOLD_RED
        green = ANSIColors.GREEN
        red = ANSIColors.RED
        reset = ANSIColors.RESET
        yellow = ANSIColors.YELLOW

Thanks for the suggestion, something like that sounds good.

I'll wait to give #117672 a change to be merged first so as not to cause conflicts there, then will update this. I might include the refactoring as part of this PR, but mainly want to make sure this is merged before the beta cutoff on 2024-05-07 so it can be in 3.13.

I'll also add some docs, but they can be another PR and can be after the beta cutoff, if necessary.

Plan C: it's been a week and #117672 isn't merged yet. So in this PR, I've moved the colourise functionality back to the traceback module. We can refactor after that PR has been merged, and we might even be able to do that after the beta cutoff? doctest already imports traceback so there's no additional performance hit there. And it makes this patch smaller.

The widely-used pytest (116m monthly downloads) uses the same ANSI colours:

https://github.com/pytest-dev/pytest/blob/58844247f7ae4a7a213a73c8af2462253b3d8fc7/src/_pytest/_io/terminalwriter.py#L44-L66

@encukou made a good point in python/cherry-picker#120 (comment):

The 16 named colours are determined by the terminal. If there's e.g. not enough contrast between background and red in the default settings, you should probably report it to the terminal devs.

That also means you shouldn't use direct RGB values, or the 256-colour palette. (Unless you also paint the background using RGB, which looks bad if it's not a “full-screen” app.)

So, my suggestion: stick to 16 named colours, implement NO_COLOR & --color/--no-color to opt out, but don't over-analyze how your terminal renders things.

We're sticking to the 16 ANSI colours and have PYTHON_COLORS and NO_COLOR to disable.

@pablogsal How does the following look for you? Is the text still legible?

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot AMD64 RHEL7 LTO + PGO 3.x has failed when building commit 975081b.

What do you need to do:

1. Don't panic.
2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
3. Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/96/builds/7341) and take a look at the build logs.
4. Check if the failure is related to this commit (975081b) or if it is a false positive.
5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/96/builds/7341

Failed tests:

• test_capi

Summary of the results of the build (if available):

==

remote: Enumerating objects: 28, done.        
remote: Counting objects:   3% (1/28)        
remote: Counting objects:   7% (2/28)        
remote: Counting objects:  10% (3/28)        
remote: Counting objects:  14% (4/28)        
remote: Counting objects:  17% (5/28)        
remote: Counting objects:  21% (6/28)        
remote: Counting objects:  25% (7/28)        
remote: Counting objects:  28% (8/28)        
remote: Counting objects:  32% (9/28)        
remote: Counting objects:  35% (10/28)        
remote: Counting objects:  39% (11/28)        
remote: Counting objects:  42% (12/28)        
remote: Counting objects:  46% (13/28)        
remote: Counting objects:  50% (14/28)        
remote: Counting objects:  53% (15/28)        
remote: Counting objects:  57% (16/28)        
remote: Counting objects:  60% (17/28)        
remote: Counting objects:  64% (18/28)        
remote: Counting objects:  67% (19/28)        
remote: Counting objects:  71% (20/28)        
remote: Counting objects:  75% (21/28)        
remote: Counting objects:  78% (22/28)        
remote: Counting objects:  82% (23/28)        
remote: Counting objects:  85% (24/28)        
remote: Counting objects:  89% (25/28)        
remote: Counting objects:  92% (26/28)        
remote: Counting objects:  96% (27/28)        
remote: Counting objects: 100% (28/28)        
remote: Counting objects: 100% (28/28), done.        
remote: Compressing objects:   6% (1/15)        
remote: Compressing objects:  13% (2/15)        
remote: Compressing objects:  20% (3/15)        
remote: Compressing objects:  26% (4/15)        
remote: Compressing objects:  33% (5/15)        
remote: Compressing objects:  40% (6/15)        
remote: Compressing objects:  46% (7/15)        
remote: Compressing objects:  53% (8/15)        
remote: Compressing objects:  60% (9/15)        
remote: Compressing objects:  66% (10/15)        
remote: Compressing objects:  73% (11/15)        
remote: Compressing objects:  80% (12/15)        
remote: Compressing objects:  86% (13/15)        
remote: Compressing objects:  93% (14/15)        
remote: Compressing objects: 100% (15/15)        
remote: Compressing objects: 100% (15/15), done.        
remote: Total 15 (delta 13), reused 2 (delta 0), pack-reused 0        
From https://github.com/python/cpython
 * branch            main       -> FETCH_HEAD
Note: checking out '975081b11e052c9f8deb42c5876104651736302e'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by performing another checkout.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -b with the checkout command again. Example:

  git checkout -b new_branch_name

HEAD is now at 975081b... gh-117225: Add color to doctest output (#117583)
Switched to and reset branch 'main'

find: ‘build’: No such file or directory
find: ‘build’: No such file or directory
find: ‘build’: No such file or directory
find: ‘build’: No such file or directory
make[2]: [Makefile:3101: clean-retain-profile] Error 1 (ignored)

make: *** [Makefile:2232: buildbottest] Error 2

Docs PR: #118268

Refactoring PR: #118283