Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Color support on Windows: TerminalWriter (PY_COLORS), colorama (PYCHARM_HOSTED) #1468

Closed
eine opened this issue Nov 30, 2019 · 8 comments
Closed
Labels
feature:new something does not exist yet, but should help:wanted Issues that have been acknowledged, a solution determined and a PR might likely be accepted.

Comments

@eine
Copy link

eine commented Nov 30, 2019

While running several tests with tox on GitHub Actions environments, tox's output is not coloured by default (see actions/starter-workflows#114). Setting PY_COLORS: 1 (#1009) works for jobs running on GNU/Linux (ubuntu-latest). Unfortunately, it seems not to be effective on windows-latest.

Note that, in those examples, the output of the commands executed by tox is coloured because --color=yes is explicitly passed to pytest through {posargs}.

@eine eine added the feature:new something does not exist yet, but should label Nov 30, 2019
@gaborbernat gaborbernat added the help:wanted Issues that have been acknowledged, a solution determined and a PR might likely be accepted. label Nov 30, 2019
@gaborbernat
Copy link
Member

We'd happy to fix this if someone can investigate and come up with a PR for it.

@eine
Copy link
Author

eine commented Nov 30, 2019

I'd be happy to investigate if I have any reference. Currently, PY_COLORS seems not to be found in this repository, except in the help: https://github.com/tox-dev/tox/search?q=PY_COLORS&type=Code

@eine
Copy link
Author

eine commented Nov 30, 2019

Here is a minimal reproducer GitHub Actions workflow:

name: "tox"
on: [push]
jobs:
  tox:
    strategy:
      fail-fast: false
      max-parallel: 2
      matrix:
        os: [ ubuntu, windows ]
    runs-on: ${{ matrix.os }}-latest
    steps:
    - uses: actions/setup-python@v1
      with:
        python-version: 3.7
    - uses: actions/checkout@v1
    - run: pip3 install tox --progress-bar off
    - run: tox -e py37-test
    - run: tox -e py37-test
      env:
        PY_COLORS: 1

Output: https://github.com/1138-4EB/actions/runs/327196070

@eine
Copy link
Author

eine commented Dec 1, 2019

Upon further investigation, it seems that tox does not explicitly handle colouring at all:

tox/src/tox/reporter.py

Lines 101 to 102 in 9ee972e

def good(self, msg):
self.logline_if(Verbosity.QUIET, "good", msg, green=True)

tox/src/tox/reporter.py

Lines 72 to 75 in 9ee972e

def logline_if(self, level, of, msg, key=None, **kwargs):
if self.verbosity >= level:
message = str(msg) if key is None else "{}{}".format(key, msg)
self.logline(of, message, **kwargs)

tox/src/tox/reporter.py

Lines 77 to 83 in 9ee972e

def logline(self, of, msg, **opts):
self.reported_lines.append((of, msg))
timestamp = ""
if REPORTER_TIMESTAMP_ON:
timestamp = "{} ".format(datetime.now() - START)
line_msg = "{}{}\n".format(timestamp, msg)
self.tw.write(line_msg, **opts)

self.tw = py.io.TerminalWriter()

Hence, PY_COLORS is handled by py.io.TerminalWriter(). This is mentioned in #163, but not in the help/docs. I'd suggest to please explain this explicitly (#1009). For example:

tox depends on py.io.TerminalWriter to handle colouring, which supports forcing the behaviour with environment variable PY_COLORS: 0 disable, 1 enable (default).


Looking at the source of TerminalWriter, the usage of PY_COLORS seems straightforward:

https://github.com/pytest-dev/py/blob/1e99d20f31ef511fc2f1a9e49ba970acad441d4e/py/_io/terminalwriter.py#L131-L138

This issue might be related to the fact that TerminalWriter seems to depend on colorama when on windows:

https://github.com/pytest-dev/py/blob/1e99d20f31ef511fc2f1a9e49ba970acad441d4e/py/_io/terminalwriter.py#L15-L24

Unfortunately, colorama is currently not installed as a dependency; users need to do it explicitly. Moreover, it seems to be used only when a tty is available:

https://github.com/pytest-dev/py/blob/1e99d20f31ef511fc2f1a9e49ba970acad441d4e/py/_io/terminalwriter.py#L157-L158

As a result, it might be a bug in TerminalWriter, as using colorama might need to be forced when PY_COLORS is set (similar to pytest-dev/py#224). Still:


An alternative solution to properly honouring PY_COLORS in both TerminalWriter and colorama, is to have tartley/colorama#226 tested and merged, and then use it in TerminalWriter too.

@gaborbernat
Copy link
Member

Closed via #1471

@eine
Copy link
Author

eine commented Dec 2, 2019

@gaborbernat, note that this was not closed automatically on purpose. #1471 is just a quick fix and it might seem to work, but I believe that this issue needs to remain open. Let me elaborate:

With the current version of TerminalWriter, ensuring that colorama is installed will avoid win32_and_ctypes being used. However, colorama is not actually used unless a tty is available (which is not the case in some CI services, such as GHA):

https://github.com/pytest-dev/py/blob/1e99d20f31ef511fc2f1a9e49ba970acad441d4e/py/_io/terminalwriter.py#L157-L158

Hence, the current master of tox generates Linux type coloring on Windows when no TTY is used. I guess that this might work on web frontend, on bash (MSYS2/MINGW) and probably on powershell; but it is likely to be broken on cmd.

This is an effort to bring TerminalWriter to tox's codebase and clean it up so that colorama is always used on Windows. Unfortunately, in this context PY_COLORS=1 alone is uneffective, because not being a tty colorama requires PYCHARM_HOSTED=1 to be set too. I tried to make tox set it automatically, but it seems not to work.

Hence, as commented above, I think that it would make sense if colorama supported PY_COLORS; or if TerminalWriter supported FORCE_COLOR and NOCOLOR (tartley/colorama#230). Any of those would allow a single envvar to be used while ensuring that colorama is allowed to handle any terminal on windows.

Anyway, I later found #1400. It seems that py is removed completely and colorama is directly used instead. I think that's a great decission and I'm looking forward to having it released. Nonetheless, I believe that this issue should remain open until it is merged or until any other of the solutions is applied.

@gaborbernat gaborbernat reopened this Dec 2, 2019
@eine eine changed the title PY_COLORS not effective on GitHub Actions windows-latest Color support on Windows: TerminalWriter (PY_COLORS), colorama (PYCHARM_HOSTED) Dec 2, 2019
@gaborbernat
Copy link
Member

My available efforts at the moment are aimed at fixing this as part of #1394, but that probably will take a while (ETA September).

@gaborbernat
Copy link
Member

I think this has been fixed on version 4 by using the colorama package.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature:new something does not exist yet, but should help:wanted Issues that have been acknowledged, a solution determined and a PR might likely be accepted.
Projects
None yet
Development

No branches or pull requests

2 participants